From: Jakub Kicinski <kuba@xxxxxxxxxx> Date: Tue, 12 Aug 2025 09:15:28 -0700 > xdp_update_skb_shared_info() needs to update skb state which > was maintained in xdp_buff / frame. Pass full flags into it, > instead of breaking it out bit by bit. We will need to add > a bit for unreadable frags (even tho XDP doesn't support > those the driver paths may be common), at which point almost > all call sites would become: > > xdp_update_skb_shared_info(skb, num_frags, > sinfo->xdp_frags_size, > MY_PAGE_SIZE * num_frags, > xdp_buff_is_frag_pfmemalloc(xdp), > xdp_buff_is_frag_unreadable(xdp)); Yeah I think this doesn't make sense, it just doesn't scale. We can make more flags in future and adding a new argument for each is not a good idea, even if more drivers would switch to generic xdp_build_skb_from_buff(). > > Keep a helper for accessing the flags, in case we need to > transform them somehow in the future (e.g. to cover up xdp_buff > vs xdp_frame differences). > > Signed-off-by: Jakub Kicinski <kuba@xxxxxxxxxx> > --- > Does anyone prefer the current form of the API, or can we change > as prosposed? > > Bonus question: while Im messing with this API could I rename > xdp_update_skb_shared_info()? Maybe to xdp_update_skb_state() ? > Not sure why the function name has "shared_info" when most of > what it updates is skb fields. I can only suspect that the author decided to name it this way due to that it's only used when xdp_buff has frags (and frags are in shinfo). But I agree it's not the best choice. xdp_update_skb_state() sounds fine to me, but given that it's all about frags, maybe something like xdp_update_skb_frags_info/state() or so? > > CC: ast@xxxxxxxxxx > CC: daniel@xxxxxxxxxxxxx > CC: hawk@xxxxxxxxxx > CC: lorenzo@xxxxxxxxxx > CC: toke@xxxxxxxxxx > CC: john.fastabend@xxxxxxxxx > CC: sdf@xxxxxxxxxxx > CC: michael.chan@xxxxxxxxxxxx > CC: anthony.l.nguyen@xxxxxxxxx > CC: przemyslaw.kitszel@xxxxxxxxx > CC: marcin.s.wojtas@xxxxxxxxx > CC: tariqt@xxxxxxxxxx > CC: mbloch@xxxxxxxxxx > CC: eperezma@xxxxxxxxxx > CC: bpf@xxxxxxxxxxxxxxx > --- > include/net/xdp.h | 21 +++++++++---------- > drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 2 +- > drivers/net/ethernet/intel/i40e/i40e_txrx.c | 4 ++-- > drivers/net/ethernet/intel/ice/ice_txrx.c | 4 ++-- > drivers/net/ethernet/marvell/mvneta.c | 2 +- > .../net/ethernet/mellanox/mlx5/core/en_rx.c | 7 +++---- > drivers/net/virtio_net.c | 2 +- > net/core/xdp.c | 11 +++++----- > 8 files changed, 26 insertions(+), 27 deletions(-) Thanks, Olek
