Re: [PATCH nf-next v2 2/3] netfilter: call nf_hook_state_init with rcu_read_lock held
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: 2016-07-14 17:19:24
Also in:
netfilter-devel
On Tue, Jul 12, 2016 at 11:32:20AM -0400, Aaron Conole wrote:
From: Florian Westphal <fw@strlen.de> This makes things simpler because we can store the head of the list in the nf_state structure without worrying about concurrent add/delete of hook elements from the list.
This is something that you need for your follow up patch, right? Then it would be good to document this here. More comments below.
quoted hunk ↗ jump to hunk
diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h index 9230f9a..ad444f0 100644 --- a/include/linux/netfilter.h +++ b/include/linux/netfilter.h@@ -174,10 +174,16 @@ static inline int nf_hook_thresh(u_int8_t pf, unsigned int hook, if (!list_empty(hook_list)) { struct nf_hook_state state; + int ret; + /* We may already have this, but read-locks nest anyway */ + rcu_read_lock(); nf_hook_state_init(&state, hook_list, hook, thresh, pf, indev, outdev, sk, net, okfn); - return nf_hook_slow(skb, &state); + + ret = nf_hook_slow(skb, &state); + rcu_read_unlock(); + return ret; } return 1; }diff --git a/include/linux/netfilter_ingress.h b/include/linux/netfilter_ingress.h index 5fcd375..6965ba0 100644 --- a/include/linux/netfilter_ingress.h +++ b/include/linux/netfilter_ingress.h@@ -14,6 +14,7 @@ static inline bool nf_hook_ingress_active(const struct sk_buff *skb) return !list_empty(&skb->dev->nf_hooks_ingress); } +/* caller must hold rcu_read_lock */ static inline int nf_hook_ingress(struct sk_buff *skb) { struct nf_hook_state state;diff --git a/net/bridge/netfilter/ebt_redirect.c b/net/bridge/netfilter/ebt_redirect.c index 20396499..2e7c4f9 100644 --- a/net/bridge/netfilter/ebt_redirect.c +++ b/net/bridge/netfilter/ebt_redirect.c@@ -24,7 +24,7 @@ ebt_redirect_tg(struct sk_buff *skb, const struct xt_action_param *par) return EBT_DROP; if (par->hooknum != NF_BR_BROUTING) - /* rcu_read_lock()ed by nf_hook_slow */ + /* rcu_read_lock()ed by nf_hook_thresh */
Why are all these comments being renamed in this patch? This patch description doesn't say anything about this.