Re: [PATCH RFC 1/4] net: skb: use line number to trace dropped skb

[Eric Dumazet]

On Thu, Feb 3, 2022 at 7:38 AM Dongli Zhang <dongli.zhang@xxxxxxxxxx> wrote:
>
> Sometimes the kernel may not directly call kfree_skb() to drop the sk_buff.
> Instead, it "goto drop" and call kfree_skb() at 'drop'. This make it
> difficult to track the reason that the sk_buff is dropped.
>
> The commit c504e5c2f964 ("net: skb: introduce kfree_skb_reason()") has
> introduced the kfree_skb_reason() to help track the reason. However, we may
> need to define many reasons for each driver/subsystem.
>
> To avoid introducing so many new reasons, this is to use line number
> ("__LINE__") to trace where the sk_buff is dropped. As a result, the reason
> will be generated automatically.
>
> Cc: Joao Martins <joao.m.martins@xxxxxxxxxx>
> Cc: Joe Jin <joe.jin@xxxxxxxxxx>
> Signed-off-by: Dongli Zhang <dongli.zhang@xxxxxxxxxx>
> ---
>  include/linux/skbuff.h     | 21 ++++-----------------
>  include/trace/events/skb.h | 35 ++++++-----------------------------
>  net/core/dev.c             |  2 +-
>  net/core/skbuff.c          |  9 ++++-----
>  net/ipv4/tcp_ipv4.c        | 14 +++++++-------
>  net/ipv4/udp.c             | 14 +++++++-------
>  6 files changed, 29 insertions(+), 66 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 8a636e678902..471268a4a497 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -307,21 +307,8 @@ struct sk_buff_head {
>
>  struct sk_buff;
>
> -/* The reason of skb drop, which is used in kfree_skb_reason().
> - * en...maybe they should be splited by group?
> - *
> - * Each item here should also be in 'TRACE_SKB_DROP_REASON', which is
> - * used to translate the reason to string.
> - */
> -enum skb_drop_reason {
> -       SKB_DROP_REASON_NOT_SPECIFIED,
> -       SKB_DROP_REASON_NO_SOCKET,
> -       SKB_DROP_REASON_PKT_TOO_SMALL,
> -       SKB_DROP_REASON_TCP_CSUM,
> -       SKB_DROP_REASON_SOCKET_FILTER,
> -       SKB_DROP_REASON_UDP_CSUM,
> -       SKB_DROP_REASON_MAX,
> -};


Seriously, we have to stop messing with things like that.

Your patch comes too late, another approach has been taken.

Please continue this effort by providing patches that improve things,
instead of throwing away effort already done.

I say no to this patch.