From: Willem De Bruijn <willemdebruijn.kernel@xxxxxxxxx> Date: Sat, 16 Nov 2024 10:31:08 -0500 > Jakub Kicinski wrote: >> On Wed, 13 Nov 2024 16:24:23 +0100 Alexander Lobakin wrote: >>> Part III does the following: >>> * does some cleanups with marking read-only bpf_prog and xdp_buff >>> arguments const for some generic functions; >>> * allows attaching already registered XDP memory model to Rxq info; >>> * allows mixing pages from several Page Pools within one XDP frame; >>> * optimizes &xdp_frame structure and removes no-more-used field; >>> * adds generic functions to build skbs from xdp_buffs (regular and >>> XSk) and attach frags to xdp_buffs (regular and XSk); >>> * adds helper to optimize XSk xmit in drivers; >>> * extends libeth Rx to support XDP requirements (headroom etc.) on Rx; >>> * adds libeth_xdp -- libeth module with common XDP and XSk routines. >> >> This clearly could be multiple series, please don't go over the limit. > > Targeting different subsystems and thus reviewers. The XDP, page_pool > and AF_XDP changes might move faster on their own. Reviewers for page_pool, XDP and XSk (no idea why everyone name it AF_XDP) are 90% time the same people. Often times, you can't avoid cross-subsystem patches. These three are closely tied to each other. > > If pulling those out into separate series, that also allows splitting > up the last patch. That weighs in at 3481 LoC, out of 4400 for the > series. 1500 of which is kdoc if you read the cover letter. 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. I remind you that the initial libeth + iavf series (11 patches) was baking on LKML for one year. Here 2 Chapters went into the kernel within 2 windows and only this one (clearly much bigger than the previous ones and containing only generic changes in contrary to the previous which had only /intel code) didn't follow this rule, which doesn't unnecessarily mean it will stuck for too long. (+ I clearly mentioned several times that Chapter III will take longer than the rest and each time you had no issues with that) > > The first 3 patches are not essential to IDFP XDP + AF_XDP either. You don't seem to read the code. libeth_xdp won't even build without them. I don't believe the model taken by some developers (not spelling names loud) "let's submit minimal changes and almost draft code, I promise I'll create a todo list and will be polishing it within next x years" works at all, not speaking that it may work better than sending polished mature code (I hope it is). > The IDPF feature does not have to not depend on them. > > Does not matter for upstream, but for the purpose of backporting this > to distro kernels, it helps if the driver feature minimizes dependency > on core kernel API changes. If patch 19 can be made to work without OOT style of thinking. Minimizing core changes == artificial self-limiting optimization and functionality potential. New kernels > LTSes and especially custom kernels which receive non-upstream (== not officially supported by the community) feature backports. Upstream shouldn't sacrifice anything in favor of those, this way we end up one day sacrificing stuff for out-of-tree drivers (which I know some people already try to do). > 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
