Re: [PATCH net-next v1 11/11] eth: fbnic: Support AF_XDP page pool zero copy

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

On Tue, Nov 10, 2026 at 11:20:32AM +0100, Björn Töpel wrote:
> Header and payload rings share one provider page pool, so refills even
> them out, and a short pool does not starve one of them. Payload
> buffers start at chunk offset 0, and the UMEM headroom rules require
> headroom only in the first buffer. A chunk is one 4 KiB device page,
> also with larger kernel pages, so rings fed by the provider post one
> descriptor per chunk.
> 
> Socket TX shares the XDP TX ring and runs after RX: XDP_TX drops
> frames that do not fit, while socket descriptors can wait. The TX
> need_wakeup flag stays set, as for ice, since clearing it can miss the
> last wakeup.
> 
> Signed-off-by: Björn Töpel <bjorn@kernel.org>
> ---
>  Documentation/networking/af_xdp.rst           |  58 +++
>  .../net/ethernet/meta/fbnic/fbnic_netdev.c    |  18 +
>  drivers/net/ethernet/meta/fbnic/fbnic_txrx.c  | 428 ++++++++++++++----
>  drivers/net/ethernet/meta/fbnic/fbnic_txrx.h  |  25 +-
>  4 files changed, 446 insertions(+), 83 deletions(-)
> 
> diff --git a/Documentation/networking/af_xdp.rst b/Documentation/networking/af_xdp.rst
> index e73a0005e169..f152ae8812d1 100644
> --- a/Documentation/networking/af_xdp.rst
> +++ b/Documentation/networking/af_xdp.rst
> @@ -346,6 +346,64 @@ Note that a UMEM can be shared between sockets on the same queue id
>  and device, as well as between queues on the same device and between
>  devices at the same time.
>  
> +Zero-copy through page pool
> +---------------------------
> +
> +Some drivers, for example fbnic, receive into the UMEM through a page
> +pool memory provider instead of their own allocator. The socket API
> +does not change: zero-copy mode is selected as before, and XDP_OPTIONS
> +reports it.
> +
> +The requirements are:
> +
> +* Aligned chunks of a size the driver supports, 4 KiB for fbnic.
> +* The queue id has an RX queue, also for a socket that only transmits.
> +  Such a socket needs the FILL and COMPLETION rings, like any other.
> +* The driver accepts the headroom, MTU and XDP program. fbnic accepts
> +  a UMEM headroom of 0, 128 or 256 bytes. An MTU larger than its header
> +  buffer needs XDP_USE_SG, and frags support in an XDP program. The
> +  first buffer of a larger frame may hold only its headers, and the
> +  other buffers start at offset 0 of their chunk. Such a frame can take
> +  one chunk more than in copy mode.
> +
> +If the device cannot do zero-copy for the socket, a bind with
> +XDP_ZEROCOPY fails with EOPNOTSUPP, while a bind without it falls back
> +to copy mode. MTU and XDP program changes that a bound socket cannot
> +handle are refused.
> +
> +Binding restarts the RX queue. The queue takes chunks from the FILL
> +ring and returns them on the RX ring. A FILL entry for a chunk that
> +the RX side still owns is dropped; TX chunks stay with userspace until
> +their completion, as before. XDP_PASS copies the packet. Redirects to
> +anything but a socket copy single-buffer packets and drop others. A
> +redirect to a socket of another queue fails, and generic XDP cannot
> +redirect to the socket.
> +
> +When the queue has no chunks left to post, RX waits for the FILL ring.
> +With XDP_USE_NEED_WAKEUP, the flag on the FILL ring stays set, and
> +userspace must call poll(), or recvfrom() with an RX ring or sendto()
> +with a TX ring, after it posts entries. Without the flag, the queue
> +polls the FILL ring, which costs CPU time while the ring is empty.
> +Post FILL entries before bind() if possible.
> +
> +A driver supports this mode when it:
> +
> +* uses netmem for header buffers and XDP_TX, and handles page pools
> +  whose buffers cannot be split (see page_pool_supports_frag()),
> +* selects the provider with PP_FLAG_MEM_PROVIDER and pp_rx_role for
> +  one page pool of each queue, and honours the rx_headroom queue
> +  configuration,
> +* calls page_pool_refill_done() after each refill,
> +* sends socket TX descriptors from the NAPI of the RX queue, with
> +  xsk_tx_peek_release_desc_batch() for the pool that xsk_rxq_pool()
> +  returns,
> +* advertises NETDEV_XDP_ACT_XSK_ZEROCOPY and xdp_zc_max_segs,
> +* checks the setup in XDP_VALIDATE_XSK_POOL, and sets xsk_page_pool
> +  in its netdev_queue_mgmt_ops.
> +
> +The core schedules that NAPI for RX and TX wakeups, so no
> +ndo_xsk_wakeup is needed.
> +
>  XDP_USE_NEED_WAKEUP bind flag
>  -----------------------------
>  
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> index ae2585508690..8cb5b5b93c70 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> @@ -5,6 +5,7 @@
>  #include <linux/ipv6.h>
>  #include <linux/types.h>
>  #include <net/netdev_queues.h>
> +#include <net/xdp_sock_drv.h>
>  
>  #include "fbnic.h"
>  #include "fbnic_netdev.h"
> @@ -276,6 +277,7 @@ static int fbnic_set_mac(struct net_device *netdev, void *p)
>  static int fbnic_change_mtu(struct net_device *dev, int new_mtu)
>  {
>  	struct fbnic_net *fbn = netdev_priv(dev);
> +	int err;
>  
>  	if (fbnic_check_split_frames(fbn->xdp_prog, new_mtu, fbn->hds_thresh)) {
>  		dev_err(&dev->dev,
> @@ -285,6 +287,13 @@ static int fbnic_change_mtu(struct net_device *dev, int new_mtu)
>  		return -EINVAL;
>  	}
>  
> +	err = xsk_validate_dev(dev, new_mtu, NULL, XDP_MODE_DRV, NULL);
> +	if (err) {
> +		dev_err(&dev->dev, "MTU %d does not fit a bound AF_XDP socket\n",
> +			new_mtu);
> +		return err;
> +	}
> +
>  	WRITE_ONCE(dev->mtu, new_mtu);
>  
>  	return 0;
> @@ -538,6 +547,13 @@ static int fbnic_bpf(struct net_device *netdev, struct netdev_bpf *bpf)
>  {
>  	struct bpf_prog *prog = bpf->prog, *prev_prog;
>  	struct fbnic_net *fbn = netdev_priv(netdev);
> +	const struct xsk_rx_check *check;
> +
> +	if (bpf->command == XDP_VALIDATE_XSK_POOL) {

do you really need a separate command here? why not rely on existing
XDP_SETUP_XSK_POOL ?

here it doesn't look like things being validated are something unique that
you could not check during XDP_SETUP_XSK_POOL and XDP_SETUP_PROG.

Overall, +400 lines for zc pp-based support...not sure what I was
expecting and I know we need Tx side implementation.

However, I am wondering if better example for this change set would be the
driver that already supports pp *and* carries the separate ZC data path
(ekhm, see ice?). Then you could show case at least in diffstat how much
less work is needed for AF_XDP support.

> +		check = bpf->xsk_check.check;
> +		prog = check->prog ? : READ_ONCE(fbn->xdp_prog);
> +		return fbnic_xsk_validate(check, prog, bpf->xsk_check.extack);
> +	}
>  
>  	if (bpf->command != XDP_SETUP_PROG)
>  		return -EINVAL;
> @@ -825,6 +841,8 @@ struct net_device *fbnic_netdev_alloc(struct fbnic_dev *fbd)
>  	netdev->xdp_features = NETDEV_XDP_ACT_BASIC |
>  			       NETDEV_XDP_ACT_REDIRECT |
>  			       NETDEV_XDP_ACT_RX_SG;
> +	netdev->xdp_features |= NETDEV_XDP_ACT_XSK_ZEROCOPY;
> +	netdev->xdp_zc_max_segs = FBNIC_MAX_RX_BUFS;
>  
>  	netdev->min_mtu = IPV6_MIN_MTU;
>  	netdev->max_mtu = FBNIC_MAX_JUMBO_FRAME_SIZE - ETH_HLEN;