Skip to contentSkip to conversation

Add the the capability to load HW RX checksum in eBPF programs

PATCH In review Lorenzo Bianconi 5 participants 25 messages Jul 15 – Jul 22

Subsystems (10)
Networking Network drivers Documentation Kselftest +6 more subsystems
lore
Share & tools
lore

Lorenzo Bianconi

author
#0 lore raw

v5 cover Add the the capability to load HW RX checksum in eBPF programs

Introduce bpf_xdp_metadata_rx_checksum() kfunc in order to load the HW RX checksum results in the eBPF program binded to the NIC. Implement xmo_rx_checksum callback for veth and ice drivers.

If the hardware detects a wrong/failed checksum, it will report CHECKSUM_NONE in the packet metadata. Moreover, CHECKSUM_NONE will be returned even if the NIC can't parse the packet (e.g. if it does not support a specific protocol). A possible use case for bpf_xdp_metadata_rx_checksum() would be to implement a XDP DDoS application [1] combining the info from bpf_xdp_metadata_rx_checksum() and bpf_xdp_metadata_rx_hash() kfuncs in order to filter packets with a wrong/failed checksum.

[1] https://blog.cloudflare.com/unimog-cloudflares-edge-load-balancer/

---
Changes in v5:
- Add new kfunc to Documentation/networking/xdp-rx-metadata.rst documentation
- Introduce bpf_skb_rx_checksum kfunc and related selftest.
- Introduce selftest to check the driver invalidates the rx checksum if running in XDP mode.

Changes in v4:
- Report cusm and csum_level separately and convert ip_summed to a
bitmask in bpf_xdp_metadata_rx_hash()
- Fix endianness issue in bpf selftests
- Add a comment about the driver expected behaviour if the the NIC reports CHECKSUM_UNNECESSARY or CHECKSUM_COMPLETE and the eBPF program modifes
the packet
- Introduce bpf_get_skb_cksum kfunc and related selftest to load the
checksum result to the SCHED_CLS eBPF program attached to the qdisc.
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Remoe leftover assignment from v2 in veth_xdp_rx_checksum()
- Fix typos
- Fix commit logs
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Remove XDP_CHECKSUM_PARTIAL definition
- Improve veth_xdp_rx_checksum() callback
- Fix uninitialized case for cksum_meta in ice_get_rx_csum()
- Fix sparse warnings in ice driver
- Fix typos
- Link to v1: https://lore.kernel.org/r/[email protected]

Changes in v1:
- Rebase on top of bpf-next
- Test ice driver using xdp_hw_metadata tool available in the bpf
kernel selftest
- Improve cover letter with an use-case for
bpf_xdp_metadata_rx_checksum()
- Link to RFC v2: https://lore.kernel.org/r/[email protected]

Changes in RFC v2:
- Squash patch 1/6 and 2/6
- Introduce enum xdp_checksum definitions
- Rework ice support to reuse ice_rx_csum codebase

---
Lorenzo Bianconi (8):
netlink: specs: add XDP RX checksum capability to XDP metadata specs
net: veth: add xmo_rx_checksum callback to veth driver
net: ice: add xmo_rx_checksum callback
selftests/bpf: add selftest support for bpf_xdp_metadata_rx_checksum selftests/bpf: add bpf_xdp_metadata_rx_checksum support to xdp_hw_metadat prog
net: add bpf_skb_rx_checksum kfunc to read skb checksum metadata selftests/bpf: Add test for bpf_skb_rx_checksum kfunc
selftests: net: add test for XDP_PASS skb checksum invalidation

 Documentation/netlink/specs/netdev.yaml            |   5 +
 Documentation/networking/xdp-rx-metadata.rst       |   8 ++
 drivers/net/ethernet/intel/ice/ice_txrx_lib.c      | 123 ++++++++++++++-------
 drivers/net/veth.c                                 |  32 ++++++
 include/net/xdp.h                                  |  13 +++
 include/uapi/linux/netdev.h                        |   3 +
 net/core/filter.c                                  |  31 ++++++
 net/core/xdp.c                                     |  32 ++++++
 tools/include/uapi/linux/netdev.h                  |   3 +
 tools/testing/selftests/bpf/bpf_kfuncs.h           |   8 ++
 .../selftests/bpf/prog_tests/skb_rx_checksum.c     |  50 +++++++++
 .../selftests/bpf/prog_tests/xdp_metadata.c        |   9 ++
 .../testing/selftests/bpf/progs/skb_rx_checksum.c  |  33 ++++++
 .../testing/selftests/bpf/progs/xdp_hw_metadata.c  |   7 ++
 tools/testing/selftests/bpf/progs/xdp_metadata.c   |   2 +
 tools/testing/selftests/bpf/skb_rx_checksum.h      |   8 ++
 tools/testing/selftests/bpf/xdp_hw_metadata.c      |  31 ++++++
 tools/testing/selftests/bpf/xdp_metadata.h         |  13 +++
 .../selftests/drivers/net/hw/xdp_metadata.py       |  55 ++++++++-
 .../selftests/net/lib/skb_metadata_csum.bpf.c      |  73 ++++++++++++
 20 files changed, 496 insertions(+), 43 deletions(-)

---
base-commit: f6f3b36c15ed44de1fbb44e645e4fae8c4a4453e
change-id: 20250925-bpf-xdp-meta-rxcksum-900685e2909d

Best regards,

Signature · 1 line
Lorenzo Bianconi <[email protected]>
author
#1 lore raw

PATCH v5 1/8 netlink: specs: add XDP RX checksum capability to XDP metadata specs

Introduce XDP RX checksum capability to XDP metadata specs. XDP RX checksum will be use by devices capable of exposing receive checksum result via bpf_xdp_metadata_rx_checksum().
Moreover, introduce xmo_rx_checksum netdev callback in order to allow the eBPF program bound to the device to retrieve the RX checksum result computed by the hw NIC and reported via DMA descriptors.

Reviewed-by Aleksandr Loktionov <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 Documentation/netlink/specs/netdev.yaml      |  5 +++++
 Documentation/networking/xdp-rx-metadata.rst |  3 +++
 include/net/xdp.h                            | 13 +++++++++++
 include/uapi/linux/netdev.h                  |  3 +++
 net/core/xdp.c                               | 32 ++++++++++++++++++++++++++++
 tools/include/uapi/linux/netdev.h            |  3 +++
 6 files changed, 59 insertions(+)
Documentation/netlink/specs/netdev.yaml +5 -0
index 5f143da7458c..6d0d90d3a614 100644
@@ -61,6 +61,11 @@ definitions:
         doc: |
           Device is capable of exposing receive packet VLAN tag via
           bpf_xdp_metadata_rx_vlan_tag().
+      -
+        name: checksum
+        doc: |
+          Device is capable of exposing receive checksum result via
+          bpf_xdp_metadata_rx_checksum().
   -
     type: flags
     name: xsk-flags
Documentation/networking/xdp-rx-metadata.rst +3 -0
index efdf5eeb49e7..93918b3769a3 100644
@@ -28,6 +28,9 @@ metadata is supported, this set will grow:
 .. kernel-doc:: net/core/xdp.c
    :identifiers: bpf_xdp_metadata_rx_vlan_tag
 
+.. kernel-doc:: net/core/xdp.c
+   :identifiers: bpf_xdp_metadata_rx_checksum
+
 An XDP program can use these kfuncs to read the metadata into stack
 variables for its own consumption. Or, to pass the metadata on to other
 consumers, an XDP program can store it into the metadata area carried
include/net/xdp.h +13 -0
index aa742f413c35..d95e126f2426 100644
@@ -586,6 +586,10 @@ void xdp_attachment_setup(struct xdp_attachment_info *info,
 			   NETDEV_XDP_RX_METADATA_VLAN_TAG, \
 			   bpf_xdp_metadata_rx_vlan_tag, \
 			   xmo_rx_vlan_tag) \
