Re: [RFC] xdp: pass flags to xdp_update_skb_shared_info() directly

[Alexander Lobakin <aleksander.lobakin@intel.com>]

From: Jakub Kicinski <kuba@kernel.org>
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@kernel.org>
> ---
> 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@kernel.org
> CC: daniel@iogearbox.net
> CC: hawk@kernel.org
> CC: lorenzo@kernel.org
> CC: toke@redhat.com
> CC: john.fastabend@gmail.com
> CC: sdf@fomichev.me
> CC: michael.chan@broadcom.com
> CC: anthony.l.nguyen@intel.com
> CC: przemyslaw.kitszel@intel.com
> CC: marcin.s.wojtas@gmail.com
> CC: tariqt@nvidia.com
> CC: mbloch@nvidia.com
> CC: eperezma@redhat.com
> CC: bpf@vger.kernel.org
> ---
>  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