BPF List
 help / color / mirror / Atom feed
From: Alexander Lobakin <aleksander.lobakin@intel.com>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: "Jakub Kicinski" <kuba@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Maciej Fijalkowski" <maciej.fijalkowski@intel.com>,
	"Stanislav Fomichev" <sdf@fomichev.me>,
	"Magnus Karlsson" <magnus.karlsson@intel.com>,
	nex.sw.ncis.osdt.itp.upstreaming@intel.com, bpf@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v5 00/19] xdp: a fistful of generic changes (+libeth_xdp)
Date: Thu, 21 Nov 2024 19:02:35 +0100	[thread overview]
Message-ID: <55628623-220c-4512-acdc-0b3bd38685e1@intel.com> (raw)
In-Reply-To: <673f55109d49_bb6d229431@willemb.c.googlers.com.notmuch>

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

> Alexander Lobakin wrote:
>> From: Willem De Bruijn <willemdebruijn.kernel@gmail.com>
>> Date: Tue, 19 Nov 2024 10:14:28 -0500
>>
>>> Alexander Lobakin wrote:
>>>> From: Willem De Bruijn <willemdebruijn.kernel@gmail.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

  reply	other threads:[~2024-11-21 18:03 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-13 15:24 [PATCH net-next v5 00/19] xdp: a fistful of generic changes (+libeth_xdp) Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 01/19] jump_label: export static_key_slow_{inc,dec}_cpuslocked() Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 02/19] skbuff: allow 2-4-argument skb_frag_dma_map() Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 03/19] unroll: add generic loop unroll helpers Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 04/19] bpf, xdp: constify some bpf_prog * function arguments Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 05/19] xdp, xsk: constify read-only arguments of some static inline helpers Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 06/19] xdp: allow attaching already registered memory model to xdp_rxq_info Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 07/19] xdp: register system page pool as an XDP memory model Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 08/19] page_pool: make page_pool_put_page_bulk() actually handle array of pages Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 09/19] page_pool: allow mixing PPs within one bulk Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 10/19] xdp: get rid of xdp_frame::mem.id Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 11/19] xdp: add generic xdp_buff_add_frag() Alexander Lobakin
2024-11-14 14:07   ` Ido Schimmel
2024-11-16  2:40   ` Jakub Kicinski
2024-11-19 12:03     ` Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 12/19] xdp: add generic xdp_build_skb_from_buff() Alexander Lobakin
2024-11-14 15:06   ` Ido Schimmel
2024-11-14 15:16     ` Ido Schimmel
2024-11-15 14:34       ` Alexander Lobakin
2024-11-17 12:42         ` Amit Cohen
2024-11-19 12:05           ` Alexander Lobakin
2024-11-26 16:38           ` Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 13/19] xsk: align &xdp_buff_xsk harder Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 14/19] xsk: allow attaching XSk pool via xdp_rxq_info_reg_mem_model() Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 15/19] xsk: make xsk_buff_add_frag really add a frag via __xdp_buff_add_frag() Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 16/19] xsk: add generic XSk &xdp_buff -> skb conversion Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 17/19] xsk: add helper to get &xdp_desc's DMA and meta pointer in one go Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 18/19] libeth: support native XDP and register memory model Alexander Lobakin
2024-11-13 15:24 ` [PATCH net-next v5 19/19] libeth: add a couple of XDP helpers (libeth_xdp) Alexander Lobakin
2024-11-16  2:43 ` [PATCH net-next v5 00/19] xdp: a fistful of generic changes (+libeth_xdp) Jakub Kicinski
2024-11-16 15:31   ` Willem de Bruijn
2024-11-19 12:28     ` Alexander Lobakin
2024-11-19 15:14       ` Willem de Bruijn
2024-11-20 15:23         ` Alexander Lobakin
2024-11-21 15:43           ` Willem de Bruijn
2024-11-21 18:02             ` Alexander Lobakin [this message]
2024-11-21 18:42               ` Willem de Bruijn
2024-11-19 12:06   ` Alexander Lobakin
2024-11-19 14:25     ` Jakub Kicinski
2024-11-20 14:40       ` Alexander Lobakin
2024-11-21 19:26         ` Jacob Keller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55628623-220c-4512-acdc-0b3bd38685e1@intel.com \
    --to=aleksander.lobakin@intel.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maciej.fijalkowski@intel.com \
    --cc=magnus.karlsson@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=nex.sw.ncis.osdt.itp.upstreaming@intel.com \
    --cc=pabeni@redhat.com \
    --cc=sdf@fomichev.me \
    --cc=toke@redhat.com \
    --cc=willemdebruijn.kernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox