On Thu, 27 Jan 2005 00:49:01 +0100
Patrick McHardy <kaber@xxxxxxxxx> 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 <kaber@xxxxxxxxx>
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?
|