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.

[ ... ]

-	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;
-}
-

[ ... ]

[ ... ]

+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