From 1610a73c4175e7d63985316b52ac932b65a4dc90 Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Thu, 3 Nov 2016 10:56:12 +0100 Subject: netfilter: kill NF_HOOK_THRESH() and state->tresh Patch c5136b15ea36 ("netfilter: bridge: add and use br_nf_hook_thresh") introduced br_nf_hook_thresh(). Replace NF_HOOK_THRESH() by br_nf_hook_thresh from br_nf_forward_finish(), so we have no more callers for this macro. As a result, state->thresh and explicit thresh parameter in the hook state structure is not required anymore. And we can get rid of skip-hook-under-thresh loop in nf_iterate() in the core path that is only used by br_netfilter to search for the filter hook. Suggested-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso --- include/linux/netfilter.h | 50 ++++++++++++----------------------------------- 1 file changed, 13 insertions(+), 37 deletions(-) (limited to 'include/linux/netfilter.h') diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h index abc7fdcb9eb1..e0d000f6c9bf 100644 --- a/include/linux/netfilter.h +++ b/include/linux/netfilter.h @@ -49,7 +49,6 @@ struct sock; struct nf_hook_state { unsigned int hook; - int thresh; u_int8_t pf; struct net_device *in; struct net_device *out; @@ -84,7 +83,7 @@ struct nf_hook_entry { static inline void nf_hook_state_init(struct nf_hook_state *p, struct nf_hook_entry *hook_entry, unsigned int hook, - int thresh, u_int8_t pf, + u_int8_t pf, struct net_device *indev, struct net_device *outdev, struct sock *sk, @@ -92,7 +91,6 @@ static inline void nf_hook_state_init(struct nf_hook_state *p, int (*okfn)(struct net *, struct sock *, struct sk_buff *)) { p->hook = hook; - p->thresh = thresh; p->pf = pf; p->in = indev; p->out = outdev; @@ -155,20 +153,16 @@ extern struct static_key nf_hooks_needed[NFPROTO_NUMPROTO][NF_MAX_HOOKS]; int nf_hook_slow(struct sk_buff *skb, struct nf_hook_state *state); /** - * nf_hook_thresh - call a netfilter hook + * nf_hook - call a netfilter hook * * Returns 1 if the hook has allowed the packet to pass. The function * okfn must be invoked by the caller in this case. Any other return * value indicates the packet has been consumed by the hook. */ -static inline int nf_hook_thresh(u_int8_t pf, unsigned int hook, - struct net *net, - struct sock *sk, - struct sk_buff *skb, - struct net_device *indev, - struct net_device *outdev, - int (*okfn)(struct net *, struct sock *, struct sk_buff *), - int thresh) +static inline int nf_hook(u_int8_t pf, unsigned int hook, struct net *net, + struct sock *sk, struct sk_buff *skb, + struct net_device *indev, struct net_device *outdev, + int (*okfn)(struct net *, struct sock *, struct sk_buff *)) { struct nf_hook_entry *hook_head; int ret = 1; @@ -185,8 +179,8 @@ static inline int nf_hook_thresh(u_int8_t pf, unsigned int hook, if (hook_head) { struct nf_hook_state state; - nf_hook_state_init(&state, hook_head, hook, thresh, - pf, indev, outdev, sk, net, okfn); + nf_hook_state_init(&state, hook_head, hook, pf, indev, outdev, + sk, net, okfn); ret = nf_hook_slow(skb, &state); } @@ -195,14 +189,6 @@ static inline int nf_hook_thresh(u_int8_t pf, unsigned int hook, return ret; } -static inline int nf_hook(u_int8_t pf, unsigned int hook, struct net *net, - struct sock *sk, struct sk_buff *skb, - struct net_device *indev, struct net_device *outdev, - int (*okfn)(struct net *, struct sock *, struct sk_buff *)) -{ - return nf_hook_thresh(pf, hook, net, sk, skb, indev, outdev, okfn, INT_MIN); -} - /* Activate hook; either okfn or kfree_skb called, unless a hook returns NF_STOLEN (in which case, it's up to the hook to deal with the consequences). @@ -220,19 +206,6 @@ static inline int nf_hook(u_int8_t pf, unsigned int hook, struct net *net, coders :) */ -static inline int -NF_HOOK_THRESH(uint8_t pf, unsigned int hook, struct net *net, struct sock *sk, - struct sk_buff *skb, struct net_device *in, - struct net_device *out, - int (*okfn)(struct net *, struct sock *, struct sk_buff *), - int thresh) -{ - int ret = nf_hook_thresh(pf, hook, net, sk, skb, in, out, okfn, thresh); - if (ret == 1) - ret = okfn(net, sk, skb); - return ret; -} - static inline int NF_HOOK_COND(uint8_t pf, unsigned int hook, struct net *net, struct sock *sk, struct sk_buff *skb, struct net_device *in, struct net_device *out, @@ -242,7 +215,7 @@ NF_HOOK_COND(uint8_t pf, unsigned int hook, struct net *net, struct sock *sk, int ret; if (!cond || - ((ret = nf_hook_thresh(pf, hook, net, sk, skb, in, out, okfn, INT_MIN)) == 1)) + ((ret = nf_hook(pf, hook, net, sk, skb, in, out, okfn)) == 1)) ret = okfn(net, sk, skb); return ret; } @@ -252,7 +225,10 @@ NF_HOOK(uint8_t pf, unsigned int hook, struct net *net, struct sock *sk, struct struct net_device *in, struct net_device *out, int (*okfn)(struct net *, struct sock *, struct sk_buff *)) { - return NF_HOOK_THRESH(pf, hook, net, sk, skb, in, out, okfn, INT_MIN); + int ret = nf_hook(pf, hook, net, sk, skb, in, out, okfn); + if (ret == 1) + ret = okfn(net, sk, skb); + return ret; } /* Call setsockopt() */ -- cgit v1.3-7-g2ca7 From 01886bd91f1ba418ce669dfe97a06ca9504e482a Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Thu, 3 Nov 2016 10:56:35 +0100 Subject: netfilter: remove hook_entries field from nf_hook_state This field is only useful for nf_queue, so store it in the nf_queue_entry structure instead, away from the core path. Pass hook_head to nf_hook_slow(). Since we always have a valid entry on the first iteration in nf_iterate(), we can use 'do { ... } while (entry)' loop instead. Signed-off-by: Pablo Neira Ayuso --- include/linux/netfilter.h | 10 ++++------ include/linux/netfilter_ingress.h | 4 ++-- include/net/netfilter/nf_queue.h | 1 + net/bridge/br_netfilter_hooks.c | 4 ++-- net/bridge/netfilter/ebtable_broute.c | 2 +- net/netfilter/core.c | 9 ++++----- net/netfilter/nf_queue.c | 13 +++++-------- net/netfilter/nfnetlink_queue.c | 2 +- 8 files changed, 20 insertions(+), 25 deletions(-) (limited to 'include/linux/netfilter.h') diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h index e0d000f6c9bf..69230140215b 100644 --- a/include/linux/netfilter.h +++ b/include/linux/netfilter.h @@ -54,7 +54,6 @@ struct nf_hook_state { struct net_device *out; struct sock *sk; struct net *net; - struct nf_hook_entry __rcu *hook_entries; int (*okfn)(struct net *, struct sock *, struct sk_buff *); }; @@ -81,7 +80,6 @@ struct nf_hook_entry { }; static inline void nf_hook_state_init(struct nf_hook_state *p, - struct nf_hook_entry *hook_entry, unsigned int hook, u_int8_t pf, struct net_device *indev, @@ -96,7 +94,6 @@ static inline void nf_hook_state_init(struct nf_hook_state *p, p->out = outdev; p->sk = sk; p->net = net; - RCU_INIT_POINTER(p->hook_entries, hook_entry); p->okfn = okfn; } @@ -150,7 +147,8 @@ void nf_unregister_sockopt(struct nf_sockopt_ops *reg); extern struct static_key nf_hooks_needed[NFPROTO_NUMPROTO][NF_MAX_HOOKS]; #endif -int nf_hook_slow(struct sk_buff *skb, struct nf_hook_state *state); +int nf_hook_slow(struct sk_buff *skb, struct nf_hook_state *state, + struct nf_hook_entry *entry); /** * nf_hook - call a netfilter hook @@ -179,10 +177,10 @@ static inline int nf_hook(u_int8_t pf, unsigned int hook, struct net *net, if (hook_head) { struct nf_hook_state state; - nf_hook_state_init(&state, hook_head, hook, pf, indev, outdev, + nf_hook_state_init(&state, hook, pf, indev, outdev, sk, net, okfn); - ret = nf_hook_slow(skb, &state); + ret = nf_hook_slow(skb, &state, hook_head); } rcu_read_unlock(); diff --git a/include/linux/netfilter_ingress.h b/include/linux/netfilter_ingress.h index fd44e4131710..2dc3b49b804a 100644 --- a/include/linux/netfilter_ingress.h +++ b/include/linux/netfilter_ingress.h @@ -26,10 +26,10 @@ static inline int nf_hook_ingress(struct sk_buff *skb) if (unlikely(!e)) return 0; - nf_hook_state_init(&state, e, NF_NETDEV_INGRESS, + nf_hook_state_init(&state, NF_NETDEV_INGRESS, NFPROTO_NETDEV, skb->dev, NULL, NULL, dev_net(skb->dev), NULL); - return nf_hook_slow(skb, &state); + return nf_hook_slow(skb, &state, e); } static inline void nf_hook_ingress_init(struct net_device *dev) diff --git a/include/net/netfilter/nf_queue.h b/include/net/netfilter/nf_queue.h index 2280cfe86c56..09948d10e38e 100644 --- a/include/net/netfilter/nf_queue.h +++ b/include/net/netfilter/nf_queue.h @@ -12,6 +12,7 @@ struct nf_queue_entry { unsigned int id; struct nf_hook_state state; + struct nf_hook_entry *hook; u16 size; /* sizeof(entry) + saved route keys */ /* extra space to store route keys */ diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c index 7e3645fa6339..8155bd2a5138 100644 --- a/net/bridge/br_netfilter_hooks.c +++ b/net/bridge/br_netfilter_hooks.c @@ -1018,10 +1018,10 @@ int br_nf_hook_thresh(unsigned int hook, struct net *net, /* We may already have this, but read-locks nest anyway */ rcu_read_lock(); - nf_hook_state_init(&state, elem, hook, NFPROTO_BRIDGE, indev, outdev, + nf_hook_state_init(&state, hook, NFPROTO_BRIDGE, indev, outdev, sk, net, okfn); - ret = nf_hook_slow(skb, &state); + ret = nf_hook_slow(skb, &state, elem); rcu_read_unlock(); if (ret == 1) ret = okfn(net, sk, skb); diff --git a/net/bridge/netfilter/ebtable_broute.c b/net/bridge/netfilter/ebtable_broute.c index 599679e3498d..8fe36dc3aab2 100644 --- a/net/bridge/netfilter/ebtable_broute.c +++ b/net/bridge/netfilter/ebtable_broute.c @@ -53,7 +53,7 @@ static int ebt_broute(struct sk_buff *skb) struct nf_hook_state state; int ret; - nf_hook_state_init(&state, NULL, NF_BR_BROUTING, + nf_hook_state_init(&state, NF_BR_BROUTING, NFPROTO_BRIDGE, skb->dev, NULL, NULL, dev_net(skb->dev), NULL); diff --git a/net/netfilter/core.c b/net/netfilter/core.c index 64623374bc5f..ebece48b8392 100644 --- a/net/netfilter/core.c +++ b/net/netfilter/core.c @@ -308,7 +308,7 @@ unsigned int nf_iterate(struct sk_buff *skb, { unsigned int verdict; - while (*entryp) { + do { repeat: verdict = (*entryp)->ops.hook((*entryp)->ops.priv, skb, state); if (verdict != NF_ACCEPT) { @@ -317,20 +317,19 @@ repeat: goto repeat; } *entryp = rcu_dereference((*entryp)->next); - } + } while (*entryp); return NF_ACCEPT; } /* Returns 1 if okfn() needs to be executed by the caller, * -EPERM for NF_DROP, 0 otherwise. Caller must hold rcu_read_lock. */ -int nf_hook_slow(struct sk_buff *skb, struct nf_hook_state *state) +int nf_hook_slow(struct sk_buff *skb, struct nf_hook_state *state, + struct nf_hook_entry *entry) { - struct nf_hook_entry *entry; unsigned int verdict; int ret; - entry = rcu_dereference(state->hook_entries); next_hook: verdict = nf_iterate(skb, state, &entry); switch (verdict & NF_VERDICT_MASK) { diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c index 0fb38966e5bf..2e39e38ae1c7 100644 --- a/net/netfilter/nf_queue.c +++ b/net/netfilter/nf_queue.c @@ -108,7 +108,7 @@ void nf_queue_nf_hook_drop(struct net *net, const struct nf_hook_entry *entry) } static int __nf_queue(struct sk_buff *skb, const struct nf_hook_state *state, - unsigned int queuenum) + struct nf_hook_entry *hook_entry, unsigned int queuenum) { int status = -ENOENT; struct nf_queue_entry *entry = NULL; @@ -136,6 +136,7 @@ static int __nf_queue(struct sk_buff *skb, const struct nf_hook_state *state, *entry = (struct nf_queue_entry) { .skb = skb, .state = *state, + .hook = hook_entry, .size = sizeof(*entry) + afinfo->route_key_size, }; @@ -163,8 +164,7 @@ int nf_queue(struct sk_buff *skb, struct nf_hook_state *state, struct nf_hook_entry *entry = *entryp; int ret; - RCU_INIT_POINTER(state->hook_entries, entry); - ret = __nf_queue(skb, state, verdict >> NF_VERDICT_QBITS); + ret = __nf_queue(skb, state, entry, verdict >> NF_VERDICT_QBITS); if (ret < 0) { if (ret == -ESRCH && (verdict & NF_VERDICT_FLAG_QUEUE_BYPASS)) { @@ -179,15 +179,12 @@ int nf_queue(struct sk_buff *skb, struct nf_hook_state *state, void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict) { - struct nf_hook_entry *hook_entry; + struct nf_hook_entry *hook_entry = entry->hook; + struct nf_hook_ops *elem = &hook_entry->ops; struct sk_buff *skb = entry->skb; const struct nf_afinfo *afinfo; - struct nf_hook_ops *elem; int err; - hook_entry = rcu_dereference(entry->state.hook_entries); - elem = &hook_entry->ops; - nf_queue_entry_release_refs(entry); /* Continue traversal iff userspace said ok... */ diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c index 5379f788a372..1e33115b399f 100644 --- a/net/netfilter/nfnetlink_queue.c +++ b/net/netfilter/nfnetlink_queue.c @@ -919,7 +919,7 @@ static struct notifier_block nfqnl_dev_notifier = { static int nf_hook_cmp(struct nf_queue_entry *entry, unsigned long entry_ptr) { - return rcu_access_pointer(entry->state.hook_entries) == + return rcu_access_pointer(entry->hook) == (struct nf_hook_entry *)entry_ptr; } -- cgit v1.3-7-g2ca7 From 0aa8c57a04907a5d02068ff9f917629be97ea78d Mon Sep 17 00:00:00 2001 From: Aaron Conole Date: Tue, 15 Nov 2016 17:48:44 -0500 Subject: netfilter: introduce accessor functions for hook entries This allows easier future refactoring. Signed-off-by: Aaron Conole Signed-off-by: Pablo Neira Ayuso --- include/linux/netfilter.h | 27 +++++++++++++++++++++++++++ net/bridge/br_netfilter_hooks.c | 2 +- net/netfilter/core.c | 10 ++++------ net/netfilter/nf_queue.c | 5 ++--- 4 files changed, 34 insertions(+), 10 deletions(-) (limited to 'include/linux/netfilter.h') diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h index 69230140215b..575aa198097e 100644 --- a/include/linux/netfilter.h +++ b/include/linux/netfilter.h @@ -79,6 +79,33 @@ struct nf_hook_entry { const struct nf_hook_ops *orig_ops; }; +static inline void +nf_hook_entry_init(struct nf_hook_entry *entry, const struct nf_hook_ops *ops) +{ + entry->next = NULL; + entry->ops = *ops; + entry->orig_ops = ops; +} + +static inline int +nf_hook_entry_priority(const struct nf_hook_entry *entry) +{ + return entry->ops.priority; +} + +static inline int +nf_hook_entry_hookfn(const struct nf_hook_entry *entry, struct sk_buff *skb, + struct nf_hook_state *state) +{ + return entry->ops.hook(entry->ops.priv, skb, state); +} + +static inline const struct nf_hook_ops * +nf_hook_entry_ops(const struct nf_hook_entry *entry) +{ + return entry->orig_ops; +} + static inline void nf_hook_state_init(struct nf_hook_state *p, unsigned int hook, u_int8_t pf, diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c index 83d937f4415e..adad2eed29e6 100644 --- a/net/bridge/br_netfilter_hooks.c +++ b/net/bridge/br_netfilter_hooks.c @@ -1010,7 +1010,7 @@ int br_nf_hook_thresh(unsigned int hook, struct net *net, elem = rcu_dereference(net->nf.hooks[NFPROTO_BRIDGE][hook]); - while (elem && (elem->ops.priority <= NF_BR_PRI_BRNF)) + while (elem && (nf_hook_entry_priority(elem) <= NF_BR_PRI_BRNF)) elem = rcu_dereference(elem->next); if (!elem) diff --git a/net/netfilter/core.c b/net/netfilter/core.c index de30e08d58f2..2bb46e2d8d30 100644 --- a/net/netfilter/core.c +++ b/net/netfilter/core.c @@ -102,15 +102,13 @@ int nf_register_net_hook(struct net *net, const struct nf_hook_ops *reg) if (!entry) return -ENOMEM; - entry->orig_ops = reg; - entry->ops = *reg; - entry->next = NULL; + nf_hook_entry_init(entry, reg); mutex_lock(&nf_hook_mutex); /* Find the spot in the list */ while ((p = nf_entry_dereference(*pp)) != NULL) { - if (reg->priority < p->orig_ops->priority) + if (reg->priority < nf_hook_entry_priority(p)) break; pp = &p->next; } @@ -140,7 +138,7 @@ void nf_unregister_net_hook(struct net *net, const struct nf_hook_ops *reg) mutex_lock(&nf_hook_mutex); while ((p = nf_entry_dereference(*pp)) != NULL) { - if (p->orig_ops == reg) { + if (nf_hook_entry_ops(p) == reg) { rcu_assign_pointer(*pp, p->next); break; } @@ -311,7 +309,7 @@ int nf_hook_slow(struct sk_buff *skb, struct nf_hook_state *state, int ret; do { - verdict = entry->ops.hook(entry->ops.priv, skb, state); + verdict = nf_hook_entry_hookfn(entry, skb, state); switch (verdict & NF_VERDICT_MASK) { case NF_ACCEPT: entry = rcu_dereference(entry->next); diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c index 77cba9f6ccb6..4a7662486f44 100644 --- a/net/netfilter/nf_queue.c +++ b/net/netfilter/nf_queue.c @@ -185,7 +185,7 @@ static unsigned int nf_iterate(struct sk_buff *skb, do { repeat: - verdict = (*entryp)->ops.hook((*entryp)->ops.priv, skb, state); + verdict = nf_hook_entry_hookfn((*entryp), skb, state); if (verdict != NF_ACCEPT) { if (verdict != NF_REPEAT) return verdict; @@ -200,7 +200,6 @@ repeat: void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict) { struct nf_hook_entry *hook_entry = entry->hook; - struct nf_hook_ops *elem = &hook_entry->ops; struct sk_buff *skb = entry->skb; const struct nf_afinfo *afinfo; int err; @@ -209,7 +208,7 @@ void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict) /* Continue traversal iff userspace said ok... */ if (verdict == NF_REPEAT) - verdict = elem->hook(elem->priv, skb, &entry->state); + verdict = nf_hook_entry_hookfn(hook_entry, skb, &entry->state); if (verdict == NF_ACCEPT) { afinfo = nf_get_afinfo(entry->state.pf); -- cgit v1.3-7-g2ca7 From d415b9eb76fc55c03ef5451691170aa5771dcea3 Mon Sep 17 00:00:00 2001 From: Aaron Conole Date: Tue, 15 Nov 2016 17:48:45 -0500 Subject: netfilter: decouple nf_hook_entry and nf_hook_ops During nfhook traversal we only need a very small subset of nf_hook_ops members. We need: - next element - hook function to call - hook function priv argument Bridge netfilter also needs 'thresh'; can be obtained via ->orig_ops. nf_hook_entry struct is now 32 bytes on x86_64. A followup patch will turn the run-time list into an array that only stores hook functions plus their priv arguments, eliminating the ->next element. Suggested-by: Florian Westphal Signed-off-by: Aaron Conole Signed-off-by: Pablo Neira Ayuso --- include/linux/netfilter.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'include/linux/netfilter.h') diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h index 575aa198097e..a4b97be30b28 100644 --- a/include/linux/netfilter.h +++ b/include/linux/netfilter.h @@ -75,7 +75,8 @@ struct nf_hook_ops { struct nf_hook_entry { struct nf_hook_entry __rcu *next; - struct nf_hook_ops ops; + nf_hookfn *hook; + void *priv; const struct nf_hook_ops *orig_ops; }; @@ -83,21 +84,22 @@ static inline void nf_hook_entry_init(struct nf_hook_entry *entry, const struct nf_hook_ops *ops) { entry->next = NULL; - entry->ops = *ops; + entry->hook = ops->hook; + entry->priv = ops->priv; entry->orig_ops = ops; } static inline int nf_hook_entry_priority(const struct nf_hook_entry *entry) { - return entry->ops.priority; + return entry->orig_ops->priority; } static inline int nf_hook_entry_hookfn(const struct nf_hook_entry *entry, struct sk_buff *skb, struct nf_hook_state *state) { - return entry->ops.hook(entry->ops.priv, skb, state); + return entry->hook(entry->priv, skb, state); } static inline const struct nf_hook_ops * -- cgit v1.3-7-g2ca7