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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 3 Feb 2022 08:48:17 -0700
From:   David Ahern <dsahern@...il.com>
To:     Dongli Zhang <dongli.zhang@...cle.com>, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, rostedt@...dmis.org,
        mingo@...hat.com, davem@...emloft.net, kuba@...nel.org,
        edumazet@...gle.com, yoshfuji@...ux-ipv6.org, dsahern@...nel.org,
        ast@...nel.org, daniel@...earbox.net, andrii@...nel.org,
        imagedong@...cent.com, joao.m.martins@...cle.com,
        joe.jin@...cle.com
Subject: Re: [PATCH RFC 1/4] net: skb: use line number to trace dropped skb

On 2/3/22 8:37 AM, Dongli Zhang 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.
> 

I don't agree with this approach. It is only marginally better than the
old kfree_skb that only gave the instruction pointer. That tells you the
function that dropped the packet, but not why the packet is dropped.
Adding the line number only makes users have to consult the source code.

When I watch drop monitor for kfree_skb I want to know *why* the packet
was dropped, not the line number in the source code. e.g., dropmon
showing OTHERHOST means too many packets are sent to this host (e.g.,
hypervisor) that do not belong to the host or the VMs running on it, or
packets have invalid checksum (IP, TCP, UDP). Usable information by
everyone, not just someone with access to the source code for that
specific kernel.