BPF List
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Lorenzo Bianconi" <lorenzo@kernel.org>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v5 3/8] net: ice: add xmo_rx_checksum callback
Date: Wed, 15 Jul 2026 21:48:00 +0000	[thread overview]
Message-ID: <20260715214801.AB7221F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260715-bpf-xdp-meta-rxcksum-v5-3-623d5c0d0ab7@kernel.org>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `ice_xdp_rx_checksum()` XDP metadata getter calls `ice_get_rx_csum()`, which has side effects that artificially inflate hardware error statistics and cause double-counting.
--

--- Patch [3]: [PATCH bpf-next v5 3/8] net: ice: add xmo_rx_checksum callback ---
commit 741cd60a0647d5876e42a8815bc6526c4da474e1
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 RX checksum code reusable from both ice_rx_csum() and
ice_xdp_rx_checksum().

> 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_rx_csum(struct ice_rx_ring *ring, struct sk_buff *skb,
> -	    union ice_32b_rx_flex_desc *rx_desc, u16 ptype)
> +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]
Since ice_get_rx_csum() increments hw_rx_eipe_error and hw_csum_rx_error, does
calling it from the XDP metadata getter artificially inflate these statistics?

An eBPF program can call the metadata getter multiple times on a single packet,
which would cause multi-counting. Furthermore, if the program returns XDP_PASS,
the networking stack will process the packet and call ice_rx_csum() which
calls ice_get_rx_csum() again, resulting in double-counting for the error.

> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-bpf-xdp-meta-rxcksum-v5-0-623d5c0d0ab7@kernel.org?part=3

  reply	other threads:[~2026-07-15 21:48 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-15 21:39 [PATCH bpf-next v5 0/8] Add the the capability to load HW RX checksum in eBPF programs Lorenzo Bianconi
2026-07-15 21:39 ` [PATCH bpf-next v5 1/8] netlink: specs: add XDP RX checksum capability to XDP metadata specs Lorenzo Bianconi
2026-07-15 22:24   ` bot+bpf-ci
2026-07-15 21:39 ` [PATCH bpf-next v5 2/8] net: veth: add xmo_rx_checksum callback to veth driver Lorenzo Bianconi
2026-07-15 22:08   ` sashiko-bot
2026-07-15 21:39 ` [PATCH bpf-next v5 3/8] net: ice: add xmo_rx_checksum callback Lorenzo Bianconi
2026-07-15 21:48   ` sashiko-bot [this message]
2026-07-15 21:39 ` [PATCH bpf-next v5 4/8] selftests/bpf: add selftest support for bpf_xdp_metadata_rx_checksum Lorenzo Bianconi
2026-07-15 21:39 ` [PATCH bpf-next v5 5/8] selftests/bpf: add bpf_xdp_metadata_rx_checksum support to xdp_hw_metadat prog Lorenzo Bianconi
2026-07-15 21:39 ` [PATCH bpf-next v5 6/8] net: add bpf_skb_rx_checksum kfunc to read skb checksum metadata Lorenzo Bianconi
2026-07-15 21:39 ` [PATCH bpf-next v5 7/8] selftests/bpf: Add test for bpf_skb_rx_checksum kfunc Lorenzo Bianconi
2026-07-15 22:08   ` sashiko-bot
2026-07-15 21:39 ` [PATCH bpf-next v5 8/8] selftests: net: add test for XDP_PASS skb checksum invalidation Lorenzo Bianconi
2026-07-15 21:58   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260715214801.AB7221F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=lorenzo@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox