From: Maciej Fijalkowski <maciej.fijalkowski@xxxxxxxxx> Date: Tue, 11 Mar 2025 15:05:38 +0100 > On Wed, Mar 05, 2025 at 05:21:19PM +0100, Alexander Lobakin wrote: >> "Couple" is a bit humbly... Add the following functionality to libeth: >> >> * XDP shared queues managing >> * XDP_TX bulk sending infra >> * .ndo_xdp_xmit() infra >> * adding buffers to &xdp_buff >> * running XDP prog and managing its verdict >> * completing XDP Tx buffers >> >> Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@xxxxxxxxx> # lots of stuff >> Signed-off-by: Alexander Lobakin <aleksander.lobakin@xxxxxxxxx> > > Patch is really big and I'm not sure how to trim this TBH to make my > comments bearable. I know this is highly optimized but it's rather hard to > follow with all of the callbacks, defines/aligns and whatnot. Any chance > to chop this commit a bit? Sometimes "highly optimized" code means "not really readable". See PeterZ's code :D I mean, I'm not able to write it to look more readable without hurting object code or not provoking code duplications. Maybe it's an art which I don't possess. I tried by best and left the documentation, even with pseudo-examples. Sorry if it doesn't help =\ > > Timers and locking logic could be pulled out to separate patches I think. > You don't ever say what improvement gave you the __LIBETH_WORD_ACCESS > approach. You've put a lot of thought onto this work and I feel like this I don't record/remember all of the perf changes. Couple percent for sure. Plus lighter object code. I can recall ~ -50-60 bytes in libeth_xdp_process_buff(), even though there's only 1 64-bit write replacing 2 32-bit writes. When there's a lot, like descriptor filling, it was 100+ bytes off, esp. when unrolling. > is not explained/described thoroughly. What would be nice to see is to > have this in the separate commit as well with a comment like 'this gave me > +X% performance boost on Y workload'. That would be probably a non-zero > effort to restructure it but generally while jumping back and forth Yeah it would be quite a big. I had a bit of hard time splitting it into 2 commits (XDP and XSk) from one, that request would cost a bunch more. Dunno if it would make sense at all? Defines, alignments etc, won't go away. Same for "head-scratching moments". Moreover, sometimes splitting the code borns more questions as it feels incomplete until the last patch and then there'll be a train of replies like "this will be added/changes in patch number X", which I don't like to do :s I mean, I would like to not sacrifice time splitting it only for the sake of split, depends on how critical this is and what it would give. > through this code I had a lot of head-scratching moments. > >> --- >> drivers/net/ethernet/intel/libeth/Kconfig | 10 +- >> drivers/net/ethernet/intel/libeth/Makefile | 7 +- >> include/net/libeth/types.h | 106 +- >> drivers/net/ethernet/intel/libeth/priv.h | 26 + >> include/net/libeth/tx.h | 30 +- >> include/net/libeth/xdp.h | 1827 ++++++++++++++++++++ >> drivers/net/ethernet/intel/libeth/tx.c | 38 + >> drivers/net/ethernet/intel/libeth/xdp.c | 431 +++++ >> 8 files changed, 2467 insertions(+), 8 deletions(-) >> create mode 100644 drivers/net/ethernet/intel/libeth/priv.h >> create mode 100644 include/net/libeth/xdp.h >> create mode 100644 drivers/net/ethernet/intel/libeth/tx.c >> create mode 100644 drivers/net/ethernet/intel/libeth/xdp.c Thanks, Olek
