netdev - Re: [Patch net-next 02/13] ipv4: introduce tracepoint trace_ip_queue

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] [day] [month] [year] [list]

Date:   Wed, 11 Aug 2021 22:46:54 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     Cong Wang <xiyou.wangcong@...il.com>
CC:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Qitao Xu <qitao.xu@...edance.com>,
        Cong Wang <cong.wang@...edance.com>, bpf <bpf@...r.kernel.org>,
        Eric Dumazet <eric.dumazet@...il.com>
Subject: Re: [Patch net-next 02/13] ipv4: introduce tracepoint
 trace_ip_queue_xmit()

On Wed, Aug 11, 2021 at 05:37:35PM -0700, Cong Wang wrote:
> On Wed, Aug 11, 2021 at 4:08 PM Martin KaFai Lau <kafai@...com> wrote:
> > Some of the function names are hardly changed.
> 
> This is obviously wrong for two reasons:
> 
> 1. Kernel developers did change them. As a quick example,
> tcp_retransmit_skb() has been changed, we do have reasons to only trace
> __tcp_retransmit_skb() instead.
I did not say it has never changed.  how often?  I don't believe
it changed often enough.

> 2. Even if kernel developers never did, compilers can do inline too. For
> example, I see nothing to stop compiler to inline tcp_transmit_skb()
> which is static and only calls __tcp_transmit_skb(). You explicitly
> mark bpf_fentry_test1() as noinline, don't you?
Yes, correct, inline is a legit reason.

Another one is to track some local variables in the function stack.

However, how many functions that you need here are indeed inlined by
compiler or need to track the local variables?

> I understand you are eager to promote ebpf, however, please keep
> reasonable on facts.
Absolutely not.  There is no need.  There is enough scripts
using bpf that does network tracing doing useful stuff without
adding so many tracepoints in the fast path.

I am only exploring other options and trying to understand
why it does not work in your case before adding all of
them in the fast path.  It is sad that you take it
this way.