Re: [PATCH nf] netfilter: arptables: use percpu jumpstack

[Florian Westphal <fw@strlen.de>]

Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2015-07-02 at 13:48 +0200, Florian Westphal wrote:
> 
> > My plan:
> > 
> > - move tee_active percpu varible to xtables core (suggested by Eric)
> > - in do_table, check if we're TEE'd or not
> > 
> > 1. if no, then just use the jumpstack from offset 0 onwards.
> > 2. If yes, then fetch jumpstack, and use the upper half:
> > 
> > if (__this_cpu_read(xt_tee_active))
> >  	jumpstack += private->stacksize;
> 
> Or maybe not using a conditional
> 
> jumpstack += private->stacksize * __this_cpu_read(xt_tee_active);
> 
> 
> BTW, I do not remember why I used a conditional in
> xt_write_recseq_begin(). This also adds extra setup cost, as @addend has
> to be preserved in the stack.
> 
> Hmm... What about something like :

Hmm.  I don't understand how this would work reliably.

   xt_write_recseq_begin(); /* value is now odd */

  /* other cpu fetches counters, blocks in read seeqlock */
   for_each_rule( .. ) {
	-> packet is sent by some target
  	  /* reentry into do_table */
	  xt_write_recseq_begin(); /* value is now even */

          -> other cpu unblocks since it thinks seqlock is taken

   }

This is problematic because get_counters() (and thus the seqlock) seems
to be used as sync point with table replacement.

And reading Jans email, we can have re-entrancy in do_table() also
via -j REJECT and SYNPROXY, not just TEE :-/

MAYBE its worth investigating a dual stack approach.

Keep a small jumpstack on the local kernel stack

If table traverse exeeds it, switch to the private->jumpstack

This adds a conditional, but would avoid all the setup/restore/save for
most rulesets.