From 88ace79b3e0e7808338e5c70b7554a59f4bc3600 Mon Sep 17 00:00:00 2001 From: Daniele Di Proietto Date: Fri, 3 Oct 2014 15:04:15 -0700 Subject: [PATCH] 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 Acked-by: Jarno Rajahalme --- lib/dpif-netdev.c | 16 +++++++++++++++- tests/dpif-netdev.at | 8 ++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 6b8201b22..6029d3f10 100644 --- 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 af7845c2e..5711ebe47 100644 --- 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 -- 2.39.5