+	XDP_METADATA_KFUNC(XDP_METADATA_KFUNC_RX_CHECKSUM, \
+			   NETDEV_XDP_RX_METADATA_CHECKSUM, \
+			   bpf_xdp_metadata_rx_checksum, \
+			   xmo_rx_checksum)
 
 enum xdp_rx_metadata {
 #define XDP_METADATA_KFUNC(name, _, __, ___) name,

@@ -643,12 +647,21 @@ enum xdp_rss_hash_type {
 	XDP_RSS_TYPE_L4_IPV6_SCTP_EX = XDP_RSS_TYPE_L4_IPV6_SCTP | XDP_RSS_L3_DYNHDR,
 };
 
+enum xdp_checksum {
+	XDP_CHECKSUM_NONE		= BIT(CHECKSUM_NONE),
+	XDP_CHECKSUM_UNNECESSARY	= BIT(CHECKSUM_UNNECESSARY),
+	XDP_CHECKSUM_COMPLETE		= BIT(CHECKSUM_COMPLETE),
+};
+
 struct xdp_metadata_ops {
 	int	(*xmo_rx_timestamp)(const struct xdp_md *ctx, u64 *timestamp);
 	int	(*xmo_rx_hash)(const struct xdp_md *ctx, u32 *hash,
 			       enum xdp_rss_hash_type *rss_type);
 	int	(*xmo_rx_vlan_tag)(const struct xdp_md *ctx, __be16 *vlan_proto,
 				   u16 *vlan_tci);
+	int	(*xmo_rx_checksum)(const struct xdp_md *ctx,
+				   enum xdp_checksum *ip_summed,
+				   u32 *cksum, u8 *cksum_level);
 };
 
 #ifdef CONFIG_NET
include/uapi/linux/netdev.h +3 -0
index 2f3ab75e8cc0..f8caade93c8c 100644
@@ -47,11 +47,14 @@ enum netdev_xdp_act {
  *   hash via bpf_xdp_metadata_rx_hash().
  * @NETDEV_XDP_RX_METADATA_VLAN_TAG: Device is capable of exposing receive
  *   packet VLAN tag via bpf_xdp_metadata_rx_vlan_tag().
+ * @NETDEV_XDP_RX_METADATA_CHECKSUM: Device is capable of exposing receive
+ *   checksum result via bpf_xdp_metadata_rx_checksum().
  */
 enum netdev_xdp_rx_metadata {
 	NETDEV_XDP_RX_METADATA_TIMESTAMP = 1,
 	NETDEV_XDP_RX_METADATA_HASH = 2,
 	NETDEV_XDP_RX_METADATA_VLAN_TAG = 4,
+	NETDEV_XDP_RX_METADATA_CHECKSUM = 8,
 };
 
 /**
net/core/xdp.c +32 -0
index 9890a30584ba..9bcaa423ad17 100644
@@ -961,6 +961,38 @@ __bpf_kfunc int bpf_xdp_metadata_rx_vlan_tag(const struct xdp_md *ctx,
 	return -EOPNOTSUPP;
 }
 
+/**
+ * bpf_xdp_metadata_rx_checksum - Read XDP frame RX checksum.
+ * @ctx: XDP context pointer.
+ * @ip_summed: Return value pointer to a bitmask indicating available checksums.
+ * @cksum: Return value pointer indicating the hw checksum value.
+ * @cksum_level: Return value pointer indicating the checksum level result.
+ *
+ * In case of success, ``ip_summed`` is set to the RX checksum result. Possible
+ * values are:
+ * ``XDP_CHECKSUM_NONE``
+ * ``XDP_CHECKSUM_UNNECESSARY``
+ * ``XDP_CHECKSUM_COMPLETE``
+ * ``XDP_CHECKSUM_COMPLETE`` | ``XDP_CHECKSUM_UNNECESSARY``
+ *
+ * In case of success, ``cksum`` contains the checksum value calculated by the
+ * NIC. ``cksum`` is valid only if ``XDP_CHECKSUM_COMPLETE`` is set in
+ * ``ip_summed``. ``cksum_level`` contains the checksum level reported by the
+ * hw. ``cksum_level`` can be considered valid only if
+ * ``XDP_CHECKSUM_UNNECESSARY`` is set in ``ip_summed``.
+ *
+ * Return:
+ * * Returns 0 on success or ``-errno`` on error.
+ * * ``-EOPNOTSUPP`` : means device driver does not implement kfunc
+ * * ``-ENODATA``    : means no RX-checksum available for this frame
+ */
+__bpf_kfunc int bpf_xdp_metadata_rx_checksum(const struct xdp_md *ctx,
+					     enum xdp_checksum *ip_summed,
+					     u32 *cksum, u8 *cksum_level)
+{
+	return -EOPNOTSUPP;
+}
+
 __bpf_kfunc_end_defs();
 
 BTF_KFUNCS_START(xdp_metadata_kfunc_ids)
tools/include/uapi/linux/netdev.h +3 -0
index 2f3ab75e8cc0..f8caade93c8c 100644
@@ -47,11 +47,14 @@ enum netdev_xdp_act {
  *   hash via bpf_xdp_metadata_rx_hash().
  * @NETDEV_XDP_RX_METADATA_VLAN_TAG: Device is capable of exposing receive
  *   packet VLAN tag via bpf_xdp_metadata_rx_vlan_tag().
+ * @NETDEV_XDP_RX_METADATA_CHECKSUM: Device is capable of exposing receive
+ *   checksum result via bpf_xdp_metadata_rx_checksum().
  */
 enum netdev_xdp_rx_metadata {
 	NETDEV_XDP_RX_METADATA_TIMESTAMP = 1,
 	NETDEV_XDP_RX_METADATA_HASH = 2,
 	NETDEV_XDP_RX_METADATA_VLAN_TAG = 4,
+	NETDEV_XDP_RX_METADATA_CHECKSUM = 8,
 };
 
 /**
Signature · 1 line
2.55.0
AI review
#2 lore raw

Re: [PATCH bpf-next v5 1/8] netlink: specs: add XDP RX checksum capability to XDP metadata specs

Earlier review of this series raised the case where the NIC reports COMPLETE or UNNECESSARY, the XDP program then modifies the packet and returns XDP_PASS. In that situation the reported checksum is…
Quoted 17 lines
net/core/xdp.c +9 -0
index 9890a30584ba..9bcaa423ad17 100644
@@ -961,6 +961,38 @@ __bpf_kfunc int bpf_xdp_metadata_rx_vlan_tag(const struct xdp_md *ctx,
 	return -EOPNOTSUPP;
 }
 
+/**
+ * bpf_xdp_metadata_rx_checksum - Read XDP frame RX checksum.
+ * @ctx: XDP context pointer.
+ * @ip_summed: Return value pointer to a bitmask indicating available checksums.
+ * @cksum: Return value pointer indicating the hw checksum value.
+ * @cksum_level: Return value pointer indicating the checksum level result.
+ *
+ * In case of success, ``ip_summed`` is set to the RX checksum result. Possible
+ * values are:

net/core/xdp.c __bpf_kfunc int bpf_xdp_metadata_rx_vlan_tag(const struct xdp_md *ctx, · line 982

Quoted 10 lines
net/core/xdp.c +10 -0
+ * ``XDP_CHECKSUM_NONE``
+ * ``XDP_CHECKSUM_UNNECESSARY``
+ * ``XDP_CHECKSUM_COMPLETE``
+ * ``XDP_CHECKSUM_COMPLETE`` | ``XDP_CHECKSUM_UNNECESSARY``
+ *
+ * In case of success, ``cksum`` contains the checksum value calculated by the
+ * NIC. ``cksum`` is valid only if ``XDP_CHECKSUM_COMPLETE`` is set in
+ * ``ip_summed``. ``cksum_level`` contains the checksum level reported by the
+ * hw. ``cksum_level`` can be considered valid only if
+ * ``XDP_CHECKSUM_UNNECESSARY`` is set in ``ip_summed``.

Earlier review of this series raised the case where the NIC reports COMPLETE or UNNECESSARY, the XDP program then modifies the packet and returns XDP_PASS. In that situation the reported checksum is stale.

It was noted on the list that drivers today do not invalidate the checksum in this case, they only stop reporting COMPLETE while an XDP program is attached.

Would it help to document that expected driver behavior here (how it works today versus what is expected), and possibly add a kselftest that checks it? If this is handled in a later patch of the series, please disregard.

net/core/xdp.c __bpf_kfunc int bpf_xdp_metadata_rx_vlan_tag(const struct xdp_md *ctx, · line 994

Quoted 12 lines
net/core/xdp.c +12 -0
+ *
+ * Return:
+ * * Returns 0 on success or ``-errno`` on error.
+ * * ``-EOPNOTSUPP`` : means device driver does not implement kfunc
+ * * ``-ENODATA``    : means no RX-checksum available for this frame
+ */
+__bpf_kfunc int bpf_xdp_metadata_rx_checksum(const struct xdp_md *ctx,
+					     enum xdp_checksum *ip_summed,
+					     u32 *cksum, u8 *cksum_level)
+{
+	return -EOPNOTSUPP;
+}

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/29453967825

maintainer
#3 lore raw

Re: [PATCH bpf-next v5 1/8] netlink: specs: add XDP RX checksum capability to XDP metadata specs

In reply to Lorenzo Bianconi
Quoted 5 lines · On Wed, 15 Jul 2026 23:39:41 +0200 Lorenzo Bianconi wrote:

+ * In case of success, ``cksum`` contains the checksum value calculated by the
+ * NIC. ``cksum`` is valid only if ``XDP_CHECKSUM_COMPLETE`` is set in
+ * ``ip_summed``. ``cksum_level`` contains the checksum level reported by the
+ * hw. ``cksum_level`` can be considered valid only if
+ * ``XDP_CHECKSUM_UNNECESSARY`` is set in ``ip_summed``.

nit: cksum -> csum like the rest of the networking stack

author
#4 lore raw

PATCH v5 2/8 net: veth: add xmo_rx_checksum callback to veth driver

In reply to Lorenzo Bianconi

Implement xmo_rx_checksum callback in veth driver to report RX checksum result to the eBPF program bounded to the veth device.

Reviewed-by Aleksandr Loktionov <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 drivers/net/veth.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
drivers/net/veth.c +32 -0
index 8170bf33ccf9..dbf30089581c 100644
@@ -1701,6 +1701,37 @@ static int veth_xdp_rx_vlan_tag(const struct xdp_md *ctx, __be16 *vlan_proto,
 	return err;
 }
 
+static int veth_xdp_rx_checksum(const struct xdp_md *ctx,
+				enum xdp_checksum *ip_summed,
+				u32 *cksum, u8 *cksum_level)
+{
+	const struct veth_xdp_buff *_ctx = (void *)ctx;
+	const struct sk_buff *skb = _ctx->skb;
+
+	if (!skb)
+		return -ENODATA;
+
+	switch (skb->ip_summed) {
+	case CHECKSUM_COMPLETE:
+		*ip_summed = XDP_CHECKSUM_COMPLETE;
+		*cksum = skb->csum;
+		*cksum_level = 0;
+		break;
+	case CHECKSUM_UNNECESSARY:
+		*ip_summed = XDP_CHECKSUM_UNNECESSARY;
+		*cksum_level = skb->csum_level;
+		*cksum = 0;
+		break;
+	default:
+		*ip_summed = XDP_CHECKSUM_NONE;
+		*cksum_level = 0;
+		*cksum = 0;
+		break;
+	}
+
+	return 0;
+}
+
 static const struct net_device_ops veth_netdev_ops = {
 	.ndo_init            = veth_dev_init,
 	.ndo_open            = veth_open,

@@ -1726,6 +1757,7 @@ static const struct xdp_metadata_ops veth_xdp_metadata_ops = {
 	.xmo_rx_timestamp		= veth_xdp_rx_timestamp,
 	.xmo_rx_hash			= veth_xdp_rx_hash,
 	.xmo_rx_vlan_tag		= veth_xdp_rx_vlan_tag,
+	.xmo_rx_checksum		= veth_xdp_rx_checksum,
 };
 
 #define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HW_CSUM | \
Signature · 1 line
2.55.0
AI review
#5 lore raw

Re: [PATCH bpf-next v5 2/8] net: veth: add xmo_rx_checksum callback to veth driver

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] veth_xdp_rx_checksum incorrectly maps CHECKSUM_PARTIAL to XDP_CHECKSUM_NONE, causing XDP programs…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] veth_xdp_rx_checksum incorrectly maps CHECKSUM_PARTIAL to XDP_CHECKSUM_NONE, causing XDP programs to drop valid locally-generated packets. --

--- Patch [2]: [PATCH bpf-next v5 2/8] net: veth: add xmo_rx_checksum callback to veth driver --- commit 34eae1c96706424d6fb3ab7f5a60445ee0f6dd08
Author: Lorenzo Bianconi <[email protected]>

net: veth: add xmo_rx_checksum callback to veth driver

Implement xmo_rx_checksum callback in veth driver to report RX checksum result to the eBPF program bounded to the veth device.

Quoted 25 lines
drivers/net/veth.c +17 -0
index 8170bf33ccf95..dbf30089581c8 100644
@@ -1701,6 +1701,37 @@ static int veth_xdp_rx_vlan_tag(const struct xdp_md *ctx, __be16 *vlan_proto,
 	return err;
 }
 
+static int veth_xdp_rx_checksum(const struct xdp_md *ctx,
+				enum xdp_checksum *ip_summed,
+				u32 *cksum, u8 *cksum_level)
+{
+	const struct veth_xdp_buff *_ctx = (void *)ctx;
+	const struct sk_buff *skb = _ctx->skb;
+
+	if (!skb)
+		return -ENODATA;
+
+	switch (skb->ip_summed) {
+	case CHECKSUM_COMPLETE:
+		*ip_summed = XDP_CHECKSUM_COMPLETE;
+		*cksum = skb->csum;
+		*cksum_level = 0;
+		break;
+	case CHECKSUM_UNNECESSARY:

drivers/net/veth.c static int veth_xdp_rx_vlan_tag(const struct xdp_md *ctx, __be16 *vlan_proto, · line 1730

Quoted 10 lines
drivers/net/veth.c +10 -0
+		*ip_summed = XDP_CHECKSUM_UNNECESSARY;
+		*cksum_level = skb->csum_level;
+		*cksum = 0;
+		break;
+	default:
+		*ip_summed = XDP_CHECKSUM_NONE;
+		*cksum_level = 0;
+		*cksum = 0;
+		break;
+	}

[Severity: High]
Is there a missing case for CHECKSUM_PARTIAL in this switch statement inside veth_xdp_rx_checksum()?

When a local process sends traffic over a veth pair with checksum offload enabled, the packet is passed to the receiving veth with skb->ip_summed set to CHECKSUM_PARTIAL.

By falling through to the default case, this maps CHECKSUM_PARTIAL to XDP_CHECKSUM_NONE. An attached XDP program that validates checksums will see XDP_CHECKSUM_NONE and attempt to compute a software checksum. The software computation will fail because the TCP/UDP checksum field only contains a pseudo-header checksum at this point, leading to dropped packets.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
author
#6 lore raw

PATCH v5 3/8 net: ice: add xmo_rx_checksum callback

In reply to Lorenzo Bianconi

Implement xmo_rx_checksum callback in ice driver to report RX checksum result to the eBPF program bounded to the NIC.
Introduce ice_get_rx_csum utility routine in order to make the rx checksum code reusable from ice_rx_csum()

Reviewed-by Aleksandr Loktionov <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 123 +++++++++++++++++---------
 1 file changed, 81 insertions(+), 42 deletions(-)
drivers/net/ethernet/intel/ice/ice_txrx_lib.c +81 -42
index e695a664e53d..3aa82ff03d9e 100644
@@ -78,69 +78,48 @@ ice_rx_hash_to_skb(const struct ice_rx_ring *rx_ring,
 		libeth_rx_pt_set_hash(skb, hash, decoded);
 }
 
-/**
- * ice_rx_gcs - Set generic checksum in skb
- * @skb: skb currently being received and modified
- * @rx_desc: receive descriptor
- */
-static void ice_rx_gcs(struct sk_buff *skb,
-		       const union ice_32b_rx_flex_desc *rx_desc)
-{
-	const struct ice_32b_rx_flex_desc_nic *desc;
-	u16 csum;
-
-	desc = (struct ice_32b_rx_flex_desc_nic *)rx_desc;
-	skb->ip_summed = CHECKSUM_COMPLETE;
-	csum = (__force u16)desc->raw_csum;
-	skb->csum = csum_unfold((__force __sum16)swab16(csum));
-}
-
-/**
- * ice_rx_csum - Indicate in skb if checksum is good
- * @ring: the ring we care about
- * @skb: skb currently being received and modified
- * @rx_desc: the receive descriptor
- * @ptype: the packet type decoded by hardware
- *
- * skb->protocol must be set before this function is called
- */
 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)
 {
-	struct libeth_rx_pt decoded;
+	struct libeth_rx_pt decoded = libie_rx_pt_parse(ptype);
 	u16 rx_status0, rx_status1;
 	bool ipv4, ipv6;
 
-	/* Start with CHECKSUM_NONE and by default csum_level = 0 */
-	skb->ip_summed = CHECKSUM_NONE;
-
-	decoded = libie_rx_pt_parse(ptype);
 	if (!libeth_rx_pt_has_checksum(ring->netdev, decoded))
-		return;
+		goto checksum_none;
 
 	rx_status0 = le16_to_cpu(rx_desc->wb.status_error0);
 	rx_status1 = le16_to_cpu(rx_desc->wb.status_error1);
-
 	if ((ring->flags & ICE_RX_FLAGS_RING_GCS) &&
 	    rx_desc->wb.rxdid == ICE_RXDID_FLEX_NIC &&
 	    (decoded.inner_prot == LIBETH_RX_PT_INNER_TCP ||
 	     decoded.inner_prot == LIBETH_RX_PT_INNER_UDP ||
 	     decoded.inner_prot == LIBETH_RX_PT_INNER_ICMP)) {
-		ice_rx_gcs(skb, rx_desc);
+		const struct ice_32b_rx_flex_desc_nic *desc;
+		__wsum wcsum;
+		u16 csum;
+
+		desc = (struct ice_32b_rx_flex_desc_nic *)rx_desc;
+		*ip_summed = XDP_CHECKSUM_COMPLETE;
+		csum = (__force u16)desc->raw_csum;
+		wcsum = csum_unfold((__force __sum16)swab16(csum));
+		*cksum = (__force u32)wcsum;
+		*cksum_level = 0;
 		return;
 	}
 
 	/* check if HW has decoded the packet and checksum */
 	if (!(rx_status0 & BIT(ICE_RX_FLEX_DESC_STATUS0_L3L4P_S)))
-		return;
+		goto checksum_none;
 
 	ipv4 = libeth_rx_pt_get_ip_ver(decoded) == LIBETH_RX_PT_OUTER_IPV4;
 	ipv6 = libeth_rx_pt_get_ip_ver(decoded) == LIBETH_RX_PT_OUTER_IPV6;
 
 	if (ipv4 && (rx_status0 & (BIT(ICE_RX_FLEX_DESC_STATUS0_XSUM_EIPE_S)))) {
 		ring->vsi->back->hw_rx_eipe_error++;
-		return;
+		goto checksum_none;
 	}
 
 	if (ipv4 && (rx_status0 & (BIT(ICE_RX_FLEX_DESC_STATUS0_XSUM_IPE_S))))

@@ -164,14 +143,51 @@ ice_rx_csum(struct ice_rx_ring *ring, struct sk_buff *skb,
 	 * we need to bump the checksum level by 1 to reflect the fact that
 	 * we are indicating we validated the inner checksum.
 	 */
-	if (decoded.tunnel_type >= LIBETH_RX_PT_TUNNEL_IP_GRENAT)
-		skb->csum_level = 1;
-
-	skb->ip_summed = CHECKSUM_UNNECESSARY;
+	*cksum_level = decoded.tunnel_type >= LIBETH_RX_PT_TUNNEL_IP_GRENAT;
+	*ip_summed = XDP_CHECKSUM_UNNECESSARY;
+	*cksum = 0;
 	return;
 
 checksum_fail:
 	ring->vsi->back->hw_csum_rx_error++;
+checksum_none:
+	*ip_summed = XDP_CHECKSUM_NONE;
+	*cksum_level = 0;
+	*cksum = 0;
+}
+
+/**
+ * ice_rx_csum - Indicate in skb if checksum is good
+ * @ring: the ring we care about
+ * @skb: skb currently being received and modified
+ * @rx_desc: the receive descriptor
+ * @ptype: the packet type decoded by hardware
+ *
+ * skb->protocol must be set before this function is called
+ */
+static void
+ice_rx_csum(struct ice_rx_ring *ring, struct sk_buff *skb,
+	    union ice_32b_rx_flex_desc *rx_desc, u16 ptype)
+{
+	enum xdp_checksum ip_summed;
+	u8 cksum_level;
+	u32 cksum;
+
+	ice_get_rx_csum(rx_desc, ptype, ring, &ip_summed, &cksum,
+			&cksum_level);
+	switch (ip_summed) {
+	case XDP_CHECKSUM_UNNECESSARY:
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
+		skb->csum_level = cksum_level;
+		break;
+	case XDP_CHECKSUM_COMPLETE:
+		skb->ip_summed = CHECKSUM_COMPLETE;
+		skb->csum = (__force __wsum)cksum;
+		break;
+	default:
+		skb->ip_summed = CHECKSUM_NONE;
+		break;
+	}
 }
 
 /**

@@ -566,6 +582,28 @@ static int ice_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash,
 	return 0;
 }
 
+/**
+ * ice_xdp_rx_checksum - RX checksum XDP hint handler
+ * @ctx: XDP buff pointer
+ * @ip_summed: RX checksum result destination address
+ * @cksum: RX checksum value destination address
+ * @cksum_level: RX checksum level value destination address
+ */
+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);
+
+	return 0;
+}
+
 /**
  * ice_xdp_rx_vlan_tag - VLAN tag XDP hint handler
  * @ctx: XDP buff pointer

@@ -598,4 +636,5 @@ const struct xdp_metadata_ops ice_xdp_md_ops = {
 	.xmo_rx_timestamp		= ice_xdp_rx_hw_ts,
 	.xmo_rx_hash			= ice_xdp_rx_hash,
 	.xmo_rx_vlan_tag		= ice_xdp_rx_vlan_tag,
+	.xmo_rx_checksum		= ice_xdp_rx_checksum,
 };
Signature · 1 line
2.55.0
AI review
#7 lore raw

Re: [PATCH bpf-next v5 3/8] net: ice: add xmo_rx_checksum callback

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…

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 <[email protected]>

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().

Quoted 4 lines
drivers/net/ethernet/intel/ice/ice_txrx_lib.c +0 -0
index e695a664e53d1..3aa82ff03d9e2 100644

[ ... ]

Quoted 7 lines

static void

drivers/net/ethernet/intel/ice/ice_txrx_lib.c +3 -2
-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)
 {

[ ... ]

Quoted 5 lines
drivers/net/ethernet/intel/ice/ice_txrx_lib.c +1 -1
 	if (ipv4 && (rx_status0 & (BIT(ICE_RX_FLEX_DESC_STATUS0_XSUM_EIPE_S)))) {
 		ring->vsi->back->hw_rx_eipe_error++;
-		return;
+		goto checksum_none;
 	}

[ ... ]

Quoted 7 lines

checksum_fail:

drivers/net/ethernet/intel/ice/ice_txrx_lib.c +5 -0
 	ring->vsi->back->hw_csum_rx_error++;
+checksum_none:
+	*ip_summed = XDP_CHECKSUM_NONE;
+	*cksum_level = 0;
+	*cksum = 0;
+}

[ ... ]

drivers/net/ethernet/intel/ice/ice_txrx_lib.c static int ice_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash, · line 602

Quoted 11 lines
drivers/net/ethernet/intel/ice/ice_txrx_lib.c +11 -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.

Quoted 3 lines
drivers/net/ethernet/intel/ice/ice_txrx_lib.c +3 -0
+
+	return 0;
+}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
author
#8 lore raw

PATCH v5 4/8 selftests/bpf: add selftest support for bpf_xdp_metadata_rx_checksum

In reply to Lorenzo Bianconi

Introduce dedicated selftest for bpf_xdp_metadata_rx_checksum kfunc to bpf selftest framework.

Reviewed-by Aleksandr Loktionov <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 tools/testing/selftests/bpf/prog_tests/xdp_metadata.c | 9 +++++++++
 tools/testing/selftests/bpf/progs/xdp_metadata.c      | 2 ++
 tools/testing/selftests/bpf/xdp_metadata.h            | 9 +++++++++
 3 files changed, 20 insertions(+)
tools/testing/selftests/bpf/prog_tests/xdp_metadata.c +9 -0
index 5c31054ad4a4..91de61c822f5 100644
@@ -310,6 +310,15 @@ static int verify_xsk_metadata(struct xsk *xsk, bool sent_from_af_xdp)
 	if (!ASSERT_NEQ(meta->rx_hash, 0, "rx_hash"))
 		return -1;
 
+	if (!ASSERT_EQ(meta->ip_summed, XDP_CHECKSUM_NONE, "rx_ip_summed"))
+		return -1;
+
+	if (!ASSERT_EQ(meta->cksum, 0, "rx_cksum"))
+		return -1;
+
+	if (!ASSERT_EQ(meta->cksum_level, 0, "rx_cksum_level"))
+		return -1;
+
 	if (!sent_from_af_xdp) {
 		if (!ASSERT_NEQ(meta->rx_hash_type & XDP_RSS_TYPE_L4, 0, "rx_hash_type"))
 			return -1;
tools/testing/selftests/bpf/progs/xdp_metadata.c +2 -0
index 09bb8a038d52..af1e19d48d67 100644
@@ -98,6 +98,8 @@ int rx(struct xdp_md *ctx)
 	bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash, &meta->rx_hash_type);
 	bpf_xdp_metadata_rx_vlan_tag(ctx, &meta->rx_vlan_proto,
 				     &meta->rx_vlan_tci);
+	bpf_xdp_metadata_rx_checksum(ctx, &meta->ip_summed, &meta->cksum,
+				     &meta->cksum_level);
 
 	return bpf_redirect_map(&xsk, ctx->rx_queue_index, XDP_PASS);
 }
tools/testing/selftests/bpf/xdp_metadata.h +9 -0
index 87318ad1117a..bca09b94af26 100644
@@ -30,6 +30,10 @@ enum xdp_meta_field {
 	XDP_META_FIELD_VLAN_TAG	= BIT(2),
 };
 
+#define XDP_CHECKSUM_NONE		BIT(0)
+#define XDP_CHECKSUM_UNNECESSARY	BIT(1)
+#define XDP_CHECKSUM_COMPLETE		BIT(2)
+
 struct xdp_meta {
 	union {
 		__u64 rx_timestamp;

@@ -48,5 +52,10 @@ struct xdp_meta {
 		};
 		__s32 rx_vlan_tag_err;
 	};
+	struct {
+		__u32 ip_summed;
+		__u32 cksum;
+		__u8 cksum_level;
+	};
 	enum xdp_meta_field hint_valid;
 };
Signature · 1 line
2.55.0
author
#9 lore raw

PATCH v5 5/8 selftests/bpf: add bpf_xdp_metadata_rx_checksum support to xdp_hw_metadat prog

In reply to Lorenzo Bianconi

Introduce the capability to dump HW rx checksum in xdp_hw_metadata program via bpf_xdp_metadata_rx_checksum() kfunc.

Reviewed-by Aleksandr Loktionov <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 .../testing/selftests/bpf/progs/xdp_hw_metadata.c  |  7 +++++
 tools/testing/selftests/bpf/xdp_hw_metadata.c      | 31 ++++++++++++++++++++++
 tools/testing/selftests/bpf/xdp_metadata.h         | 12 ++++++---
 3 files changed, 46 insertions(+), 4 deletions(-)
tools/testing/selftests/bpf/progs/xdp_hw_metadata.c +7 -0
index 330ece2eabdb..5eeadb7e27cf 100644
@@ -110,6 +110,13 @@ int rx(struct xdp_md *ctx)
 	else
 		meta->hint_valid |= XDP_META_FIELD_VLAN_TAG;
 
+	err = bpf_xdp_metadata_rx_checksum(ctx, &meta->ip_summed,
+					   &meta->cksum, &meta->cksum_level);
+	if (err)
+		meta->rx_cksum_err = err;
+	else
+		meta->hint_valid |= XDP_META_FIELD_CHECKSUM;
+
 	__sync_add_and_fetch(&pkts_redir, 1);
 	return bpf_redirect_map(&xsk, ctx->rx_queue_index, XDP_PASS);
 }
tools/testing/selftests/bpf/xdp_hw_metadata.c +31 -0
index 6db3b5555a22..c63a70a54075 100644
@@ -8,6 +8,7 @@
  * - Metadata verified:
  *   - rx_timestamp
  *   - rx_hash
+ *   - rx_checksum
  *
  * TX:
  * - UDP 9091 packets trigger TX reply

@@ -219,6 +220,30 @@ static void print_vlan_tci(__u16 tag)
 	printf("PCP=%u, DEI=%d, VID=0x%X\n", pcp, dei, vlan_id);
 }
 
+static void print_rx_cksum(__u8 ip_summed, __u32 cksum, __u8 cksum_level)
+{
+	const char *cksum_str;
+
+	switch (ip_summed) {
+	case XDP_CHECKSUM_COMPLETE | XDP_CHECKSUM_UNNECESSARY:
+		cksum_str = "CHECKSUM_COMPLETE,CHECKSUM_UNNECESSARY";
+		break;
+	case XDP_CHECKSUM_UNNECESSARY:
+		cksum_str = "CHECKSUM_UNNECESSARY";
+		break;
+	case XDP_CHECKSUM_COMPLETE:
+		cksum_str = "CHECKSUM_COMPLETE";
+		break;
+	case XDP_CHECKSUM_NONE:
+	default:
+		cksum_str = "CHECKSUM_NONE";
+		break;
+	}
+
+	printf("rx-cksum: %s, csum=0x%x, cksum_level=0x%x\n",
+	       cksum_str, cksum, cksum_level);
+}
+
 static void verify_xdp_metadata(void *data, clockid_t clock_id)
 {
 	struct xdp_meta *meta;

@@ -254,6 +279,12 @@ static void verify_xdp_metadata(void *data, clockid_t clock_id)
 		printf("No rx_vlan_tci or rx_vlan_proto, err=%d\n",
 		       meta->rx_vlan_tag_err);
 	}
+
+	if (meta->hint_valid & XDP_META_FIELD_CHECKSUM)
+		print_rx_cksum(meta->ip_summed, meta->cksum,
+			       meta->cksum_level);
+	else
+		printf("No rx_cksum, err=%d\n", meta->rx_cksum_err);
 }
 
 static void verify_skb_metadata(int fd)
tools/testing/selftests/bpf/xdp_metadata.h +8 -4
index bca09b94af26..f864d4a8bd8c 100644
@@ -28,6 +28,7 @@ enum xdp_meta_field {
 	XDP_META_FIELD_TS	= BIT(0),
 	XDP_META_FIELD_RSS	= BIT(1),
 	XDP_META_FIELD_VLAN_TAG	= BIT(2),
+	XDP_META_FIELD_CHECKSUM = BIT(3),
 };
 
 #define XDP_CHECKSUM_NONE		BIT(0)

@@ -52,10 +53,13 @@ struct xdp_meta {
 		};
 		__s32 rx_vlan_tag_err;
 	};
-	struct {
-		__u32 ip_summed;
-		__u32 cksum;
-		__u8 cksum_level;
+	union {
+		struct {
+			__u32 ip_summed;
+			__u32 cksum;
+			__u8 cksum_level;
+		};
+		__s32 rx_cksum_err;
 	};
 	enum xdp_meta_field hint_valid;
 };
Signature · 1 line
2.55.0
author
#10 lore raw

PATCH v5 6/8 net: add bpf_skb_rx_checksum kfunc to read skb checksum metadata

In reply to Lorenzo Bianconi

Add a new BPF kfunc, bpf_skb_rx_checksum(), that allows TC and other skb-attached BPF programs to read the skb->ip_summed field and associated checksum metadata. This is needed because the existing bpf_csum_level() helper only distinguishes CHECKSUM_UNNECESSARY from all other states, making it unsuitable when the caller needs to differentiate CHECKSUM_NONE from CHECKSUM_COMPLETE (e.g. after XDP_PASS where the driver may have set either).

The kfunc writes two output values:
- ip_summed: the raw skb->ip_summed value (CHECKSUM_NONE=0, CHECKSUM_UNNECESSARY=1, CHECKSUM_COMPLETE=2, CHECKSUM_PARTIAL=3)
- csum_meta: CHECKSUM_COMPLETE -> hardware checksum (skb->csum)

               CHECKSUM_UNNECESSARY -> checksum level (skb->csum_level)
               otherwise -> 0
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 net/core/filter.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
net/core/filter.c +31 -0
index b446aa8be5c3..71d58ea53512 100644
@@ -12546,6 +12546,36 @@ __bpf_kfunc int bpf_xdp_pull_data(struct xdp_md *x, u32 len)
 	return 0;
 }
 
+/**
+ * bpf_skb_rx_checksum() - Read skb RX checksum info.
+ * @skb_: socket buffer to read from
+ * @ip_summed: return value for skb->ip_summed
+ * @csum_meta: checksum metadata
+ *
+ * Reads the checksum-related fields from a socket buffer. For
+ * %CHECKSUM_COMPLETE, csum_meta reports the hardware checksum value.
+ * For %CHECKSUM_UNNECESSARY, csum_meta reports the checksum level.
+ * For %CHECKSUM_NONE, csum_meta is zero.
+ *
+ * Return:
+ * * %0 - success
+ */
+__bpf_kfunc int bpf_skb_rx_checksum(struct __sk_buff *skb_, u32 *ip_summed,
+				    u32 *csum_meta)
+{
+	struct sk_buff *skb = (struct sk_buff *)skb_;
+
+	*ip_summed = skb->ip_summed;
+	if (skb->ip_summed == CHECKSUM_COMPLETE)
+		*csum_meta = skb->csum;
+	else if (skb->ip_summed == CHECKSUM_UNNECESSARY)
+		*csum_meta = skb->csum_level;
+	else
+		*csum_meta = 0;
+
+	return 0;
+}
+
 __bpf_kfunc_end_defs();
 
 int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,

@@ -12565,6 +12595,7 @@ int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
 
 BTF_KFUNCS_START(bpf_kfunc_check_set_skb)
 BTF_ID_FLAGS(func, bpf_dynptr_from_skb)
+BTF_ID_FLAGS(func, bpf_skb_rx_checksum)
 BTF_KFUNCS_END(bpf_kfunc_check_set_skb)
 
 BTF_KFUNCS_START(bpf_kfunc_check_set_skb_meta)
Signature · 1 line
2.55.0
author
#11 lore raw

PATCH v5 7/8 selftests/bpf: Add test for bpf_skb_rx_checksum kfunc

In reply to Lorenzo Bianconi

Add a selftest for the newly added bpf_skb_rx_checksum() kfunc. The test consists of a TC BPF program that calls the kfunc and records which checksum type each packet has in a per-type counter map, and a userspace test runner that exercises two scenarios:

- csum_none: runs with bpf_prog_test_run_opts (no special flags), verifies all packets are reported as CHECKSUM_NONE.
- csum_complete: runs with BPF_F_TEST_SKB_CHECKSUM_COMPLETE flag, verifies all packets are reported as CHECKSUM_COMPLETE.

Signed-off-by Lorenzo Bianconi <[email protected]>
---
 tools/testing/selftests/bpf/bpf_kfuncs.h           |  8 ++++
 .../selftests/bpf/prog_tests/skb_rx_checksum.c     | 50 ++++++++++++++++++++++
 .../testing/selftests/bpf/progs/skb_rx_checksum.c  | 33 ++++++++++++++
 tools/testing/selftests/bpf/skb_rx_checksum.h      |  8 ++++
 4 files changed, 99 insertions(+)
tools/testing/selftests/bpf/bpf_kfuncs.h +8 -0
index ae71e9b69051..d03572e26df2 100644
@@ -64,6 +64,14 @@ struct bpf_tcp_req_attrs;
 extern int bpf_sk_assign_tcp_reqsk(struct __sk_buff *skb, struct sock *sk,
 				   struct bpf_tcp_req_attrs *attrs, int attrs__sz) __ksym;
 
+/* Description
+ *  Read skb RX checksum info (ip_summed and csum_meta).
+ * Returns
+ *  0 on success
+ */
+extern int bpf_skb_rx_checksum(struct __sk_buff *skb, __u32 *ip_summed,
+			       __u32 *csum_meta) __ksym;
+
 void *bpf_cast_to_kern_ctx(void *) __ksym;
 
 extern void *bpf_rdonly_cast(const void *obj, __u32 btf_id) __ksym __weak;
tools/testing/selftests/bpf/prog_tests/skb_rx_checksum.c new file +50 -0

50 lines not on this page · raw message

tools/testing/selftests/bpf/progs/skb_rx_checksum.c new file +33 -0
new file mode 100644
index 000000000000..dc4e192905d5
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include <linux/pkt_cls.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_kfuncs.h"
+#include "skb_rx_checksum.h"
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, SKB_CSUM_PARTIAL);
+	__type(key, __u32);
+	__type(value, __u64);
+} csum_cnt SEC(".maps");
+
+SEC("tc")
+int tc_rx_csum(struct __sk_buff *skb)
+{
+	enum skb_csum ip_summed;
+	__u32 csum_meta;
+	__u64 *cnt;
+
+	bpf_skb_rx_checksum(skb, &ip_summed, &csum_meta);
+	if (ip_summed < SKB_CSUM_PARTIAL) {
+		cnt = bpf_map_lookup_elem(&csum_cnt, &ip_summed);
+		if (cnt)
+			__sync_fetch_and_add(cnt, 1);
+	}
+
+	return TC_ACT_OK;
+}
+
+char _license[] SEC("license") = "GPL";
tools/testing/selftests/bpf/skb_rx_checksum.h new file +8 -0
new file mode 100644
index 000000000000..60aad3561843
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+enum skb_csum {
+	SKB_CSUM_NONE		= 0,
+	SKB_CSUM_UNNECESSARY	= 1,
+	SKB_CSUM_COMPLETE	= 2,
+	SKB_CSUM_PARTIAL	= 3,
+};
Signature · 1 line
2.55.0
AI review
#12 lore raw

