Re: [net-next,v2,3/6] net: bcmgenet: add basic XDP support (PASS/DROP)
From: Simon Horman <horms@kernel.org>
Date: 2026-03-17 16:19:40
Also in:
lkml
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: bcmgenet: add basic XDP support (PASS/DROP) This commit adds XDP program attachment via ndo_bpf and executes XDP programs in the RX path. XDP_PASS builds an SKB from the xdp_buff (handling xdp_adjust_head/tail), and XDP_DROP returns the page to page_pool without SKB allocation.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c index 6e610e73e12f..f6a2567af7cb 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2274,6 +2276,53 @@ static int bcmgenet_rx_refill(struct bcmgenet_rx_ring *ring, return 0; } +static struct sk_buff *bcmgenet_xdp_build_skb(struct bcmgenet_rx_ring *ring, + struct xdp_buff *xdp, + struct page *rx_page) +{ + unsigned int metasize; + struct sk_buff *skb; + + skb = napi_build_skb(xdp->data_hard_start, PAGE_SIZE); + if (unlikely(!skb)) + return NULL; + + skb_mark_for_recycle(skb); + + metasize = xdp->data - xdp->data_meta; + skb_reserve(skb, xdp->data - xdp->data_hard_start); + __skb_put(skb, xdp->data_end - xdp->data); + + if (metasize) + skb_metadata_set(skb, metasize);
^^^^ Can this corrupt memory? Looking at bcmgenet_desc_rx(), the xdp_buff is initialized with meta_valid=false: xdp_prepare_buff(&xdp, page_address(rx_page), GENET_RX_HEADROOM, pkt_len, false); When meta_valid is false, xdp_prepare_buff() sets xdp->data_meta to xdp->data + 1 as a sentinel value to indicate no metadata is present. In bcmgenet_xdp_build_skb(), the metasize calculation becomes: metasize = xdp->data - xdp->data_meta = xdp->data - (xdp->data + 1) = (ptrdiff_t)-1 When assigned to 'unsigned int metasize', this wraps to UINT_MAX. The 'if (metasize)' check passes since UINT_MAX != 0, and skb_metadata_set(skb, metasize) truncates UINT_MAX to u8, setting skb_shinfo(skb)->meta_len = 255. This incorrect meta_len of 255 can cause downstream SKB consumers (TC BPF programs, skb_data_move) to read or write 255 bytes before skb_mac_header(skb) when accessing the metadata region. Other XDP drivers that use skb_metadata_set() (bnxt, mvneta, ixgbe, i40e, virtio_net) all pass meta_valid=true to xdp_prepare_buff(), which sets data_meta=data, giving metasize=0 and correctly skipping skb_metadata_set(). Should xdp_prepare_buff() be called with meta_valid=true instead of false, since bcmgenet_xdp_build_skb() handles XDP metadata?
+ + return skb; +}
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2403,26 +2455,52 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring, goto next; } /* error packet */ - /* Build SKB from the page - data starts at hard_start, - * frame begins after RSB(64) + pad(2) = 66 bytes. - */ - skb = napi_build_skb(hard_start, PAGE_SIZE - GENET_XDP_HEADROOM); - if (unlikely(!skb)) { - BCMGENET_STATS64_INC(stats, dropped); - page_pool_put_full_page(ring->page_pool, rx_page, - true); - goto next; - } - - skb_mark_for_recycle(skb); + /* XDP: frame data starts after RSB + pad */ + if (xdp_prog) { + struct xdp_buff xdp; + unsigned int xdp_act; + int pkt_len; + + pkt_len = len - GENET_RSB_PAD; + if (priv->crc_fwd_en) + pkt_len -= ETH_FCS_LEN; + + xdp_init_buff(&xdp, PAGE_SIZE, &ring->xdp_rxq); + xdp_prepare_buff(&xdp, page_address(rx_page), + GENET_RX_HEADROOM, pkt_len, false); + + xdp_act = bcmgenet_run_xdp(ring, xdp_prog, &xdp, + rx_page); + if (xdp_act != XDP_PASS) + goto next; + + /* XDP_PASS: build SKB from (possibly modified) xdp */ + skb = bcmgenet_xdp_build_skb(ring, &xdp, rx_page); + if (unlikely(!skb)) { + BCMGENET_STATS64_INC(stats, dropped); + page_pool_put_full_page(ring->page_pool, + rx_page, true); + goto next; + } + } else { + /* Build SKB from the page - data starts at + * hard_start, frame begins after RSB(64) + pad(2). + */ + skb = napi_build_skb(hard_start, + PAGE_SIZE - GENET_XDP_HEADROOM); + if (unlikely(!skb)) { + BCMGENET_STATS64_INC(stats, dropped); + page_pool_put_full_page(ring->page_pool, + rx_page, true); + goto next; + } - /* Reserve the RSB + pad, then set the data length */ - skb_reserve(skb, GENET_RSB_PAD); - __skb_put(skb, len - GENET_RSB_PAD); + skb_mark_for_recycle(skb); + skb_reserve(skb, GENET_RSB_PAD); + __skb_put(skb, len - GENET_RSB_PAD); - if (priv->crc_fwd_en) { - skb_trim(skb, skb->len - ETH_FCS_LEN); - len -= ETH_FCS_LEN; + if (priv->crc_fwd_en) + skb_trim(skb, skb->len - ETH_FCS_LEN); }
[ ... ]