Message ID | 20170327152850.19812-1-ap420073@gmail.com |
---|---|
State | Accepted |
Delegated to: | Pablo Neira |
Headers | show |
On Tue, Mar 28, 2017 at 12:28:50AM +0900, Taehee Yoo wrote: > __nf_nat_decode_session is called from nf_nat_decode_session as decodefn. > before calling decodefn, it already set rcu_read_lock. so rcu_read_lock in > __nf_nat_decode_session can be removed. Could you have close look at the tree to confirm if we have more spots where rcu_read_lock is unnecessary? $ git grep rcu_read_lock net/netfilter/ | wc -l 158 $ git grep rcu_read_lock net/ipv4/netfilter/ | wc -l 3 $ git grep rcu_read_lock net/ipv6/netfilter/ | wc -l 2 $ git grep rcu_read_lock net/bridge/netfilter/ | wc -l It's a fair good amount, it would take me around one hour probably to evaluate those here. If you can help us on verifying this, it would be simply great. Thanks! -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thank you for your review! 2017-04-07 4:51 GMT+09:00 Pablo Neira Ayuso <pablo@netfilter.org>: > On Tue, Mar 28, 2017 at 12:28:50AM +0900, Taehee Yoo wrote: >> __nf_nat_decode_session is called from nf_nat_decode_session as decodefn. >> before calling decodefn, it already set rcu_read_lock. so rcu_read_lock in >> __nf_nat_decode_session can be removed. > > Could you have close look at the tree to confirm if we have more spots > where rcu_read_lock is unnecessary? > > $ git grep rcu_read_lock net/netfilter/ | wc -l > 158 > $ git grep rcu_read_lock > net/ipv4/netfilter/ | wc -l > 3 > $ git grep rcu_read_lock > net/ipv6/netfilter/ | wc -l > 2 > $ git grep rcu_read_lock > net/bridge/netfilter/ | wc -l > > It's a fair good amount, it would take me around one hour probably to > evaluate those here. > > If you can help us on verifying this, it would be simply great. > > Thanks! okay, I confirm whole rcu_read_lock then I will resend a single patch as soon as possible. may be it needs a week. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 28, 2017 at 12:28:50AM +0900, Taehee Yoo wrote: > __nf_nat_decode_session is called from nf_nat_decode_session as decodefn. > before calling decodefn, it already set rcu_read_lock. so rcu_read_lock in > __nf_nat_decode_session can be removed. Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c index 94b14c5..9b68676 100644 --- a/net/netfilter/nf_nat_core.c +++ b/net/netfilter/nf_nat_core.c @@ -71,11 +71,10 @@ static void __nf_nat_decode_session(struct sk_buff *skb, struct flowi *fl) if (ct == NULL) return; - family = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.l3num; - rcu_read_lock(); + family = nf_ct_l3num(ct); l3proto = __nf_nat_l3proto_find(family); if (l3proto == NULL) - goto out; + return; dir = CTINFO2DIR(ctinfo); if (dir == IP_CT_DIR_ORIGINAL) @@ -84,8 +83,6 @@ static void __nf_nat_decode_session(struct sk_buff *skb, struct flowi *fl) statusbit = IPS_SRC_NAT; l3proto->decode_session(skb, ct, dir, statusbit, fl); -out: - rcu_read_unlock(); } int nf_xfrm_me_harder(struct net *net, struct sk_buff *skb, unsigned int family)
__nf_nat_decode_session is called from nf_nat_decode_session as decodefn. before calling decodefn, it already set rcu_read_lock. so rcu_read_lock in __nf_nat_decode_session can be removed. Signed-off-by: Taehee Yoo <ap420073@gmail.com> --- net/netfilter/nf_nat_core.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)