[ovs-dev] [PATCH] datapath: Fix ovs_flow_free() ovs-lock assert.

Pravin Shelar pshelar at nicira.com
Thu Jan 30 00:42:16 UTC 2014


On Wed, Jan 29, 2014 at 4:38 PM, Jesse Gross <jesse at nicira.com> wrote:
> On Wed, Jan 29, 2014 at 4:35 PM, Pravin Shelar <pshelar at nicira.com> wrote:
>> On Wed, Jan 29, 2014 at 4:21 PM, Jesse Gross <jesse at nicira.com> wrote:
>>> On Wed, Jan 29, 2014 at 11:22 AM, Pravin Shelar <pshelar at nicira.com> wrote:
>>>> On Wed, Jan 29, 2014 at 10:44 AM, Jesse Gross <jesse at nicira.com> wrote:
>>>>> On Wed, Jan 29, 2014 at 9:20 AM,  <pshelar at nicira.com> wrote:
>>>>>> From: Pravin <pshelar at nicira.com>
>>>>>>
>>>>>> ovs_flow_free() is not called under ovs-lock during packet
>>>>>> execute path. Since packet execute does not touch flow->mask,
>>>>>> there is no need to take that lock either. So move assert in
>>>>>> case where flow->mask is checked.
>>>>>>
>>>>>> Found by code inspection.
>>>>>>
>>>>>> Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
>>>>>
>>>>> The idea for putting it here is that callers should always hold OVS
>>>>> mutex and by putting the check earlier it increases the chances that
>>>>> problems will be caught sooner.
>>>>
>>>> But it is not protecting any data outside of that flow->mask case, so
>>>> it is not required.
>>>
>>> I agree that it's not required but I don't understand the benefit to
>>> removing it. Are there cases where we want to call this function where
>>> OVS mutex isn't held because there is no flow mask?
>>
>> I kept it because it was very recently added. I am ok with removing
>> it. I will send patch.
>
> I think we are saying the opposite - I don't understand why we want to
> remove it/make it trigger less often.

yes, from ovs_packet_cmd_execute().



More information about the dev mailing list