On Wed, Jan 29, 2014 at 4:42 PM, Pravin Shelar <[email protected]> wrote: > On Wed, Jan 29, 2014 at 4:38 PM, Jesse Gross <[email protected]> wrote: >> On Wed, Jan 29, 2014 at 4:35 PM, Pravin Shelar <[email protected]> wrote: >>> On Wed, Jan 29, 2014 at 4:21 PM, Jesse Gross <[email protected]> wrote: >>>> On Wed, Jan 29, 2014 at 11:22 AM, Pravin Shelar <[email protected]> wrote: >>>>> On Wed, Jan 29, 2014 at 10:44 AM, Jesse Gross <[email protected]> wrote: >>>>>> On Wed, Jan 29, 2014 at 9:20 AM, <[email protected]> wrote: >>>>>>> From: Pravin <[email protected]> >>>>>>> >>>>>>> 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 <[email protected]> >>>>>> >>>>>> 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().
Ah, OK. When I read packet execute in the commit message, I thought that you were referring to general packet processing. I understand the problem now - can you update the commit message with the specific function name? Acked-by: Jesse Gross <[email protected]> _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
