[ovs-dev] [PATCH 1/6] dpif-netdev: fix dp_netdev_free()

[Jarno Rajahalme]

Thanks Daniele!

For the series:

Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>

and pushed to master.

  Jarno

On Oct 3, 2014, at 9:15 AM, Daniele Di Proietto <ddiproietto at vmware.com> wrote:

> 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 at vmware.com>
> ---
> 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 6b8201b..6029d3f 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 af7845c..5711ebe 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 at br0])
> +AT_CHECK([ovs-appctl dpctl/del-dp dummy at br0])
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> -- 
> 2.1.0.rc1
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev