Messages in this thread | | | Date | Wed, 16 Mar 2022 20:18:53 -0700 | From | Jakub Kicinski <> | Subject | Re: [PATCH net-next v3 3/3] net: icmp: add reasons of the skb drops to icmp protocol |
| |
On Wed, 16 Mar 2022 14:31:48 +0800 menglong8.dong@gmail.com wrote: > From: Menglong Dong <imagedong@tencent.com> > > Replace kfree_skb() used in icmp_rcv() and icmpv6_rcv() with > kfree_skb_reason(). > > In order to get the reasons of the skb drops after icmp message handle, > we change the return type of 'handler()' in 'struct icmp_control' from > 'bool' to 'enum skb_drop_reason'. This may change its original > intention, as 'false' means failure, but 'SKB_NOT_DROPPED_YET' means > success now. Therefore, all 'handler' and the call of them need to be > handled. Following 'handler' functions are involved: > > icmp_unreach() > icmp_redirect() > icmp_echo() > icmp_timestamp() > icmp_discard() > > And following new drop reasons are added: > > SKB_DROP_REASON_ICMP_CSUM > SKB_DROP_REASON_ICMP_TYPE > SKB_DROP_REASON_ICMP_BROADCAST > > Reviewed-by: Hao Peng <flyingpeng@tencent.com> > Reviewed-by: Jiang Biao <benbjiang@tencent.com> > Signed-off-by: Menglong Dong <imagedong@tencent.com>
I guess this set raises the follow up question to Dave if adding drop reasons to places with MIB exception stats means improving the granularity or one MIB stat == one reason?
> -bool ping_rcv(struct sk_buff *skb) > +enum skb_drop_reason ping_rcv(struct sk_buff *skb) > { > + enum skb_drop_reason reason = SKB_DROP_REASON_NO_SOCKET; > struct sock *sk; > struct net *net = dev_net(skb->dev); > struct icmphdr *icmph = icmp_hdr(skb); > - bool rc = false; > > /* We assume the packet has already been checked by icmp_rcv */ > > @@ -980,15 +980,17 @@ bool ping_rcv(struct sk_buff *skb) > struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC); > > pr_debug("rcv on socket %p\n", sk); > - if (skb2 && !ping_queue_rcv_skb(sk, skb2)) > - rc = true; > + if (skb2) > + reason = __ping_queue_rcv_skb(sk, skb2); > + else > + reason = SKB_DROP_REASON_NOMEM; > sock_put(sk); > } > > - if (!rc) > + if (reason) > pr_debug("no socket, dropping\n");
This is going to be printed on memory allocation failures now as well.
| |