diff options
| author | Jesper Dangaard Brouer <brouer@redhat.com> | 2017-01-09 16:04:14 +0100 |
|---|---|---|
| committer | David S. Miller <davem@davemloft.net> | 2017-01-09 21:49:12 +0100 |
| commit | 7ba91ecb16824f74ba4fcbc4e88cd4d24a839b25 (patch) | |
| tree | 23bd866126b5115cf4a14541f7700117a38efc29 /net/ipv4/icmp.c | |
| parent | net: reduce cycles spend on ICMP replies that gets rate limited (diff) | |
| download | linux-7ba91ecb16824f74ba4fcbc4e88cd4d24a839b25.tar.xz linux-7ba91ecb16824f74ba4fcbc4e88cd4d24a839b25.zip | |
net: for rate-limited ICMP replies save one atomic operation
It is possible to avoid the atomic operation in icmp{v6,}_xmit_lock,
by checking the sysctl_icmp_msgs_per_sec ratelimit before these calls,
as pointed out by Eric Dumazet, but the BH disabled state must be correct.
The icmp_global_allow() call states it must be called with BH
disabled. This protection was given by the calls icmp_xmit_lock and
icmpv6_xmit_lock. Thus, split out local_bh_disable/enable from these
functions and maintain it explicitly at callers.
Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/ipv4/icmp.c')
| -rw-r--r-- | net/ipv4/icmp.c | 34 |
1 files changed, 21 insertions, 13 deletions
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index 58d75ca58b83..fc310db2708b 100644 --- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c @@ -209,19 +209,17 @@ static struct sock *icmp_sk(struct net *net) return *this_cpu_ptr(net->ipv4.icmp_sk); } +/* Called with BH disabled */ static inline struct sock *icmp_xmit_lock(struct net *net) { struct sock *sk; - local_bh_disable(); - sk = icmp_sk(net); if (unlikely(!spin_trylock(&sk->sk_lock.slock))) { /* This can happen if the output path signals a * dst_link_failure() for an outgoing ICMP packet. */ - local_bh_enable(); return NULL; } return sk; @@ -229,7 +227,7 @@ static inline struct sock *icmp_xmit_lock(struct net *net) static inline void icmp_xmit_unlock(struct sock *sk) { - spin_unlock_bh(&sk->sk_lock.slock); + spin_unlock(&sk->sk_lock.slock); } int sysctl_icmp_msgs_per_sec __read_mostly = 1000; @@ -417,14 +415,17 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb) if (ip_options_echo(&icmp_param->replyopts.opt.opt, skb)) return; - sk = icmp_xmit_lock(net); - if (!sk) - return; - inet = inet_sk(sk); + /* Needed by both icmp_global_allow and icmp_xmit_lock */ + local_bh_disable(); /* global icmp_msgs_per_sec */ if (!icmpv4_global_allow(net, type, code)) - goto out_unlock; + goto out_bh_enable; + + sk = icmp_xmit_lock(net); + if (!sk) + goto out_bh_enable; + inet = inet_sk(sk); icmp_param->data.icmph.checksum = 0; @@ -459,6 +460,8 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb) ip_rt_put(rt); out_unlock: icmp_xmit_unlock(sk); +out_bh_enable: + local_bh_enable(); } #ifdef CONFIG_IP_ROUTE_MULTIPATH @@ -668,13 +671,16 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info) } } - sk = icmp_xmit_lock(net); - if (!sk) - goto out; + /* Needed by both icmp_global_allow and icmp_xmit_lock */ + local_bh_disable(); /* Check global sysctl_icmp_msgs_per_sec ratelimit */ if (!icmpv4_global_allow(net, type, code)) - goto out_unlock; + goto out_bh_enable; + + sk = icmp_xmit_lock(net); + if (!sk) + goto out_bh_enable; /* * Construct source address and options. @@ -750,6 +756,8 @@ ende: ip_rt_put(rt); out_unlock: icmp_xmit_unlock(sk); +out_bh_enable: + local_bh_enable(); out:; } EXPORT_SYMBOL(icmp_send); |