linux-kernel - Re: [PATCH net-next v5 00/19] xdp: a fistful of generic changes (+libeth

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: <55628623-220c-4512-acdc-0b3bd38685e1@intel.com>
Date: Thu, 21 Nov 2024 19:02:35 +0100
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
CC: Jakub Kicinski <kuba@...nel.org>, "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Toke Høiland-Jørgensen <toke@...hat.com>, "Alexei
 Starovoitov" <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, "John
 Fastabend" <john.fastabend@...il.com>, Andrii Nakryiko <andrii@...nel.org>,
	Maciej Fijalkowski <maciej.fijalkowski@...el.com>, Stanislav Fomichev
	<sdf@...ichev.me>, Magnus Karlsson <magnus.karlsson@...el.com>,
	<nex.sw.ncis.osdt.itp.upstreaming@...el.com>, <bpf@...r.kernel.org>,
	<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v5 00/19] xdp: a fistful of generic changes
 (+libeth_xdp)

From: Willem De Bruijn <willemdebruijn.kernel@...il.com>
Date: Thu, 21 Nov 2024 10:43:12 -0500

> Alexander Lobakin wrote:
>> From: Willem De Bruijn <willemdebruijn.kernel@...il.com>
>> Date: Tue, 19 Nov 2024 10:14:28 -0500
>>
>>> Alexander Lobakin wrote:
>>>> From: Willem De Bruijn <willemdebruijn.kernel@...il.com>
>>>> Date: Sat, 16 Nov 2024 10:31:08 -0500
>>
>> [...]
>>
>>>> libeth_xdp depends on every patch from the series. I don't know why you
>>>> believe this might anyhow move faster. Almost the whole series got
>>>> reviewed relatively quickly, except drivers/intel folder which people
>>>> often tend to avoid.
>>>
>>> Smaller focused series might have been merged already.
>>
>> Half of this series merged wouldn't change that the whole set wouldn't
>> fit into one window (which is what you want). Half of this series merged
>> wouldn't allow sending idpf XDP parts.
> 
> I meant that smaller series are easier to facilitate feedback and
> iterate on quickly. So multiple focused series can make the same
> window.

You get reviews on more patches with bigger series. I'm not saying 19 is
fine, but I don't see any way how this series split into two could get
reviewed and accepted in full if the whole series didn't do that.
And please don't say that the delays between different revisions were
too long. I don't remember Mina sending devmem every single day. I
already hit the top negative review:series ratio score this window while
I was reviewing stuff when I had time.
Chapter II also got delayed a bit due to that most of the maintainers
were on vacations and I was helping with the reviews back then as well.
It's not only about code when it comes to upstream, it's not just you
and me being here.

[...]

>> I clearly remember Kuba's position that he wants good quality of
>> networking core and driver code. I'm pretty sure every netdev maintainer
>> has the same position. Again, feel free to argue with them, saying they
>> must take whatever trash is sent to LKML because customer X wants it
>> backported to his custom kernel Y ASAP.
> 
> Not asking for massive changes, just suggesting a different patch
> order. That does not affect code quality.
> 
> The core feature set does not depend on loop unrolling, constification,

I need to remind once again that you have mail from me somewhere
describing every patch in detail and why it's needed?
When we agreed with Kuba to drop stats off the Chapter II, it took me a
while to resolve all the dependencies, but you keep saying that wasting
time on downgrading code is better and faster than upstreaming what was
already done and works good.

> removal of xdp_frame::mem.id, etcetera. These can probably be reviewed

You see already that I even receive additional requests (Amit).
Sometimes generic (and not only) changes cause chain reaction and you
can't say to people "let me handle this later", because there's once
again no "later" here.
idpf still has 3 big lists of todos/followups to be done since it was
upstreamed and I haven't seen any activity here (not my team
responsibility), so I don't believe in "laters".

> and merged more quickly independent from this driver change, even.
> 
> Within IDPF, same for for per queue(set) link up/down and chunked
> virtchannel messages. A deeper analysis can probably carve out
> other changes not critical to landing XDP/XSK (sw marker removal).

You also said once that XDP Rx Hints can be implemented in 3 lines,
while it took couple hundred and several revisions for Larysa to
implement it in ice.

Just BTW, even if Chapter 3 + 4 + 5 is taken literally today, XDP
doesn't work on C0 board revisions at all because the FW is broken and I
also doesn't have any activity in fixing this for half a year already.

> 
>>>
>>>>> some of the changes in 1..18, that makes it more robust from that PoV.
>>>>
>>>> No it can't, I thought people first read the code and only then comment,
>>>> otherwise it's just wasting time.

Thanks,
Olek