On Mon, Aug 26, 2013 at 2:03 AM, Viresh Kumar <viresh.ku...@linaro.org> wrote: > On 24 August 2013 06:30, Jesse Gross <je...@nicira.com> wrote: >> Putting a big lock around the majority of the packet processing >> doesn't seem like a particularly good idea for performance and you >> would need to make sure that you get all the entry points. You would >> probably be better off serializing the parts that actually need it, >> like updating stats counters. > > Obviously.. I didn't knew earlier what exactly are we looking to protect > here :) > > Does something like this makes sense? > > diff --git a/datapath/actions.c b/datapath/actions.c > index fa4b904..37ab132 100644 > --- a/datapath/actions.c > +++ b/datapath/actions.c > @@ -607,7 +607,12 @@ int ovs_execute_actions(struct datapath *dp, > struct sk_buff *skb) > int error; > > /* Check whether we've looped too much. */ > +#ifdef CONFIG_PREEMPT_RT_FULL > + get_cpu_light(); > loop = &__get_cpu_var(loop_counters); > +#else > + loop = &get_cpu_var(loop_counters); > +#endif
Please look at the implementation of the loop counter. The current method will yield non-deterministic results in the presence of preemption. Have you also audited the other use of per-CPU variables? _______________________________________________ discuss mailing list discuss@openvswitch.org http://openvswitch.org/mailman/listinfo/discuss