On Thu, 27 Jan 2005 00:49:01 +0100
Patrick McHardy <[EMAIL PROTECTED]> wrote:

> Bart De Schuymer wrote:
> 
> >Does anyone have objections to this patch, which reduces the netfilter
> >call chain length?
> >
> Looks fine to me.
> 
> Signed-off-by: Patrick McHardy <[EMAIL PROTECTED]>

Ok, I applied this.

While reviewing I thought it may be an issue that the new macros
potentially change skb.  It really isn't an issue because NF_HOOK()
calls pass ownership of the SKB over from the caller.

Although technically, someone could go:

        skb_get(skb);
        err = NF_HOOK(... skb ...);
        ... do stuff with skb ...
        kfree_skb(skb);

but that would cause other problems and I audited the entire tree
and nobody attempts anything like this currently.  'skb' always
dies at the NF_HOOK() call site.

I guess if we wanted to preserve NF_HOOK*() semantics even in such
a case we could use a local "__skb" var in the macro's basic block.

Another huge downside to this change I was worried about
was from a code generation point of view.  Since we now take the
address of "skb", gcc cannot generate tail-calls for the common
case of:

        return NF_HOOK(...);

when netfilter is enabled.  Ho hum...

Wait...

This is actually an important point!  Since gcc is generating a tail-
call for NF_HOOK() today, there is no stack savings for NF_HOOK()
created by this patch.  The only real gain is the NF_STOP stuff
for bridge netfilter.

I'm backing this out of my tree, let's think about this some more.
Perhaps it's only worth adding the NF_STOP thing and just making
nf_hook_slow() do the okfn(skb); call in that case?
_______________________________________________
Bridge mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/bridge

Reply via email to