Re: [PATCH bpf-next v5 7/8] selftests/bpf: Add test for bpf_skb_rx_checksum kfunc

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Medium] The `csum_cnt` array map is sized incorrectly and explicitly drops valid `SKB_CSUM_PARTIAL`…

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The csum_cnt array map is sized incorrectly and explicitly drops valid SKB_CSUM_PARTIAL checksum states.
- [Low] Passing an enum skb_csum * to a kfunc expecting a __u32 * causes an incompatible pointer type warning.
- [Low] The multi-line comment for bpf_skb_rx_checksum violates the required BPF subsystem comment style. --

--- Patch [7]: [PATCH bpf-next v5 7/8] selftests/bpf: Add test for bpf_skb_rx_checksum kfunc --- commit b45897a4d5ef22742e4af5ab8e7a00ba0c92d2f2
Author: Lorenzo Bianconi <[email protected]>

selftests/bpf: Add test for bpf_skb_rx_checksum kfunc

This commit adds a selftest for the newly added bpf_skb_rx_checksum() kfunc. The test consists of a TC BPF program that calls the kfunc and records the checksum type, along with a userspace test runner.

tools/testing/selftests/bpf/bpf_kfuncs.h struct bpf_tcp_req_attrs; · line 68

Quoted 10 lines
tools/testing/selftests/bpf/bpf_kfuncs.h +2 -0
index ae71e9b690518..d03572e26df2d 100644
@@ -64,6 +64,14 @@ struct bpf_tcp_req_attrs;
 extern int bpf_sk_assign_tcp_reqsk(struct __sk_buff *skb, struct sock *sk,
 				   struct bpf_tcp_req_attrs *attrs, int attrs__sz) __ksym;
 
