netdev - Re: [PATCH v2 bpf-next] bpf, tracing: unbreak lttng

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20180327153523.0a2c5d33@gandalf.local.home>
Date:   Tue, 27 Mar 2018 15:35:23 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Alexei Starovoitov <ast@...nel.org>, davem@...emloft.net,
        torvalds@...ux-foundation.org, peterz@...radead.org,
        mathieu.desnoyers@...icios.com, netdev@...r.kernel.org,
        kernel-team@...com, linux-api@...r.kernel.org
Subject: Re: [PATCH v2 bpf-next] bpf, tracing: unbreak lttng

On Tue, 27 Mar 2018 11:39:19 +0200
Daniel Borkmann <daniel@...earbox.net> wrote:

> On 03/27/2018 02:07 AM, Steven Rostedt wrote:
> > On Mon, 26 Mar 2018 16:02:20 -0700
> > Alexei Starovoitov <ast@...nel.org> wrote:
> >   
> >> for_each_kernel_tracepoint() is used by out-of-tree lttng module
> >> and therefore cannot be changed.  
> > 
> > This is false and misleading. NACK.  
> 
> Steven, while I really don't care about this particular function, you wrote
> a few emails ago, quote:
> 
>   Look, the tracepoint code was written by Mathieu for LTTng, and perf
>   and ftrace were able to benefit because of it, as well as your bpf
>   code. For this, we agreed to keep this function around for his use,
>   as its the only thing he requires. Everyone has been fine with that.
>   [...] Having that function for LTTng does not hurt us. And I will NACK
>   removing it.
> 
> So later saying that "this is false and misleading" is kind of misleading
> by itself. ;-) Anyway, it would probably make sense to add a comment to
> for_each_kernel_tracepoint() that this is used by LTTng so that people
> don't accidentally remove it due to missing in-tree user. I would think
> some sort of clarification/background in a comment or such would have
> avoided the whole confusion and resulting discussion around this in the
> first place.

I agree, a comment should be added. But the function can still be
modified, which is what I meant here by being misleading.

> 
> Btw, in networking land, as soon as there is no in-tree user for a particular
> kernel function, it will get ripped out, no matter what. Given this is also
> the typical convention in the kernel, it may have caused some confusion with
> above preference.

This is usually the case with me too. This came from Mathieu doing a
lot of work to help perf and ftrace, but keep this for him to maintain
LTTng. This was the solution to a long drawn out flame war. Yes,
there's a lot of history behind that function, and I agree that we
should comment the history behind it.

> 
> Anyway, given v6 is out now, I've tossed the old series from bpf-next tree.
> So I hope we can all move on with some more constructive discussion. :-)

Yes, which we are doing around the kallsyms part. ;-)

-- Steve