Re: [PATCH bpf-next v4 3/6] net: ice: Add xmo_rx_checksum callback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The XDP metadata getter `ice_xdp_rx_checksum` calls `ice_get_rx_csum`, which increments hardware statistics (`hw_csum_rx_error` and `hw_rx_eipe_error`). This violates the expectation that BPF metadata getters are stateless/idempotent, allowing an eBPF program to arbitrarily inflate these statistics by repeatedly querying the checksum, and causes double-counting when `XDP_PASS` is used.
--

--- Patch [3]: [PATCH bpf-next v4 3/6] net: ice: Add xmo_rx_checksum callback ---
commit 7b93052dd91e42401898b8ce9ae946cf9d0f21ec
Author: Lorenzo Bianconi <lorenzo@kernel.org>

net: ice: Add xmo_rx_checksum callback

This patch implements the xmo_rx_checksum callback in the ice driver to
report RX checksum results to eBPF programs. It introduces the
ice_get_rx_csum utility routine to make the checksum code reusable from
ice_rx_csum().

> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
> index e695a664e53d1..3aa82ff03d9e2 100644
> --- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c

[ ... ]

> +static void
> +ice_get_rx_csum(const union ice_32b_rx_flex_desc *rx_desc, u16 ptype,
> +		struct ice_rx_ring *ring, enum xdp_checksum *ip_summed,
> +		u32 *cksum, u8 *cksum_level)
>  {

[ ... ]

>  	if (ipv4 && (rx_status0 & (BIT(ICE_RX_FLEX_DESC_STATUS0_XSUM_EIPE_S)))) {
>  		ring->vsi->back->hw_rx_eipe_error++;
> -		return;
> +		goto checksum_none;
>  	}

[ ... ]

>  checksum_fail:
>  	ring->vsi->back->hw_csum_rx_error++;
> +checksum_none:
> +	*ip_summed = XDP_CHECKSUM_NONE;
> +	*cksum_level = 0;
> +	*cksum = 0;
> +}

[ ... ]

> +static int ice_xdp_rx_checksum(const struct xdp_md *ctx,
> +			       enum xdp_checksum *ip_summed,
> +			       u32 *cksum, u8 *cksum_level)
> +{
> +	const struct libeth_xdp_buff *xdp_ext = (void *)ctx;
> +	const union ice_32b_rx_flex_desc *rx_desc = xdp_ext->desc;
> +	struct ice_rx_ring *ring;
> +
> +	ring = libeth_xdp_buff_to_rq(xdp_ext, typeof(*ring), xdp_rxq);
> +	ice_get_rx_csum(rx_desc, ice_get_ptype(rx_desc), ring, ip_summed,
> +			cksum, cksum_level);

[Severity: Medium]
Will this lead to inflated hardware error statistics?

The XDP metadata getter ice_xdp_rx_checksum() calls ice_get_rx_csum(), which
increments hardware statistics like hw_csum_rx_error and hw_rx_eipe_error when
a checksum error is detected.

Since XDP metadata getters can be queried multiple times by an eBPF program,
could this cause the statistics to be incremented multiple times for a single
packet?

Additionally, if the program returns XDP_PASS, the normal SKB receive path
will call ice_rx_csum(), which again calls ice_get_rx_csum(). Does this lead
to double-counting the errors for the same packet? 

Could the statistics increments be decoupled from the metadata getter?

> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708203410.45121-1-deliran@verdict.gg?part=3