+/* Description
+ *  Read skb RX checksum info (ip_summed and csum_meta).

[Severity: Low]
This isn't a bug, but does this multi-line comment follow the BPF subsystem comment style? The guidelines require the opening /* to be on its own line.

Quoted 6 lines
tools/testing/selftests/bpf/bpf_kfuncs.h +6 -0
+ * Returns
+ *  0 on success
+ */
+extern int bpf_skb_rx_checksum(struct __sk_buff *skb, __u32 *ip_summed,
+			       __u32 *csum_meta) __ksym;
+

[ ... ]

Quoted 7 lines
tools/testing/selftests/bpf/progs/skb_rx_checksum.c new file +1 -0
new file mode 100644
index 0000000000000..dc4e192905d53
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0

tools/testing/selftests/bpf/progs/skb_rx_checksum.c line 11

Quoted 10 lines
tools/testing/selftests/bpf/progs/skb_rx_checksum.c new file +10 -0
+
+#include <linux/bpf.h>
+#include <linux/pkt_cls.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_kfuncs.h"
+#include "skb_rx_checksum.h"
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, SKB_CSUM_PARTIAL);

[Severity: Medium]
Does this incorrectly restrict the map size? SKB_CSUM_PARTIAL is defined as 3, which means the map can only hold entries for 0, 1, and 2, leaving no room to track the SKB_CSUM_PARTIAL state itself.

tools/testing/selftests/bpf/progs/skb_rx_checksum.c line 23

Quoted 12 lines
tools/testing/selftests/bpf/progs/skb_rx_checksum.c new file +12 -0
+	__type(key, __u32);
+	__type(value, __u64);
+} csum_cnt SEC(".maps");
+
+SEC("tc")
+int tc_rx_csum(struct __sk_buff *skb)
+{
+	enum skb_csum ip_summed;
+	__u32 csum_meta;
+	__u64 *cnt;
+
+	bpf_skb_rx_checksum(skb, &ip_summed, &csum_meta);

[Severity: Low]
Will this cause an incompatible pointer type compiler warning in tc_rx_csum()? The kfunc bpf_skb_rx_checksum() expects a __u32 * for its second argument, but &ip_summed is a pointer to an enum skb_csum.

tools/testing/selftests/bpf/progs/skb_rx_checksum.c line 24

Quoted 1 line
tools/testing/selftests/bpf/progs/skb_rx_checksum.c new file +1 -0
+	if (ip_summed < SKB_CSUM_PARTIAL) {

[Severity: Medium]
By using strictly less than SKB_CSUM_PARTIAL, does this silently drop valid packets that have the SKB_CSUM_PARTIAL state?

Quoted 7 lines
tools/testing/selftests/bpf/progs/skb_rx_checksum.c new file +7 -0
+		cnt = bpf_map_lookup_elem(&csum_cnt, &ip_summed);
+		if (cnt)
+			__sync_fetch_and_add(cnt, 1);
+	}
+
+	return TC_ACT_OK;
+}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
author
#13 lore raw

PATCH v5 8/8 selftests: net: add test for XDP_PASS skb checksum invalidation

In reply to Lorenzo Bianconi

Add a test that verifies skb->ip_summed is set to CHECKSUM_NONE when a device running in XDP mode creates an skb from a xdp_buff if the attached ebpf program returns an XDP_PASS.
The test attaches an XDP program returning XDP_PASS, and a TC ingress program that runs the bpf_skb_rx_checksum() kfunc to inspect the resulting skb. After XDP_PASS the driver must invalidate any previously computed hardware RX checksum since XDP may have modified the packet data.
The BPF program counts packets per checksum type in a map, and the test runner verifies that after sending traffic the CHECKSUM_NONE counter is non-zero while CHECKSUM_UNNECESSARY and CHECKSUM_COMPLETE counters are zero.

Signed-off-by Lorenzo Bianconi <[email protected]>
---
 Documentation/networking/xdp-rx-metadata.rst       |  5 ++
 .../selftests/drivers/net/hw/xdp_metadata.py       | 55 +++++++++++++++-
 .../selftests/net/lib/skb_metadata_csum.bpf.c      | 73 ++++++++++++++++++++++
 3 files changed, 132 insertions(+), 1 deletion(-)
Documentation/networking/xdp-rx-metadata.rst +5 -0
index 93918b3769a3..7434ac98242a 100644
@@ -90,6 +90,11 @@ conversion, and the XDP metadata is not used by the kernel when building
 ``skbs``. However, TC-BPF programs can access the XDP metadata area using
 the ``data_meta`` pointer.
 
+If a driver is running in XDP mode, any existing hardware RX checksum
+(``CHECKSUM_UNNECESSARY`` or ``CHECKSUM_COMPLETE``) must be invalidated
+by setting ``skb->ip_summed`` to ``CHECKSUM_NONE`` before passing the
+skb to the kernel, since XDP may have modified the packet data.
+
 In the future, we'd like to support a case where an XDP program
 can override some of the metadata used for building ``skbs``.
tools/testing/selftests/drivers/net/hw/xdp_metadata.py +54 -1

78 lines not on this page · raw message

tools/testing/selftests/net/lib/skb_metadata_csum.bpf.c new file +73 -0

73 lines not on this page · raw message

Signature · 1 line
2.55.0
AI review
#14 lore raw

Re: [PATCH bpf-next v5 8/8] selftests: net: add test for XDP_PASS skb checksum invalidation

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] The `tc_check_csum` BPF program strictly requires IPv6 traffic, but the python test probe can…

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The tc_check_csum BPF program strictly requires IPv6 traffic, but the python test probe can send IPv4 traffic depending on the environment configuration, causing the test to falsely fail.
- [Medium] The test leaks a tc filter and its associated BPF program if the interface already had a clsact qdisc.
--

