Re: [v5,net-next,06/15] net: fec: add fec_enet_rx_queue_xdp() for XDP path

[Simon Horman <horms@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: fec: add fec_enet_rx_queue_xdp() for XDP path

This commit separates XDP path packet processing from protocol stack
processing by creating a dedicated fec_enet_rx_queue_xdp() function.
The goal is to optimize XDP performance by removing unnecessary code
paths that were interleaved with regular packet processing.

> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c

[ ... ]

> +static void fec_xdp_drop(struct fec_enet_priv_rx_q *rxq,
> +			 struct xdp_buff *xdp, u32 sync)
> +{
> +	struct page *page = virt_to_head_page(xdp->data);
> +
> +	page_pool_put_page(rxq->page_pool, page, sync, true);
> +}

[ ... ]

> +static int fec_enet_rx_queue_xdp(struct fec_enet_private *fep, int queue,
> +				 int budget, struct bpf_prog *prog)
> +{

[ ... ]

> +		switch (act) {
> +		case XDP_PASS:
> +			rxq->stats[RX_XDP_PASS]++;
> +			/* The packet length includes FCS, but we don't want to
> +			 * include that when passing upstream as it messes up
> +			 * bridging applications.
> +			 */
> +			skb = fec_build_skb(fep, rxq, bdp, page, pkt_len);
> +			if (!skb) {
> +				fec_xdp_drop(rxq, &xdp, sync);
> +				trace_xdp_exception(ndev, prog, XDP_PASS);
> +			} else {
> +				napi_gro_receive(&fep->napi, skb);
> +			}
> +			break;

When fec_build_skb() fails, it already returns the page to the page pool
via page_pool_recycle_direct():

    static struct sk_buff *fec_build_skb(...)
    {
        skb = build_skb(page_address(page), ...);
        if (unlikely(!skb)) {
            page_pool_recycle_direct(rxq->page_pool, page);
            ...
            return NULL;
        }
        ...
    }

Calling fec_xdp_drop() after fec_build_skb() returns NULL will return
the same page to the pool a second time. Can this cause page pool
corruption under memory pressure when build_skb() fails?

[ ... ]