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?
