Re: [PATCH v3 net 5/7] net: enetc: pad short XDP frames coming from devmap
flat view
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-16 23:35:59
Also in:
bpf, imx, lkml
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 1 · Low: 0 - [Medium] In enetc_xdp_xmit() (drivers/net/ethernet/freescale/enetc/enetc.c), the newly added short-… -- net: enetc: pad short XDP frames coming from devmap This adds a generic best-effort xdp_frame_pad() helper and calls it from enetc_xdp_xmit(), together with a direct check of the head buffer length against ENETC_MIN_BUFF_SIZE, so that short XDP frames redirected into ENETC are padded or dropped instead of being transmitted.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c index bbad942041f5a..8a9ba168eab1e 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc.c +++ b/drivers/net/ethernet/freescale/enetc/enetc.c@@ -1838,15 +1838,23 @@ int enetc_xdp_xmit(struct net_device *ndev, int num_frames, prefetchw(ENETC_TXBD(*tx_ring, tx_ring->next_to_use)); for (k = 0; k < num_frames; k++) { - if (xdp_frame_has_frags(frames[k])) { - shinfo = xdp_get_shared_info_from_frame(frames[k]); + struct xdp_frame *xdpf = frames[k]; + + if (xdp_frame_has_frags(xdpf)) { + shinfo = xdp_get_shared_info_from_frame(xdpf); if (unlikely((shinfo->nr_frags + 1) > ENETC_MAX_SKB_FRAGS)) break; } + if (unlikely(xdp_frame_pad(xdpf) || + xdpf->len < ENETC_MIN_BUFF_SIZE)) { + tx_ring->stats.xdp_tx_drops++; + break; + }
[Severity: Medium] Does this count the rejected frame twice in tx_ring->stats.xdp_tx_drops? The new branch increments the counter and then breaks with k still pointing at the frame that was just rejected, and the epilogue of enetc_xdp_xmit() charges everything from k onwards again: tx_ring->stats.xdp_tx += k; tx_ring->stats.xdp_tx_drops += num_frames - k; so the counter grows by 1 + (num_frames - k) rather than num_frames - k. The other three break sites in this loop (the shinfo->nr_frags overflow check, the enetc_xdp_frame_to_xdp_tx_swbd() failure and the enetc_xdp_tx() failure) rely only on the remainder-based accounting added earlier in this series by "net: enetc: consistenly track dropped frames in enetc_xdp_xmit()", whose message states that the counter should be incremented "by the remainder, not just by 1". Would dropping the explicit increment here keep this site consistent with the other break paths? The branch is reachable when xdp_frame_pad() returns -EOPNOTSUPP (frame shorter than ETH_ZLEN and fragmented) or -ENOMEM (no tailroom before the shared info), and also when the frame total is at least ETH_ZLEN but the head buffer is shorter than ENETC_MIN_BUFF_SIZE, which is the XDP_REDIRECT-from-veth case described in the commit message. Since xdp_tx_drops is exported to userspace by enetc_get_ethtool_stats() via ethtool -S, can the reported drop count end up larger than the number of frames the core passed to ndo_xdp_xmit()?
quoted hunk ↗ jump to hunk
+ xdp_tx_bd_cnt = enetc_xdp_frame_to_xdp_tx_swbd(tx_ring, xdp_redirect_arr, - frames[k]); + xdpf); if (unlikely(xdp_tx_bd_cnt < 0)) break;
[ ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915222735.1016937-1-vladimir.oltean%40nxp.com