dpif-netdev: fix dp_netdev_free()

dp_netdev_free() must free 'dp->upcall_rwlock', but when upcalls are
disabled (if the datapath is being freed upcalls should be disabled)
'dp->upcall_rwlock' is taken and freeing it causes an assertion to
fail.

This commit takes makes sure that the upcalls are disabled and
releases 'dp->upcall_rwlock' before freeing it. A simple testcase is
added to detect the failure.

Signed-off-by: Daniele Di Proietto <ddiproietto@vmware.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 6b8201b..6029d3f 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -618,6 +618,18 @@ dpif_netdev_open(const struct dpif_class *class, const char *name,
     return error;
 }
 
+static void
+dp_netdev_destroy_upcall_lock(struct dp_netdev *dp)
+    OVS_NO_THREAD_SAFETY_ANALYSIS
+{
+    /* Check that upcalls are disabled, i.e. that the rwlock is taken */
+    ovs_assert(fat_rwlock_tryrdlock(&dp->upcall_rwlock));
+
+    /* Before freeing a lock we should release it */
+    fat_rwlock_unlock(&dp->upcall_rwlock);
+    fat_rwlock_destroy(&dp->upcall_rwlock);
+}
+
 /* Requires dp_netdev_mutex so that we can't get a new reference to 'dp'
  * through the 'dp_netdevs' shash while freeing 'dp'. */
 static void
@@ -652,7 +664,9 @@ dp_netdev_free(struct dp_netdev *dp)
     ovs_mutex_destroy(&dp->flow_mutex);
     seq_destroy(dp->port_seq);
     cmap_destroy(&dp->ports);
-    fat_rwlock_destroy(&dp->upcall_rwlock);
+
+    /* Upcalls must be disabled at this point */
+    dp_netdev_destroy_upcall_lock(dp);
 
     free(dp->pmd_cmask);
     free(CONST_CAST(char *, dp->name));

diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index af7845c..5711ebe 100644 (file)
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -122,3 +122,11 @@ skb_priority(0/0),skb_mark(0/0),recirc_id(0),dp_hash(0/0),in_port(1),eth(src=50:
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([dpif-netdev - Datapath removal])
+OVS_VSWITCHD_START()
+AT_CHECK([ovs-appctl dpctl/add-dp dummy@br0])
+AT_CHECK([ovs-appctl dpctl/del-dp dummy@br0])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP