Thanks for the prompt review Jarno!
Daniele
On 10/3/14, 3:18 PM, "Jarno Rajahalme" <jrajaha...@nicira.com> wrote:
>Thanks Daniele!
>
>For the series:
>
>Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
>
>and pushed to master.
>
> Jarno
>
>On Oct 3, 2014, at 9:15 AM, Daniele Di Proietto <ddiproie...@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 <ddiproie...@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@br0])
>> +AT_CHECK([ovs-appctl dpctl/del-dp dummy@br0])
>> +
>> +OVS_VSWITCHD_STOP
>> +AT_CLEANUP
>> --
>> 2.1.0.rc1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>>
>>https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman
>>/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=MV9BdLjtFIdhBDBaw5z%2
>>BU6SSA2gAfY4L%2F1HCy3VjlKU%3D%0A&m=cW4LDH7S4%2BoFVjdmcu1JEr2G%2FvqhcuxY6q
>>dPqJ%2F3x%2FI%3D%0A&s=af637d4aae6b775e42f846234879d8fa5949ff2f7ae7377f01a
>>391603adb263f
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
