lkml.org 
[lkml]   [2022]   [Mar]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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.

\
 
 \ /
  Last update: 2022-03-17 04:19    [W:0.141 / U:3.636 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site