--- Patch [8]: [PATCH bpf-next v5 8/8] selftests: net: add test for XDP_PASS skb checksum invalidation --- commit 2b96bd0af0eb846efe81e2f0246ecebcc855acd6
Author: Lorenzo Bianconi <[email protected]>

selftests: net: add test for XDP_PASS skb checksum invalidation

This commit adds a selftest to verify that a device running in XDP mode correctly invalidates the hardware RX checksum when an attached XDP program returns XDP_PASS. It checks this by attaching an XDP program and a TC ingress program to inspect the resulting skb via the bpf_skb_rx_checksum() kfunc.

Quoted 4 lines
tools/testing/selftests/drivers/net/hw/xdp_metadata.py +0 -0
index 33a1985356d90..8ccd34b776b4d 100755

[ ... ]

Quoted 17 lines
tools/testing/selftests/drivers/net/hw/xdp_metadata.py +13 -0
@@ -130,6 +131,57 @@ def test_xdp_rss_hash(cfg, proto):
             f"RSS hash type should include L4 for {proto.upper()} traffic")
 
 
+def test_xdp_pass_rx_csum(cfg):
+    """Test xdp_pass sets CHECKSUM_NONE on the resulting skb.
+
+    Attaches an XDP program that returns XDP_PASS and a TC ingress
+    program that checks skb->ip_summed via bpf_skb_rx_checksum().
+    Verifies the value is CHECKSUM_NONE.
+    """
+
+    bpf_obj = cfg.net_lib_dir / "skb_metadata_csum.bpf.o"
+    xdp_obj = cfg.net_lib_dir / "xdp_dummy.bpf.o"
+
+    # GRO may overwrite skb->ip_summed after the driver sets it,
+    # so disable it to preserve the checksum set by the driver.

tools/testing/selftests/drivers/net/hw/xdp_metadata.py def test_xdp_rss_hash(cfg, proto): · line 156

Quoted 10 lines
tools/testing/selftests/drivers/net/hw/xdp_metadata.py +10 -0
+    ethtool(f"-K {cfg.ifname} gro off")
+    defer(ethtool, f"-K {cfg.ifname} gro on")
+    ip(f"link set dev {cfg.ifname} xdp obj {xdp_obj} sec xdp")
+    defer(ip, f"link set dev {cfg.ifname} xdp off")
+
+    qdiscs = json.loads(cmd(f"tc -j qdisc show dev {cfg.ifname}").stdout)
+    if not any(q['kind'] == 'clsact' for q in qdiscs):
+        cmd(f"tc qdisc add dev {cfg.ifname} clsact")
+        defer(cmd, f"tc qdisc del dev {cfg.ifname} clsact")
+    cmd(f"tc filter add dev {cfg.ifname} ingress bpf da obj {bpf_obj} sec tc")

[Severity: Medium]
Does this leak the tc filter and BPF program if the interface already had a clsact qdisc?

Since the filter deletion is not explicitly deferred here, it appears the filter will remain active if the test bypassed the qdisc creation and deletion. This could cause subsequent test runs to falsely fail if they discover and query the leaked BPF program instead of the newly attached one.

Quoted 8 lines
tools/testing/selftests/drivers/net/hw/xdp_metadata.py +8 -0
+
+    progs = bpftool("prog list", json=True)
+    tc_prog_id = None
+    for p in progs:
+        if p.get("name") == "tc_check_csum":
+            tc_prog_id = p["id"]
+            break
+

tools/testing/selftests/drivers/net/hw/xdp_metadata.py def test_xdp_rss_hash(cfg, proto): · line 174

Quoted 10 lines
tools/testing/selftests/drivers/net/hw/xdp_metadata.py +10 -0
+    if tc_prog_id is None:
+        raise KsftSkipEx("Could not find tc_check_csum BPF program")
+
+    maps = bpf_prog_map_ids(tc_prog_id)
+    csum_map_id = maps.get("map_csum_result")
+    if csum_map_id is None:
+        raise KsftSkipEx("Could not find map_csum_result map")
+
+    for _ in range(10):
+        _send_probe(cfg, 12345, proto="udp")

[Severity: Medium]
Will this cause spurious test failures if the test environment defaults to IPv4?

The _send_probe() method will send IPv4 traffic if the environment's cfg.addr_ipver defaults to 4. However, the BPF program tc_check_csum explicitly ignores non-IPv6 traffic. If IPv4 packets are sent, the counters will remain at zero and the test will fail.

Quoted 2 lines
tools/testing/selftests/drivers/net/hw/xdp_metadata.py +2 -0
+
+    result = bpf_map_dump(csum_map_id)

[ ... ]

Quoted 5 lines
tools/testing/selftests/net/lib/skb_metadata_csum.bpf.c new file +0 -0
new file mode 100644
index 0000000000000..6a953c4acfe78

[ ... ]

Quoted 1 line
tools/testing/selftests/net/lib/skb_metadata_csum.bpf.c new file +0 -0
@@ -0,0 +1,73 @@

[ ... ]

Quoted 5 lines
tools/testing/selftests/net/lib/skb_metadata_csum.bpf.c new file +5 -0
+SEC("tc")
+int tc_check_csum(struct __sk_buff *skb)
+{
+	void *data_end = (void *)(long)skb->data_end;
+	void *data = (void *)(long)skb->data;

tools/testing/selftests/net/lib/skb_metadata_csum.bpf.c line 45

Quoted 10 lines
tools/testing/selftests/net/lib/skb_metadata_csum.bpf.c new file +10 -0
+	__u32 ip_summed, csum_meta;
+	struct ethhdr *eth = data;
+	struct ipv6hdr *ip6;
+	struct udphdr *udp;
+
+	if ((void *)(eth + 1) > data_end)
+		return TC_ACT_OK;
+
+	if (eth->h_proto != bpf_htons(ETH_P_IPV6))
+		return TC_ACT_OK;

[Severity: Medium]
Can this strict IPv6 check cause a mismatch with the python script?

As noted above, the python script calling _send_probe() might send IPv4 UDP packets depending on the environment configuration, which would be silently dropped here, bypassing the counter updates entirely.

Quoted 5 lines
tools/testing/selftests/net/lib/skb_metadata_csum.bpf.c new file +5 -0
+
+	ip6 = (void *)(eth + 1);
+	if ((void *)(ip6 + 1) > data_end)
+		return TC_ACT_OK;
+
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
#15 lore raw

Re: [PATCH bpf-next v5 8/8] selftests: net: add test for XDP_PASS skb checksum invalidation

In reply to Lorenzo Bianconi
Quoted 27 lines · Lorenzo Bianconi

Add a test that verifies skb->ip_summed is set to CHECKSUM_NONE when a device running in XDP mode creates an skb from a xdp_buff if the attached ebpf program returns an XDP_PASS.
The test attaches an XDP program returning XDP_PASS, and a TC ingress program that runs the bpf_skb_rx_checksum() kfunc to inspect the resulting skb. After XDP_PASS the driver must invalidate any previously computed hardware RX checksum since XDP may have modified the packet data.
The BPF program counts packets per checksum type in a map, and the test runner verifies that after sending traffic the CHECKSUM_NONE counter is non-zero while CHECKSUM_UNNECESSARY and CHECKSUM_COMPLETE counters are zero.

Signed-off-by Lorenzo Bianconi <[email protected]>
---
 Documentation/networking/xdp-rx-metadata.rst       |  5 ++
 .../selftests/drivers/net/hw/xdp_metadata.py       | 55 +++++++++++++++-
 .../selftests/net/lib/skb_metadata_csum.bpf.c      | 73 ++++++++++++++++++++++
 3 files changed, 132 insertions(+), 1 deletion(-)
Documentation/networking/xdp-rx-metadata.rst +0 -0
index 93918b3769a3..7434ac98242a 100644
@@ -90,6 +90,11 @@ conversion, and the XDP metadata is not used by the kernel when building
 ``skbs``. However, TC-BPF programs can access the XDP metadata area using
 the ``data_meta`` pointer.

[..]

Documentation/networking/xdp-rx-metadata.rst conversion, and the XDP metadata is not used by the kernel when building · line 99

Quoted 7 lines
Documentation/networking/xdp-rx-metadata.rst +5 -0
+If a driver is running in XDP mode, any existing hardware RX checksum
+(``CHECKSUM_UNNECESSARY`` or ``CHECKSUM_COMPLETE``) must be invalidated
+by setting ``skb->ip_summed`` to ``CHECKSUM_NONE`` before passing the
+skb to the kernel, since XDP may have modified the packet data.
+
 In the future, we'd like to support a case where an XDP program
 can override some of the metadata used for building ``skbs``.

Sorry for keeping nitpicking on this, but I'm still not convinced that it is what we currently do. From my previous reply:

Quoted 6 lines
Quoted 6 lines

Looking at a few drivers:
- bnxt (bnxt_rx_pkt) does UNNECESSARY - ok
- mlx5 (mlx5e_handle_csum) does UNNECESSARY and skips COMPLETE if there is bpf prog attached
- fbnic (fbnic_rx_csum) - can do COMPLETE even with xdp attached?
- gve (gve_rx) - can do COMPLETE even with xdp attached?

(although for gve I might be wrong, there is also gve_rx_skb_csum that only does UNNECESSARY).

I'd wait for Jakub to chime in, but it feels like we should just document what we currently do as a recommended approach: for the drivers that support COMPLETE, do not report it when the bpf program is attached. Both NONE and UNNECESSARY are ok.

Also, did you run this test on real HW? NIPA now has HW tests, maybe it makes sense to route this series via net-next to get the real coverage?

author
#16 lore raw

Re: [PATCH bpf-next v5 8/8] selftests: net: add test for XDP_PASS skb checksum invalidation

Quoted 31 lines · Stanislav Fomichev
Quoted 27 lines · Lorenzo Bianconi

Add a test that verifies skb->ip_summed is set to CHECKSUM_NONE when a device running in XDP mode creates an skb from a xdp_buff if the attached ebpf program returns an XDP_PASS.
The test attaches an XDP program returning XDP_PASS, and a TC ingress program that runs the bpf_skb_rx_checksum() kfunc to inspect the resulting skb. After XDP_PASS the driver must invalidate any previously computed hardware RX checksum since XDP may have modified the packet data.
The BPF program counts packets per checksum type in a map, and the test runner verifies that after sending traffic the CHECKSUM_NONE counter is non-zero while CHECKSUM_UNNECESSARY and CHECKSUM_COMPLETE counters are zero.

Signed-off-by Lorenzo Bianconi <[email protected]>
---
 Documentation/networking/xdp-rx-metadata.rst       |  5 ++
 .../selftests/drivers/net/hw/xdp_metadata.py       | 55 +++++++++++++++-
 .../selftests/net/lib/skb_metadata_csum.bpf.c      | 73 ++++++++++++++++++++++
 3 files changed, 132 insertions(+), 1 deletion(-)
Documentation/networking/xdp-rx-metadata.rst +0 -0
index 93918b3769a3..7434ac98242a 100644
@@ -90,6 +90,11 @@ conversion, and the XDP metadata is not used by the kernel when building
 ``skbs``. However, TC-BPF programs can access the XDP metadata area using
 the ``data_meta`` pointer.

[..]

Quoted 10 lines

Documentation/networking/xdp-rx-metadata.rst conversion, and the XDP metadata is not used by the kernel when building · line 99

Quoted 7 lines
Documentation/networking/xdp-rx-metadata.rst +5 -0
+If a driver is running in XDP mode, any existing hardware RX checksum
+(``CHECKSUM_UNNECESSARY`` or ``CHECKSUM_COMPLETE``) must be invalidated
+by setting ``skb->ip_summed`` to ``CHECKSUM_NONE`` before passing the
+skb to the kernel, since XDP may have modified the packet data.
+
 In the future, we'd like to support a case where an XDP program
 can override some of the metadata used for building ``skbs``.

Sorry for keeping nitpicking on this, but I'm still not convinced that it is what we currently do. From my previous reply:

no worries :)
My current take-away from the previous discussion is we just need to document what would be the driver expected behaviour adding a kselftest for it (without modifying any driver).

