linux-kernel - Re: [v6,net-next,06/15] net: fec: add fec_enet_rx_queue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260205052837.2449389-1-kuba@kernel.org>
Date: Wed,  4 Feb 2026 21:28:37 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: wei.fang@....com
Cc: Jakub Kicinski <kuba@...nel.org>,
	ast@...nel.org,
	andrew+netdev@...n.ch,
	davem@...emloft.net,
	pabeni@...hat.com,
	xiaoning.wang@....com,
	john.fastabend@...il.com,
	daniel@...earbox.net,
	sdf@...ichev.me,
	frank.li@....com,
	linux-kernel@...r.kernel.org,
	hawk@...nel.org,
	horms@...nel.org,
	bpf@...r.kernel.org,
	edumazet@...gle.com,
	netdev@...r.kernel.org,
	imx@...ts.linux.dev,
	shenwei.wang@....com
Subject: Re: [v6,net-next,06/15] net: fec: add fec_enet_rx_queue_xdp() for XDP path

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 fec_enet_rx_queue()
into a new function fec_enet_rx_queue_xdp() to optimize XDP performance.
Performance testing on iMX93 shows improvements across all xdp-bench tests.

> 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
> @@ -1666,71 +1666,6 @@ static int fec_enet_update_cbd(struct fec_enet_priv_rx_q *rxq,
>  	return 0;
>  }
>
> -static u32
> -fec_enet_run_xdp(struct fec_enet_private *fep, struct bpf_prog *prog,
> -		 struct xdp_buff *xdp, struct fec_enet_priv_rx_q *rxq, int cpu)
> -{

[ ... ]

> -	default:
> -		bpf_warn_invalid_xdp_action(fep->netdev, prog, act);
> -		fallthrough;
> -
> -	case XDP_ABORTED:
> -		fallthrough;    /* handle aborts by dropping packet */
> -
> -	case XDP_DROP:
> -		rxq->stats[RX_XDP_DROP]++;
> -xdp_err:
> -		ret = FEC_ENET_XDP_CONSUMED;
> -		page = virt_to_head_page(xdp->data);
> -		page_pool_put_page(rxq->page_pool, page, sync, true);
> -		if (act != XDP_DROP)
> -			trace_xdp_exception(fep->netdev, prog, act);
> -		break;
> -	}
> -
> -	return ret;
> -}
> -

[ ... ]

> @@ -1962,7 +1879,167 @@

[ ... ]

> +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)
> +				trace_xdp_exception(ndev, prog, XDP_PASS);
> +			else
> +				napi_gro_receive(&fep->napi, skb);
> +
> +			break;
> +		case XDP_REDIRECT:
> +			rxq->stats[RX_XDP_REDIRECT]++;
> +			err = xdp_do_redirect(ndev, &xdp, prog);
> +			if (unlikely(err)) {
> +				fec_xdp_drop(rxq, &xdp, sync);
> +				trace_xdp_exception(ndev, prog, XDP_REDIRECT);
> +			} else {
> +				xdp_res |= FEC_ENET_XDP_REDIR;
> +			}
> +			break;
> +		case XDP_TX:
> +			rxq->stats[RX_XDP_TX]++;
> +			err = fec_enet_xdp_tx_xmit(fep, cpu, &xdp, sync);
> +			if (unlikely(err)) {
> +				rxq->stats[RX_XDP_TX_ERRORS]++;
> +				fec_xdp_drop(rxq, &xdp, sync);
> +				trace_xdp_exception(ndev, prog, XDP_TX);
> +			}
> +			break;
> +		default:
> +			bpf_warn_invalid_xdp_action(ndev, prog, act);
> +			fallthrough;
> +		case XDP_ABORTED:
> +			/* handle aborts by dropping packet */
> +			fallthrough;
> +		case XDP_DROP:
> +			rxq->stats[RX_XDP_DROP]++;
> +			fec_xdp_drop(rxq, &xdp, sync);
> +			break;

Are XDP_ABORTED and invalid XDP actions being traced properly? In the
deleted fec_enet_run_xdp() function, the error path included this check:

    if (act != XDP_DROP)
        trace_xdp_exception(fep->netdev, prog, act);

This would trace both XDP_ABORTED and invalid actions (after calling
bpf_warn_invalid_xdp_action). The new code in fec_enet_rx_queue_xdp()
calls trace_xdp_exception() for XDP_PASS failures, XDP_REDIRECT failures,
and XDP_TX failures, but not for XDP_ABORTED or invalid actions.

This trace information is useful for debugging XDP program issues and
monitoring abnormal XDP behavior.

> +		}
> +
> +rx_processing_done:

[ ... ]

> +	return pkt_received;
> +}
-- 
pw-bot: cr