I updated commit and added comment and pushed patch to master and 2.1. Thanks, Pravin.
On Thu, Jan 30, 2014 at 6:13 PM, Jesse Gross <je...@nicira.com> wrote: > On Wed, Jan 29, 2014 at 7:18 PM, Jesse Gross <je...@nicira.com> wrote: >> On Wed, Jan 29, 2014 at 4:42 PM, Pravin Shelar <pshe...@nicira.com> wrote: >>> On Wed, Jan 29, 2014 at 4:38 PM, Jesse Gross <je...@nicira.com> wrote: >>>> On Wed, Jan 29, 2014 at 4:35 PM, Pravin Shelar <pshe...@nicira.com> wrote: >>>>> On Wed, Jan 29, 2014 at 4:21 PM, Jesse Gross <je...@nicira.com> wrote: >>>>>> On Wed, Jan 29, 2014 at 11:22 AM, Pravin Shelar <pshe...@nicira.com> >>>>>> wrote: >>>>>>> On Wed, Jan 29, 2014 at 10:44 AM, Jesse Gross <je...@nicira.com> wrote: >>>>>>>> On Wed, Jan 29, 2014 at 9:20 AM, <pshe...@nicira.com> wrote: >>>>>>>>> From: Pravin <pshe...@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 <pshe...@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(). >> >> 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 <je...@nicira.com> > > If this is all set, would you mind pushing it? Thomas asked that I > send a pull request with fixes for net-next and I would like to > include this at the same time. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