Quoted 8 lines
Quoted 7 lines
Quoted 7 lines

Looking at a few drivers:
- bnxt (bnxt_rx_pkt) does UNNECESSARY - ok
- mlx5 (mlx5e_handle_csum) does UNNECESSARY and skips COMPLETE if there is bpf prog attached
- fbnic (fbnic_rx_csum) - can do COMPLETE even with xdp attached?
- gve (gve_rx) - can do COMPLETE even with xdp attached?

Quoted 7 lines

(although for gve I might be wrong, there is also gve_rx_skb_csum that only does UNNECESSARY).

I'd wait for Jakub to chime in, but it feels like we should just document what we currently do as a recommended approach: for the drivers that support COMPLETE, do not report it when the bpf program is attached. Both NONE and UNNECESSARY are ok.

I am not completely sure the UNNECESSARY case is different from the COMPLETE one. What are we supposed to do if the driver reports UNNECESSARY and the ebpf program modifies some fields covered by the rx-checksum?

Quoted 3 lines

Also, did you run this test on real HW? NIPA now has HW tests, maybe it makes sense to route this series via net-next to get the real coverage?

What about splitting this series and have two different series:
- bpf-next: add xdp rx kfunc and related selftest
- net-next: add kselftest for the driver expected behaviour.

What do you think?

Regards,
Lorenzo

#17 lore raw

Re: [PATCH bpf-next v5 8/8] selftests: net: add test for XDP_PASS skb checksum invalidation

Quoted 59 lines · Lorenzo Bianconi
Quoted 31 lines · Stanislav Fomichev
Quoted 27 lines · Lorenzo Bianconi

Add a test that verifies skb->ip_summed is set to CHECKSUM_NONE when a device running in XDP mode creates an skb from a xdp_buff if the attached ebpf program returns an XDP_PASS.
The test attaches an XDP program returning XDP_PASS, and a TC ingress program that runs the bpf_skb_rx_checksum() kfunc to inspect the resulting skb. After XDP_PASS the driver must invalidate any previously computed hardware RX checksum since XDP may have modified the packet data.
The BPF program counts packets per checksum type in a map, and the test runner verifies that after sending traffic the CHECKSUM_NONE counter is non-zero while CHECKSUM_UNNECESSARY and CHECKSUM_COMPLETE counters are zero.

Signed-off-by Lorenzo Bianconi <[email protected]>
---
 Documentation/networking/xdp-rx-metadata.rst       |  5 ++
 .../selftests/drivers/net/hw/xdp_metadata.py       | 55 +++++++++++++++-
 .../selftests/net/lib/skb_metadata_csum.bpf.c      | 73 ++++++++++++++++++++++
 3 files changed, 132 insertions(+), 1 deletion(-)
Documentation/networking/xdp-rx-metadata.rst +0 -0
index 93918b3769a3..7434ac98242a 100644
@@ -90,6 +90,11 @@ conversion, and the XDP metadata is not used by the kernel when building
 ``skbs``. However, TC-BPF programs can access the XDP metadata area using
 the ``data_meta`` pointer.

[..]

Quoted 10 lines

Documentation/networking/xdp-rx-metadata.rst conversion, and the XDP metadata is not used by the kernel when building · line 99

Quoted 7 lines
Documentation/networking/xdp-rx-metadata.rst +5 -0
+If a driver is running in XDP mode, any existing hardware RX checksum
+(``CHECKSUM_UNNECESSARY`` or ``CHECKSUM_COMPLETE``) must be invalidated
+by setting ``skb->ip_summed`` to ``CHECKSUM_NONE`` before passing the
+skb to the kernel, since XDP may have modified the packet data.
+
 In the future, we'd like to support a case where an XDP program
 can override some of the metadata used for building ``skbs``.

Sorry for keeping nitpicking on this, but I'm still not convinced that it is what we currently do. From my previous reply:

no worries :)
My current take-away from the previous discussion is we just need to document what would be the driver expected behaviour adding a kselftest for it (without modifying any driver).

Quoted 11 lines
Quoted 7 lines
Quoted 7 lines

Looking at a few drivers:
- bnxt (bnxt_rx_pkt) does UNNECESSARY - ok
- mlx5 (mlx5e_handle_csum) does UNNECESSARY and skips COMPLETE if there is bpf prog attached
- fbnic (fbnic_rx_csum) - can do COMPLETE even with xdp attached?
- gve (gve_rx) - can do COMPLETE even with xdp attached?

(although for gve I might be wrong, there is also gve_rx_skb_csum that only does UNNECESSARY).

Quoted 8 lines
Quoted 4 lines

I'd wait for Jakub to chime in, but it feels like we should just document what we currently do as a recommended approach: for the drivers that support COMPLETE, do not report it when the bpf program is attached. Both NONE and UNNECESSARY are ok.

I am not completely sure the UNNECESSARY case is different from the COMPLETE one. What are we supposed to do if the driver reports UNNECESSARY and the ebpf program modifies some fields covered by the rx-checksum?

For unnecessary, I think the safe expectation is that the bpf program will update the value of the checksum in the packet if it touches the data?

Quoted 8 lines
Quoted 2 lines

Also, did you run this test on real HW? NIPA now has HW tests, maybe it makes sense to route this series via net-next to get the real coverage?

What about splitting this series and have two different series:
- bpf-next: add xdp rx kfunc and related selftest
- net-next: add kselftest for the driver expected behaviour.

What do you think?

I'd post everything to net-next to get the HW coverage. Once you get all the acks we can ask the maintainers' guidance.

author
#18 lore raw

Re: [PATCH bpf-next v5 8/8] selftests: net: add test for XDP_PASS skb checksum invalidation

Quoted 65 lines · Lorenzo Bianconi
Quoted 64 lines · Stanislav Fomichev
Quoted 31 lines · Lorenzo Bianconi
Quoted 27 lines

Add a test that verifies skb->ip_summed is set to CHECKSUM_NONE when a device running in XDP mode creates an skb from a xdp_buff if the attached ebpf program returns an XDP_PASS.
The test attaches an XDP program returning XDP_PASS, and a TC ingress program that runs the bpf_skb_rx_checksum() kfunc to inspect the resulting skb. After XDP_PASS the driver must invalidate any previously computed hardware RX checksum since XDP may have modified the packet data.
The BPF program counts packets per checksum type in a map, and the test runner verifies that after sending traffic the CHECKSUM_NONE counter is non-zero while CHECKSUM_UNNECESSARY and CHECKSUM_COMPLETE counters are zero.

Signed-off-by Lorenzo Bianconi <[email protected]>
---
 Documentation/networking/xdp-rx-metadata.rst       |  5 ++
 .../selftests/drivers/net/hw/xdp_metadata.py       | 55 +++++++++++++++-
 .../selftests/net/lib/skb_metadata_csum.bpf.c      | 73 ++++++++++++++++++++++
 3 files changed, 132 insertions(+), 1 deletion(-)
Documentation/networking/xdp-rx-metadata.rst +0 -0
index 93918b3769a3..7434ac98242a 100644
@@ -90,6 +90,11 @@ conversion, and the XDP metadata is not used by the kernel when building
 ``skbs``. However, TC-BPF programs can access the XDP metadata area using
 the ``data_meta`` pointer.

[..]

Quoted 10 lines

Documentation/networking/xdp-rx-metadata.rst conversion, and the XDP metadata is not used by the kernel when building · line 99

Quoted 7 lines
Documentation/networking/xdp-rx-metadata.rst +5 -0
+If a driver is running in XDP mode, any existing hardware RX checksum
+(``CHECKSUM_UNNECESSARY`` or ``CHECKSUM_COMPLETE``) must be invalidated
+by setting ``skb->ip_summed`` to ``CHECKSUM_NONE`` before passing the
+skb to the kernel, since XDP may have modified the packet data.
+
 In the future, we'd like to support a case where an XDP program
 can override some of the metadata used for building ``skbs``.

Sorry for keeping nitpicking on this, but I'm still not convinced that it is what we currently do. From my previous reply:

no worries :)
My current take-away from the previous discussion is we just need to document what would be the driver expected behaviour adding a kselftest for it (without modifying any driver).

Quoted 15 lines
Quoted 7 lines
Quoted 7 lines

Looking at a few drivers:
- bnxt (bnxt_rx_pkt) does UNNECESSARY - ok
- mlx5 (mlx5e_handle_csum) does UNNECESSARY and skips COMPLETE if there is bpf prog attached
- fbnic (fbnic_rx_csum) - can do COMPLETE even with xdp attached?
- gve (gve_rx) - can do COMPLETE even with xdp attached?

(although for gve I might be wrong, there is also gve_rx_skb_csum that only does UNNECESSARY).

I'd wait for Jakub to chime in, but it feels like we should just document what we currently do as a recommended approach: for the drivers that support COMPLETE, do not report it when the bpf program is attached. Both NONE and UNNECESSARY are ok.

Quoted 6 lines
Quoted 3 lines

I am not completely sure the UNNECESSARY case is different from the COMPLETE one. What are we supposed to do if the driver reports UNNECESSARY and the ebpf program modifies some fields covered by the rx-checksum?

For unnecessary, I think the safe expectation is that the bpf program will update the value of the checksum in the packet if it touches the data?

I do not have a strong opinion on it.
@Jakub: any input on it?

Quoted 4 lines
Quoted 4 lines
Quoted 3 lines

Also, did you run this test on real HW? NIPA now has HW tests, maybe it makes sense to route this series via net-next to get the real coverage?

