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

Reply via email to