Re: [PATCH/RFC] Reduce call chain length in netfilter

[Patrick McHardy <kaber@trash.net>]

David S. Miller wrote:

>I've been trying to figure out ways to decrease the number of
>args that get sent to nf_hook_slow but this would require
>some API changes unfortunately.
>
>One idea goes like this, we create little descriptors of the form:
>
>struct nf_hook_desc {
>	int (*okfn)(struct sk_buff *);
>	int pf;
>	int hook;
>};
>
>Then NF_HOOK*() callsites do something like this:
>
>static const struct nf_hook_desc nf_ip_local_out = {
>	.okfn = dst_output,
>	.pf = PF_INET,
>	.hook = NF_IP_LOCAL_OUT,
>};
>
>...
>
>	/* Send it out. */
>	return NF_HOOK(&nf_ip_local_out, skb, NULL, rt->u.dst.dev);
>
>This gets us down to 4 arguments from 6.  I think we can kill
>one more.
>
>It is never the case that both indev and outdev are both
>set, so we can use some nf_hook_desc piece of state to
>indicate which (in or out) the passed device pointer is.
>  
>
indev and outdev are both set in the forward hook.

>Oh yes, we can nicely add the thresh thing in here too
>while we're at it.
>
>So the final nf_hook_desc might look something something like:
>
>struct nf_hook_desc {
>	int (*okfn)(struct sk_buff *);
>	int hook;
>	int thresh;
>	u8 pf; /* AF_MAX is 32 */
>	u8 is_output;
>};
>
>Hook could possibly use a smaller type as well to condense
>the size of this thing even further.  I don't know if there
>are any nice assumptions we can make about the hook numbers.
>  
>
There are currently five hooks. I really hope we'll never reach
256, so u8 should be big enough.

>Now, back to the compatability issue.  We could create a
>new macro, NF_HOOK_DESC() and keep the existing ones around
>via some nf_hook_slow() that basically does:
>
>int nf_hook_slow(int pf, unsigned int hook, struct sk_buff *skb,
>		 struct net_device *indev, struct net_device *outdev,
>		 int (*okfn)(struct sk_buff *), int thresh)
>{
>	struct nf_hook_desc desc;
>
>	desc.okfn = okfn;
>	desc.hook = hook;
>	desc.thresh = thresh;
>	desc.pf = pf;
>	desc.is_output = (outdev != NULL);
>	return nf_hook_desc(&desc, skb, (outdev ? outdev : indev));
>}
>
>So the final new stuff looks something like:
>
>#ifdef CONFIG_NETFILTER
>struct nf_hook_desc {
>	int (*okfn)(struct sk_buff *);
>	int hook;
>	int thresh;
>	u8 pf; /* AF_MAX is 32 */
>	u8 is_output;
>};
>#define NF_DESC_DECLARE(_name, _okfn, _hook, _thresh, _pf, _is_output) \
>static const struct nf_hook_desc _name = { \
>	.okfn = _okfn, \
>	.hook = _hook, \
>	.thresh = _thresh, \
>	.pf = _pf, \
>	.is_output = _is_output, \
>};
>
>extern int nf_hook_desc(struct nf_hook_desc *desc, struct sk_buff *skb,
>			struct net_device *dev);
>
>#define NF_HOOK_DESC(_desc, _skb, _dev) \
>	nf_hook_desc(_desc, _skb, _dev)
>#endif
>
>Just throwing around ideas... comments?
>  
>
Sounds like a good idea to get rid of the static arguments to
nf_hook_slow. Keeping both devices we are still down from 7 to
4 arguments with your suggestion.

Regards
Patrick