netdev - Re: [PATCH 2/2] net: tcp: Add noinline_for_tracing annotation for tcp_drop

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]

Message-ID: <6162222b-0a2e-4fb7-b605-c57fa8420bc9@redhat.com>
Date: Thu, 31 Oct 2024 15:23:35 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Yafang Shao <laoar.shao@...il.com>, Daniel Xu <dxu@...uu.xyz>
Cc: Eric Dumazet <edumazet@...gle.com>, David Miller <davem@...emloft.net>,
 dsahern@...nel.org, Jakub Kicinski <kuba@...nel.org>,
 netdev@...r.kernel.org, Menglong Dong <menglong8.dong@...il.com>
Subject: Re: [PATCH 2/2] net: tcp: Add noinline_for_tracing annotation for
 tcp_drop_reason()

Hi,

On 10/25/24 07:58, Yafang Shao wrote:
> On Fri, Oct 25, 2024 at 4:57 AM Daniel Xu <dxu@...uu.xyz> wrote:
>> On Thu, Oct 24, 2024, at 2:37 AM, Yafang Shao wrote:
>>> We previously hooked the tcp_drop_reason() function using BPF to monitor
>>> TCP drop reasons. However, after upgrading our compiler from GCC 9 to GCC
>>> 11, tcp_drop_reason() is now inlined, preventing us from hooking into it.
>>> To address this, it would be beneficial to make noinline explicitly for
>>> tracing.
>>
>> It looks like kfree_skb() tracepoint has rx_sk field now. Added in
>> c53795d48ee8 ("net: add rx_sk to trace_kfree_skb").
> 
> This commit is helpful. Thank you for providing the information. I
> plan to backport it to our local kernel.
> 
>>
>> Between sk and skb, is there enough information to monitor TCP drops?
>> Or do you need something particular about tcp_drop_reason()?
> 
> There's nothing else specific to mention. The @rx_sk introduced in the
> commit you referred to will be beneficial to us.

The implications of the above statement are not clear to me. Do you mean
this patchset is not needed anymore?

Thanks!

Paolo