Quoted 8 lines
Quoted 5 lines

What about splitting this series and have two different series:
- bpf-next: add xdp rx kfunc and related selftest
- net-next: add kselftest for the driver expected behaviour.

What do you think?

I'd post everything to net-next to get the HW coverage. Once you get all the acks we can ask the maintainers' guidance.

ack, I am fine with that.

Regards,
Lorenzo

maintainer
#19 lore raw

Re: [PATCH bpf-next v5 8/8] selftests: net: add test for XDP_PASS skb checksum invalidation

In reply to Stanislav Fomichev
Quoted 8 lines · On Mon, 20 Jul 2026 12:32:55 -0700 Stanislav Fomichev wrote:
Quoted 8 lines
Quoted 7 lines

(although for gve I might be wrong, there is also gve_rx_skb_csum that only does UNNECESSARY).

I'd wait for Jakub to chime in, but it feels like we should just document what we currently do as a recommended approach: for the drivers that support COMPLETE, do not report it when the bpf program is attached. Both NONE and UNNECESSARY are ok.

Quoted 6 lines
Quoted 3 lines

I am not completely sure the UNNECESSARY case is different from the COMPLETE one. What are we supposed to do if the driver reports UNNECESSARY and the ebpf program modifies some fields covered by the rx-checksum?

For unnecessary, I think the safe expectation is that the bpf program will update the value of the checksum in the packet if it touches the data?

Documenting as expected behavior which no driver currently follows is a bit silly. I thought the ask was to sketch out the plan of explicitly updating/invalidating the checksum even if we don't implement it today?

Quoted 11 lines
Quoted 8 lines
Quoted 2 lines

Also, did you run this test on real HW? NIPA now has HW tests, maybe it makes sense to route this series via net-next to get the real coverage?

What about splitting this series and have two different series:
- bpf-next: add xdp rx kfunc and related selftest
- net-next: add kselftest for the driver expected behaviour.

What do you think?

I'd post everything to net-next to get the HW coverage. Once you get all the acks we can ask the maintainers' guidance.

+1, and please add proper tests for the real drivers exercising the kfunc with traffic (would be good to try to send a non-TCP/UDP frame to try to cajole the driver into using COMPLETE).

#20 lore raw

Re: [PATCH bpf-next v5 8/8] selftests: net: add test for XDP_PASS skb checksum invalidation

Quoted 13 lines · Jakub Kicinski
Quoted 12 lines · On Mon, 20 Jul 2026 12:32:55 -0700 Stanislav Fomichev wrote:
Quoted 11 lines
Quoted 7 lines

(although for gve I might be wrong, there is also gve_rx_skb_csum that only does UNNECESSARY).

I'd wait for Jakub to chime in, but it feels like we should just document what we currently do as a recommended approach: for the drivers that support COMPLETE, do not report it when the bpf program is attached. Both NONE and UNNECESSARY are ok.

I am not completely sure the UNNECESSARY case is different from the COMPLETE one. What are we supposed to do if the driver reports UNNECESSARY and the ebpf program modifies some fields covered by the rx-checksum?

Quoted 7 lines
Quoted 2 lines

For unnecessary, I think the safe expectation is that the bpf program will update the value of the checksum in the packet if it touches the data?

Documenting as expected behavior which no driver currently follows is a bit silly. I thought the ask was to sketch out the plan of explicitly updating/invalidating the checksum even if we don't implement it today?

This is about current drivers that only report UNNECESSARY with xdp: the xdp prog has to maintain in-packet checksum if it changes the payload. I think it's a fair assumption?

In terms of documentation, here is what I have on my side, lmk if that makes sense, roughly:

- TODAY
- some drivers (correctly) disable reporting COMPLETE when XDP is attached
- the xdp program has to modify the packet checksum value if it changes the payload
- some drivers (incorrectly?) report COMPLETE for xdp-to-skb path -> unsafe, needs to be fixed
- updating the payload doesn't update skb->csum, so the safest option right now is to only do UNNECESSARY with xdp for all drivers
- the hw test needs to make sure that the csum is either
NONE or UNNECESSARY, and will error out on COMPLETE

- TOMORROW
- if we want to properly support COMPLETE, we need a new set of kfuncs

    to update skb->csum value in the skb

Does that make sense?

maintainer
#21 lore raw

Re: [PATCH bpf-next v5 8/8] selftests: net: add test for XDP_PASS skb checksum invalidation

Quoted 11 lines · On Tue, 21 Jul 2026 16:03:37 -0700 Stanislav Fomichev wrote:
Quoted 7 lines
Quoted 2 lines

For unnecessary, I think the safe expectation is that the bpf program will update the value of the checksum in the packet if it touches the data?

Documenting as expected behavior which no driver currently follows is a bit silly. I thought the ask was to sketch out the plan of explicitly updating/invalidating the checksum even if we don't implement it today?

This is about current drivers that only report UNNECESSARY with xdp: the xdp prog has to maintain in-packet checksum if it changes the payload. I think it's a fair assumption?

What about decap? If we decap the header that device validated as UNNECESSARY there's no possibility of maintaining the checksum (by which IIUC you mean correcting it in along the payload changes).

I think we'd need some API to "decrement the csum level" ? Or some heuristic in the drivers to decrement if the head moved by at least len(IP+UDP) into the packet ?

Quoted 11 lines

In terms of documentation, here is what I have on my side, lmk if that makes sense, roughly:

- TODAY
- some drivers (correctly) disable reporting COMPLETE when XDP is attached
- the xdp program has to modify the packet checksum value if it changes the payload
- some drivers (incorrectly?) report COMPLETE for xdp-to-skb path -> unsafe, needs to be fixed
- updating the payload doesn't update skb->csum, so the safest option right now is to only do UNNECESSARY with xdp for all drivers

Yes, that sounds fair.

Quoted 2 lines

- the hw test needs to make sure that the csum is either NONE or UNNECESSARY, and will error out on COMPLETE

Driver can report COMPLETE to XDP, just not to the stack. I think we can use a tracepoint (bpftrace) or attach in TCP to check the skb state?

Quoted 5 lines

- TOMORROW
- if we want to properly support COMPLETE, we need a new set of kfuncs

    to update skb->csum value in the skb

Does that make sense?

#22 lore raw

Re: [PATCH bpf-next v5 8/8] selftests: net: add test for XDP_PASS skb checksum invalidation

Quoted 13 lines · Jakub Kicinski
Quoted 11 lines · On Tue, 21 Jul 2026 16:03:37 -0700 Stanislav Fomichev wrote:
Quoted 7 lines
Quoted 2 lines

For unnecessary, I think the safe expectation is that the bpf program will update the value of the checksum in the packet if it touches the data?

Documenting as expected behavior which no driver currently follows is a bit silly. I thought the ask was to sketch out the plan of explicitly updating/invalidating the checksum even if we don't implement it today?

This is about current drivers that only report UNNECESSARY with xdp: the xdp prog has to maintain in-packet checksum if it changes the payload. I think it's a fair assumption?

Quoted 7 lines

What about decap? If we decap the header that device validated as UNNECESSARY there's no possibility of maintaining the checksum (by which IIUC you mean correcting it in along the payload changes).

I think we'd need some API to "decrement the csum level" ? Or some heuristic in the drivers to decrement if the head moved by at least len(IP+UDP) into the packet ?

True :-/ I guess one more case to document?

Quoted 12 lines
Quoted 11 lines

In terms of documentation, here is what I have on my side, lmk if that makes sense, roughly:

- TODAY
- some drivers (correctly) disable reporting COMPLETE when XDP is attached
- the xdp program has to modify the packet checksum value if it changes the payload
- some drivers (incorrectly?) report COMPLETE for xdp-to-skb path -> unsafe, needs to be fixed
- updating the payload doesn't update skb->csum, so the safest option right now is to only do UNNECESSARY with xdp for all drivers

Quoted 8 lines

Yes, that sounds fair.

Quoted 2 lines

- the hw test needs to make sure that the csum is either NONE or UNNECESSARY, and will error out on COMPLETE

Driver can report COMPLETE to XDP, just not to the stack. I think we can use a tracepoint (bpftrace) or attach in TCP to check the skb state?

No strong opinion on this, it seems easier to report the same to both xdp and bpf (at least initially)

maintainer
#23 lore raw

Re: [PATCH bpf-next v5 8/8] selftests: net: add test for XDP_PASS skb checksum invalidation

Quoted 8 lines · On Wed, 22 Jul 2026 11:00:07 -0700 Stanislav Fomichev wrote:
Quoted 8 lines
Quoted 3 lines

This is about current drivers that only report UNNECESSARY with xdp: the xdp prog has to maintain in-packet checksum if it changes the payload. I think it's a fair assumption?

What about decap? If we decap the header that device validated as UNNECESSARY there's no possibility of maintaining the checksum (by which IIUC you mean correcting it in along the payload changes).

Quoted 5 lines
Quoted 3 lines

I think we'd need some API to "decrement the csum level" ? Or some heuristic in the drivers to decrement if the head moved by at least len(IP+UDP) into the packet ?

True :-/ I guess one more case to document?

What would we be documenting? That it's a known bug and we have no idea how to address it? Or am I missing something?

Again:
.. the ask was to sketch out the plan of explicitly updating/invalidating the checksum even if we don't implement it today

#24 lore raw

Re: [PATCH bpf-next v5 8/8] selftests: net: add test for XDP_PASS skb checksum invalidation

Quoted 9 lines · Jakub Kicinski
Quoted 8 lines · On Wed, 22 Jul 2026 11:00:07 -0700 Stanislav Fomichev wrote:
Quoted 8 lines
Quoted 3 lines

This is about current drivers that only report UNNECESSARY with xdp: the xdp prog has to maintain in-packet checksum if it changes the payload. I think it's a fair assumption?

What about decap? If we decap the header that device validated as UNNECESSARY there's no possibility of maintaining the checksum (by which IIUC you mean correcting it in along the payload changes).

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

I think we'd need some API to "decrement the csum level" ? Or some heuristic in the drivers to decrement if the head moved by at least len(IP+UDP) into the packet ?

True :-/ I guess one more case to document?

What would we be documenting? That it's a known bug and we have no idea how to address it? Or am I missing something?

Yeah, that UNECESSARY + decap in xdp has this corner case of inner csum being not validated (and currently no way to flip it to NONE from bpf).

(idk whether we really have nics that do csum_level>=2, so I'd just add bpf_invalidate_csum or something, but it's a separate conversation)

Quoted 4 lines

Again:
.. the ask was to sketch out the plan of explicitly updating/invalidating the checksum even if we don't implement it today

SGTM, I'll leave it up to Lorenzo. I had this TODAY vs TOMORROW summary, we can expand on that with UNNECESSARY+csum_level plus maybe more details on the shape of kfuncs for TOMORROW section..