On Fri, 06 Jan 2017 14:08:06 -0800 Eric Dumazet <eric.duma...@gmail.com> wrote:
> On Fri, 2017-01-06 at 11:40 -0800, Eric Dumazet wrote: > > On Fri, 2017-01-06 at 18:39 +0100, Jesper Dangaard Brouer wrote: > > > > > > > @@ -648,13 +668,17 @@ void icmp_send(struct sk_buff *skb_in, int type, > > > int code, __be32 info) > > > } > > > } > > > > > > - icmp_param = kmalloc(sizeof(*icmp_param), GFP_ATOMIC); > > > - if (!icmp_param) > > > - return; > > > - > > > sk = icmp_xmit_lock(net); > > > if (!sk) > > > - goto out_free; > > > + goto out; > > > + > > > + /* Check global sysctl_icmp_msgs_per_sec ratelimit */ > > > + if (!icmpv4_global_allow(net, type, code)) > > > + goto out_unlock; > > > + > > > + icmp_param = kmalloc(sizeof(*icmp_param), GFP_ATOMIC); > > > + if (!icmp_param) > > > + goto out_unlock; > > > > You could call icmp_xmit_lock() _after_ checking global limit perhaps. > > That would remove one atomic op. The reason I don't do this is, because icmp_xmit_lock() disables BH and icmp_global_allow() have a comment that it need to run with BH-disabled. Thus, I'm depending on the BH-disable from icmp_xmit_lock(). I would have to move out the BH-disable part of icmp_xmit_lock, to do this. Would that be acceptable? -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
