netdev - Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)

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: <20190829115004.2701b217@cakuba.netronome.com>
Date:   Thu, 29 Aug 2019 11:50:04 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
Cc:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Julia Kartseva <hex@...com>, ast@...nel.org,
        Thomas Gleixner <tglx@...utronix.de>, rdna@...com,
        bpf@...r.kernel.org, daniel@...earbox.net, netdev@...r.kernel.org,
        kernel-team@...com
Subject: Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf:
 add libbpf_prog_type_(from|to)_str helpers

On Thu, 29 Aug 2019 15:10:39 -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Aug 29, 2019 at 10:16:56AM -0700, Alexei Starovoitov escreveu:
> > On Thu, Aug 29, 2019 at 08:51:51AM +0200, Greg Kroah-Hartman wrote:  
> > > That being said, from a "are you keeping the correct authorship info",
> > > yes, it sounds like you are doing the correct thing here.  
> 
> > > Look at what I do for stable kernels, I take the original commit and add
> > > it to "another tree" keeping the original author and s-o-b chain intact,
> > > and adding a "this is the original git commit id" type message to the
> > > changelog text so that people can link it back to the original.  
>  
> > I think you're describing 'git cherry-pick -x'.
> > The question was about taking pieces of the original commit. Not the whole commit.
> > Author field obviously stays, but SOB is questionable.
> > If author meant to change X and Y and Z. Silently taking only Z chunk of the diff
> > doesn't quite seem right.
> > If we document that such commit split happens in Documentation/bpf/bpf_devel_QA.rst
> > do you think it will be enough to properly inform developers?  
> 
> Can't we instead establish the rule that for something to be added to
> tools/include/ it should first land in a separate commit in include/,
> ditto for the other things tools/ copies from the kernel sources.

In practice in for BPF work the tools/include/ patch is always part of
the same patch set, since the patch sets usually include libbpf support,
tests that need libbpf etc.
 
> That was the initial intention of tools/include/ and also that is how
> tools/perf/check-headers.h works, warning when something ot out of sync,
> etc.
> 
> I.e. the tools/ maintainers should refuse patches that touch both
> tools/include and tools/.
> 
> wdyt?

It's not only about include/. The series that sparked this query is
moving code from tools/bpf/ to tools/lib/bpf/. And each move is split
into two commits add and delete. That's utterly pointless and a waste
of reviewers' time.

But the question is larger still. As I said vendors maintain
out-of-tree version of their drivers, by necessity, e.g.:

https://github.com/Netronome/nfp-drv-kmods is a #ifdef'd version of 
driver/net/ethernet/netronome/nfp.

If there is a problem of loosing SOB when we only apply a part of a
commit, e.g.

https://github.com/Netronome/nfp-drv-kmods/commit/79941cccea4a7720539e35a72c3ba789e4d4bf8c

which is part of:

ef01adae0e43 ("net: sched: use major priority number as hardware priority")

upstream - then we really need a clear ruling here.