[PATCH net v4 0/2] xsk: fix multi-buffer TX software checksum

[PATCH net v4 0/2] xsk: fix multi-buffer TX software checksum

[Rongzheng Wang]

This is v4 of the AF_XDP multi-buffer TX software checksum fix.

I found this while testing multi-buffer TX with XDP_UMEM_TX_SW_CSUM: a
two-descriptor packet had a bad wire checksum, while a single descriptor was
correct. There was no production or syzbot report.

Patch 1 moves the software checksum calculation from xsk_skb_metadata() to
the point where the last descriptor has been added to the skb.  The old
code calculated the checksum after the first descriptor, so multi-buffer
packets only included the first fragment in the checksum.

Patch 2 sends the packet using every descriptor count from 1 through
MAX_SKB_FRAGS + 1, and runs the existing metadata and checksum verification
for each case.  The last case is the copy-mode frame limit documented in
af_xdp.rst.

Patch 1 is unchanged since v3.  The updated selftest passes in a local QEMU
run.

v4:
- exercise every descriptor count up to MAX_SKB_FRAGS + 1 (Maciej)
- keep the last descriptor within the packet length (Maciej)
- fix the local variable declaration order (Maciej)

v3: https://lore.kernel.org/all/20261028025026.11401-1-testwrzwrz@gmail.com/
v2: https://lore.kernel.org/all/20261024141157.59301-1-testwrzwrz@gmail.com/
v1: https://lore.kernel.org/all/20261024091256.17918-1-testwrzwrz@gmail.com/

Rongzheng Wang (2):
  xsk: defer software checksum until the skb is complete
  selftests/bpf: cover AF_XDP multi-buffer TX software checksum

 net/xdp/xsk.c                                 |  23 ++-
 .../selftests/bpf/prog_tests/xdp_metadata.c   | 147 +++++++++++-------
 2 files changed, 107 insertions(+), 63 deletions(-)

-- 
2.39.5 (Apple Git-154)

[PATCH net v4 1/2] xsk: defer software checksum until the skb is complete

[Rongzheng Wang]

When XDP_UMEM_TX_SW_CSUM is used with multi-buffer TX, xsk_skb_metadata()
calls skb_checksum_help() while only the first descriptor has been added to
the skb. The checksum therefore covers just the first fragment, and
skb_checksum_help() sets ip_summed to CHECKSUM_NONE, so the remaining
fragments are never included in the checksum.

Move the software checksum calculation to the point where the last
descriptor has been added to the skb.

On a checksum failure the packet is now dropped instead of being returned
to the application for a retry. This path is not reachable with the
existing metadata validation, but the behavior change is intentional.

Fixes: 11614723af26 ("xsk: Add option to calculate TX checksum in SW")
Cc: stable@vger.kernel.org
Signed-off-by: Rongzheng Wang <testwrzwrz@gmail.com>
---
 net/xdp/xsk.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 33475b180ea6..aa6b010192ad 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -1011,14 +1011,6 @@ static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
 		skb->csum_start = hr + csum_start;
 		skb->csum_offset = csum_offset;
 		skb->ip_summed = CHECKSUM_PARTIAL;
-
-		if (unlikely(pool->tx_sw_csum)) {
-			int err;
-
-			err = skb_checksum_help(skb);
-			if (err)
-				return err;
-		}
 	}
 
 	if (flags & XDP_TXMD_FLAGS_LAUNCH_TIME)
@@ -1030,6 +1022,15 @@ static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
 	return 0;
 }
 
+static int xsk_skb_sw_csum(struct sk_buff *skb, struct xsk_buff_pool *pool)
+{
+	if (unlikely(pool->tx_sw_csum &&
+		     skb->ip_summed == CHECKSUM_PARTIAL))
+		return skb_checksum_help(skb);
+
+	return 0;
+}
+
 static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
 					      struct xdp_desc *desc)
 {
@@ -1275,6 +1276,12 @@ static int __xsk_generic_xmit(struct sock *sk)
 		if (xp_mb_desc(&desc)) {
 			xs->skb = skb;
 			continue;
+		} else {
+			err = xsk_skb_sw_csum(skb, xs->pool);
+			if (unlikely(err)) {
+				xsk_drop_skb(skb);
+				goto out;
+			}
 		}
 
 		err = __dev_direct_xmit(skb, xs->queue_id);
-- 
2.39.5 (Apple Git-154)

[PATCH net v4 2/2] selftests/bpf: cover AF_XDP multi-buffer TX software checksum

[Rongzheng Wang]

Extend test_xdp_metadata to send the same UDP packet using every descriptor
count from 1 through MAX_TX_DESCS (MAX_SKB_FRAGS + 1), and to run the
existing metadata and checksum verification for each case.

Build the packet once and write it into the TX descriptors in a loop
instead of using a flat two-descriptor layout. Keep the first descriptor
large enough to cover the checksum offset; the remaining payload is split
across the continuation descriptors, with the last descriptor taking the
remaining length.

Reuse the existing AF_XDP setup, packet generation, completion, and receive
helpers in xdp_metadata.c instead of adding a separate test harness.

Signed-off-by: Rongzheng Wang <testwrzwrz@gmail.com>
---
 .../selftests/bpf/prog_tests/xdp_metadata.c   | 147 +++++++++++-------
 1 file changed, 92 insertions(+), 55 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
index 5c31054ad4a4..17da0a145666 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
@@ -18,12 +18,15 @@
 #define TX_NAME "veTX"
 #define RX_NAME "veRX"
 
-#define UDP_PAYLOAD_BYTES 4
+#define UDP_PAYLOAD_BYTES 32
+
+#define MAX_SKB_FRAGS 17 /* minimum CONFIG_MAX_SKB_FRAGS */
+#define MAX_TX_DESCS (MAX_SKB_FRAGS + 1)
 
 #define UDP_SOURCE_PORT 1234
 #define AF_XDP_CONSUMER_PORT 8080
 
-#define UMEM_NUM 16
+#define UMEM_NUM 64
 #define UMEM_FRAME_SIZE XSK_UMEM__DEFAULT_FRAME_SIZE
 #define UMEM_SIZE (UMEM_FRAME_SIZE * UMEM_NUM)
 #define XDP_FLAGS XDP_FLAGS_DRV_MODE
@@ -56,13 +59,13 @@ struct xsk {
 	struct xsk_socket *socket;
 };
 
-static int open_xsk(int ifindex, struct xsk *xsk)
+static int open_xsk(int ifindex, struct xsk *xsk, __u32 bind_flags)
 {
 	int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
 	const struct xsk_socket_config socket_config = {
 		.rx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
 		.tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
-		.bind_flags = XDP_COPY,
+		.bind_flags = bind_flags,
 	};
 	const struct xsk_umem_config umem_config = {
 		.fill_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
@@ -133,31 +136,35 @@ static void close_xsk(struct xsk *xsk)
 	munmap(xsk->umem_area, UMEM_SIZE);
 }
 
-static int generate_packet(struct xsk *xsk, __u16 dst_port)
+static int generate_packet(struct xsk *xsk, __u16 dst_port, u32 num)
 {
 	struct xsk_tx_metadata *meta;
 	struct xdp_desc *tx_desc;
 	struct udphdr *udph;
 	struct ethhdr *eth;
 	struct iphdr *iph;
+	u16 first_len;
+	u16 pkt_len;
+	u8 pkt[256];
+	u32 offset;
 	void *data;
 	__u32 idx;
+	u32 addr;
 	int ret;
+	u32 i;
 
-	ret = xsk_ring_prod__reserve(&xsk->tx, 1, &idx);
-	if (!ASSERT_EQ(ret, 1, "xsk_ring_prod__reserve"))
+	pkt_len = sizeof(*eth) + sizeof(*iph) + sizeof(*udph) + UDP_PAYLOAD_BYTES;
+	first_len = sizeof(*eth) + sizeof(*iph) + sizeof(*udph);
+	if (!num || num > MAX_TX_DESCS ||
+	    (num > 1 && pkt_len - first_len < num - 1))
 		return -1;
 
-	tx_desc = xsk_ring_prod__tx_desc(&xsk->tx, idx);
-	tx_desc->addr = idx % (UMEM_NUM / 2) * UMEM_FRAME_SIZE + sizeof(struct xsk_tx_metadata);
-	printf("%p: tx_desc[%u]->addr=%llx\n", xsk, idx, tx_desc->addr);
-	data = xsk_umem__get_data(xsk->umem_area, tx_desc->addr);
-
-	meta = data - sizeof(struct xsk_tx_metadata);
-	memset(meta, 0, sizeof(*meta));
-	meta->flags = XDP_TXMD_FLAGS_TIMESTAMP;
+	ret = xsk_ring_prod__reserve(&xsk->tx, num, &idx);
+	if (!ASSERT_EQ(ret, num, "xsk_ring_prod__reserve"))
+		return -1;
 
-	eth = data;
+	memset(pkt, 0, sizeof(pkt));
+	eth = (void *)pkt;
 	iph = (void *)(eth + 1);
 	udph = (void *)(iph + 1);
 
@@ -180,18 +187,43 @@ static int generate_packet(struct xsk *xsk, __u16 dst_port)
 	udph->source = htons(UDP_SOURCE_PORT);
 	udph->dest = htons(dst_port);
 	udph->len = htons(sizeof(*udph) + UDP_PAYLOAD_BYTES);
+	memset(udph + 1, 0xAA, UDP_PAYLOAD_BYTES);
 	udph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
 					 ntohs(udph->len), IPPROTO_UDP, 0);
 
-	memset(udph + 1, 0xAA, UDP_PAYLOAD_BYTES);
-
-	meta->flags |= XDP_TXMD_FLAGS_CHECKSUM;
-	meta->request.csum_start = sizeof(*eth) + sizeof(*iph);
-	meta->request.csum_offset = offsetof(struct udphdr, check);
+	for (i = 0, offset = 0; i < num; i++) {
+		u32 len;
+
+		if (num == 1)
+			len = pkt_len;
+		else if (!i)
+			len = first_len;
+		else if (i == num - 1)
+			len = pkt_len - offset;
+		else
+			len = (pkt_len - first_len) / (num - 1);
+
+		addr = (idx + i) % (UMEM_NUM / 2) * UMEM_FRAME_SIZE;
+		tx_desc = xsk_ring_prod__tx_desc(&xsk->tx, idx + i);
+		tx_desc->addr = addr + sizeof(*meta);
+		tx_desc->len = len;
+		tx_desc->options = i ? 0 : XDP_TX_METADATA;
+		if (i + 1 < num)
+			tx_desc->options |= XDP_PKT_CONTD;
+		printf("%p: tx_desc[%u]->addr=%llx\n", xsk, idx + i, tx_desc->addr);
+		data = xsk_umem__get_data(xsk->umem_area, tx_desc->addr);
+		if (!i) {
+			meta = data - sizeof(*meta);
+			memset(meta, 0, sizeof(*meta));
+			meta->flags = XDP_TXMD_FLAGS_TIMESTAMP | XDP_TXMD_FLAGS_CHECKSUM;
+			meta->request.csum_start = sizeof(*eth) + sizeof(*iph);
+			meta->request.csum_offset = offsetof(struct udphdr, check);
+		}
+		memcpy(data, pkt + offset, len);
+		offset += len;
+	}
 
-	tx_desc->len = sizeof(*eth) + sizeof(*iph) + sizeof(*udph) + UDP_PAYLOAD_BYTES;
-	tx_desc->options |= XDP_TX_METADATA;
-	xsk_ring_prod__submit(&xsk->tx, 1);
+	xsk_ring_prod__submit(&xsk->tx, num);
 
 	ret = sendto(xsk_socket__fd(xsk->socket), NULL, 0, MSG_DONTWAIT, NULL, 0);
 	if (!ASSERT_GE(ret, 0, "sendto"))
@@ -224,24 +256,27 @@ static int generate_packet_inet(void)
 	return err;
 }
 
-static void complete_tx(struct xsk *xsk)
+static void complete_tx(struct xsk *xsk, u32 num)
 {
 	struct xsk_tx_metadata *meta;
 	__u64 addr;
 	void *data;
 	__u32 idx;
-
-	if (ASSERT_EQ(xsk_ring_cons__peek(&xsk->comp, 1, &idx), 1, "xsk_ring_cons__peek")) {
-		addr = *xsk_ring_cons__comp_addr(&xsk->comp, idx);
-
-		printf("%p: complete tx idx=%u addr=%llx\n", xsk, idx, addr);
-
-		data = xsk_umem__get_data(xsk->umem_area, addr);
-		meta = data - sizeof(struct xsk_tx_metadata);
-
-		ASSERT_NEQ(meta->completion.tx_timestamp, 0, "tx_timestamp");
-
-		xsk_ring_cons__release(&xsk->comp, 1);
+	__u32 i;
+
+	if (ASSERT_EQ(xsk_ring_cons__peek(&xsk->comp, num, &idx), num,
+		      "xsk_ring_cons__peek")) {
+		for (i = 0; i < num; i++) {
+			addr = *xsk_ring_cons__comp_addr(&xsk->comp, idx + i);
+			printf("%p: complete tx idx=%u addr=%llx\n", xsk, idx + i, addr);
+
+			if (i == 0) {
+				data = xsk_umem__get_data(xsk->umem_area, addr);
+				meta = data - sizeof(*meta);
+				ASSERT_NEQ(meta->completion.tx_timestamp, 0, "tx_timestamp");
+			}
+		}
+		xsk_ring_cons__release(&xsk->comp, num);
 	}
 }
 
@@ -325,7 +360,7 @@ static int verify_xsk_metadata(struct xsk *xsk, bool sent_from_af_xdp)
 	ASSERT_EQ(meta->rx_hash_type, 0, "rx_hash_type");
 
 	/* checksum offload */
-	ASSERT_EQ(udph->check, htons(0x721c), "csum");
+	ASSERT_EQ(udph->check, htons(0x1c8f), "csum");
 
 done:
 	xsk_ring_cons__release(&xsk->rx, 1);
@@ -361,6 +396,7 @@ void test_xdp_metadata(void)
 	int retries = 10;
 	int rx_ifindex;
 	int tx_ifindex;
+	unsigned int i;
 	int sock_fd;
 	int ret;
 
@@ -398,7 +434,7 @@ void test_xdp_metadata(void)
 
 	/* Setup separate AF_XDP for RX interface. */
 
-	ret = open_xsk(rx_ifindex, &rx_xsk);
+	ret = open_xsk(rx_ifindex, &rx_xsk, XDP_COPY);
 	if (!ASSERT_OK(ret, "open_xsk(RX_NAME)"))
 		goto out;
 
@@ -462,27 +498,28 @@ void test_xdp_metadata(void)
 
 	/* Setup separate AF_XDP for TX interface nad send packet to the RX socket. */
 	tx_ifindex = if_nametoindex(TX_NAME);
-	ret = open_xsk(tx_ifindex, &tx_xsk);
+	ret = open_xsk(tx_ifindex, &tx_xsk, XDP_COPY | XDP_USE_SG);
 	if (!ASSERT_OK(ret, "open_xsk(TX_NAME)"))
 		goto out;
 
-	if (!ASSERT_GE(generate_packet(&tx_xsk, AF_XDP_CONSUMER_PORT), 0,
-		       "generate AF_XDP_CONSUMER_PORT"))
-		goto out;
+	for (i = 1; i <= MAX_TX_DESCS; i++) {
+		if (!ASSERT_GE(generate_packet(&tx_xsk, AF_XDP_CONSUMER_PORT, i), 0,
+			       "generate AF_XDP packet"))
+			goto out;
 
-	switch_ns_to_rx(&tok);
-	if (!ASSERT_OK_PTR(tok, "setns rx"))
-		goto out;
+		switch_ns_to_rx(&tok);
+		if (!ASSERT_OK_PTR(tok, "setns rx"))
+			goto out;
 
-	/* Verify packet sent from AF_XDP has proper metadata. */
-	if (!ASSERT_GE(verify_xsk_metadata(&rx_xsk, true), 0,
-		       "verify_xsk_metadata"))
-		goto out;
+		if (!ASSERT_GE(verify_xsk_metadata(&rx_xsk, true), 0,
+			       "verify xsk metadata"))
+			goto out;
 
-	switch_ns_to_tx(&tok);
-	if (!ASSERT_OK_PTR(tok, "setns tx"))
-		goto out;
-	complete_tx(&tx_xsk);
+		switch_ns_to_tx(&tok);
+		if (!ASSERT_OK_PTR(tok, "setns tx"))
+			goto out;
+		complete_tx(&tx_xsk, i);
+	}
 
 	/* Now check metadata of packet, generated with network stack */
 	if (!ASSERT_GE(generate_packet_inet(), 0, "generate UDP packet"))
@@ -518,7 +555,7 @@ void test_xdp_metadata(void)
 		goto out;
 
 	/* Send packet to trigger . */
-	if (!ASSERT_GE(generate_packet(&tx_xsk, AF_XDP_CONSUMER_PORT), 0,
+	if (!ASSERT_GE(generate_packet(&tx_xsk, AF_XDP_CONSUMER_PORT, 1), 0,
 		       "generate freplace packet"))
 		goto out;
 
-- 
2.39.5 (Apple Git-154)

Re: [PATCH net v4 1/2] xsk: defer software checksum until the skb is complete

[Jason Xing]

On Fri, Oct 30, 2026 at 10:33 AM Rongzheng Wang <testwrzwrz@gmail.com> wrote:
>
> When XDP_UMEM_TX_SW_CSUM is used with multi-buffer TX, xsk_skb_metadata()
> calls skb_checksum_help() while only the first descriptor has been added to
> the skb. The checksum therefore covers just the first fragment, and
> skb_checksum_help() sets ip_summed to CHECKSUM_NONE, so the remaining
> fragments are never included in the checksum.
>
> Move the software checksum calculation to the point where the last
> descriptor has been added to the skb.
>
> On a checksum failure the packet is now dropped instead of being returned
> to the application for a retry. This path is not reachable with the
> existing metadata validation, but the behavior change is intentional.
>
> Fixes: 11614723af26 ("xsk: Add option to calculate TX checksum in SW")
> Cc: stable@vger.kernel.org
> Signed-off-by: Rongzheng Wang <testwrzwrz@gmail.com>
> ---
>  net/xdp/xsk.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 33475b180ea6..aa6b010192ad 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -1011,14 +1011,6 @@ static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
>                 skb->csum_start = hr + csum_start;
>                 skb->csum_offset = csum_offset;
>                 skb->ip_summed = CHECKSUM_PARTIAL;
> -
> -               if (unlikely(pool->tx_sw_csum)) {
> -                       int err;
> -
> -                       err = skb_checksum_help(skb);
> -                       if (err)
> -                               return err;
> -               }
>         }
>
>         if (flags & XDP_TXMD_FLAGS_LAUNCH_TIME)
> @@ -1030,6 +1022,15 @@ static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
>         return 0;
>  }
>
> +static int xsk_skb_sw_csum(struct sk_buff *skb, struct xsk_buff_pool *pool)
> +{
> +       if (unlikely(pool->tx_sw_csum &&
> +                    skb->ip_summed == CHECKSUM_PARTIAL))
> +               return skb_checksum_help(skb);
> +
> +       return 0;
> +}
> +
>  static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
>                                               struct xdp_desc *desc)
>  {
> @@ -1275,6 +1276,12 @@ static int __xsk_generic_xmit(struct sock *sk)
>                 if (xp_mb_desc(&desc)) {
>                         xs->skb = skb;
>                         continue;
> +               } else {

Why are you still using this 'else' as you already posted a new one?
Please take a look at my previous feedback.

And please carry the reviewed-by tags through the version next time.

Thanks,
Jason


> +                       err = xsk_skb_sw_csum(skb, xs->pool);
> +                       if (unlikely(err)) {
> +                               xsk_drop_skb(skb);
> +                               goto out;
> +                       }
>                 }
>
>                 err = __dev_direct_xmit(skb, xs->queue_id);
> --
> 2.39.5 (Apple Git-154)
>

Re: [PATCH net v4 1/2] xsk: defer software checksum until the skb is complete

[Rongzheng Wang]

Hi Jason,

Thanks for the review. This was a timing race, not intentional: your v3
reply (the Reviewed-by plus the else-branch nit) arrived at 10:30, while
v4 was sent at 10:32 and had already been prepared without it. I should
have checked the thread before sending.

I'll send a v5 as a new thread with the unnecessary else removed, exactly
as you suggested, and your Reviewed-by carried on patch 1. Patch 2 is
unchanged.

Thanks,
Rongzheng

Re: [PATCH net v4 1/2] xsk: defer software checksum until the skb is complete

[Jason Xing]

On Fri, Oct 30, 2026 at 4:40 PM Rongzheng Wang <testwrzwrz@gmail.com> wrote:
>
> Hi Jason,
>
> Thanks for the review. This was a timing race, not intentional: your v3
> reply (the Reviewed-by plus the else-branch nit) arrived at 10:30, while
> v4 was sent at 10:32 and had already been prepared without it. I should
> have checked the thread before sending.
>
> I'll send a v5 as a new thread with the unnecessary else removed, exactly
> as you suggested, and your Reviewed-by carried on patch 1. Patch 2 is
> unchanged.

Thanks.

Since I'm not that familiar with the selftest, I would not comment on
that one. I believe Maciej and Bjorn will give their opinions, so I
highly recommend you to wait for their feedback on the selftest patch
:)

Thanks,
Jason

Re: [PATCH net v4 1/2] xsk: defer software checksum until the skb is complete

[Rongzheng Wang]

Hi Jason,

Makes sense, thanks. I'll wait for their feedback on the selftest patch
before sending v5, and fold it in together with the rest.

Thanks,
Rongzheng

Re: [PATCH net v4 1/2] xsk: defer software checksum until the skb is complete

[Maciej Fijalkowski]

On Fri, Oct 30, 2026 at 05:46:33PM +0800, Rongzheng Wang wrote:
> Hi Jason,
> 
> Makes sense, thanks. I'll wait for their feedback on the selftest patch
> before sending v5, and fold it in together with the rest.

Can you stop removing all the previous conversations and rather reply
inline to related paragraphs?

I don't think v5 is necessary. And as Jason said you received 3 review/ack
tags and none of them got included in v4.

> 
> Thanks,
> Rongzheng

Re: [PATCH net v4 2/2] selftests/bpf: cover AF_XDP multi-buffer TX software checksum

[Maciej Fijalkowski]

On Fri, Oct 30, 2026 at 10:32:48AM +0800, Rongzheng Wang wrote:
> Extend test_xdp_metadata to send the same UDP packet using every descriptor
> count from 1 through MAX_TX_DESCS (MAX_SKB_FRAGS + 1), and to run the
> existing metadata and checksum verification for each case.
> 
> Build the packet once and write it into the TX descriptors in a loop
> instead of using a flat two-descriptor layout. Keep the first descriptor
> large enough to cover the checksum offset; the remaining payload is split
> across the continuation descriptors, with the last descriptor taking the
> remaining length.
> 
> Reuse the existing AF_XDP setup, packet generation, completion, and receive
> helpers in xdp_metadata.c instead of adding a separate test harness.
> 
> Signed-off-by: Rongzheng Wang <testwrzwrz@gmail.com>

Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

> ---
>  .../selftests/bpf/prog_tests/xdp_metadata.c   | 147 +++++++++++-------
>  1 file changed, 92 insertions(+), 55 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
> index 5c31054ad4a4..17da0a145666 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
> @@ -18,12 +18,15 @@
>  #define TX_NAME "veTX"
>  #define RX_NAME "veRX"
>  
> -#define UDP_PAYLOAD_BYTES 4
> +#define UDP_PAYLOAD_BYTES 32
> +
> +#define MAX_SKB_FRAGS 17 /* minimum CONFIG_MAX_SKB_FRAGS */
> +#define MAX_TX_DESCS (MAX_SKB_FRAGS + 1)
>  
>  #define UDP_SOURCE_PORT 1234
>  #define AF_XDP_CONSUMER_PORT 8080
>  
> -#define UMEM_NUM 16
> +#define UMEM_NUM 64
>  #define UMEM_FRAME_SIZE XSK_UMEM__DEFAULT_FRAME_SIZE
>  #define UMEM_SIZE (UMEM_FRAME_SIZE * UMEM_NUM)
>  #define XDP_FLAGS XDP_FLAGS_DRV_MODE
> @@ -56,13 +59,13 @@ struct xsk {
>  	struct xsk_socket *socket;
>  };
>  
> -static int open_xsk(int ifindex, struct xsk *xsk)
> +static int open_xsk(int ifindex, struct xsk *xsk, __u32 bind_flags)
>  {
>  	int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
>  	const struct xsk_socket_config socket_config = {
>  		.rx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
>  		.tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
> -		.bind_flags = XDP_COPY,
> +		.bind_flags = bind_flags,
>  	};
>  	const struct xsk_umem_config umem_config = {
>  		.fill_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
> @@ -133,31 +136,35 @@ static void close_xsk(struct xsk *xsk)
>  	munmap(xsk->umem_area, UMEM_SIZE);
>  }
>  
> -static int generate_packet(struct xsk *xsk, __u16 dst_port)
> +static int generate_packet(struct xsk *xsk, __u16 dst_port, u32 num)
>  {
>  	struct xsk_tx_metadata *meta;
>  	struct xdp_desc *tx_desc;
>  	struct udphdr *udph;
>  	struct ethhdr *eth;
>  	struct iphdr *iph;
> +	u16 first_len;
> +	u16 pkt_len;
> +	u8 pkt[256];
> +	u32 offset;
>  	void *data;
>  	__u32 idx;
> +	u32 addr;
>  	int ret;
> +	u32 i;
>  
> -	ret = xsk_ring_prod__reserve(&xsk->tx, 1, &idx);
> -	if (!ASSERT_EQ(ret, 1, "xsk_ring_prod__reserve"))
> +	pkt_len = sizeof(*eth) + sizeof(*iph) + sizeof(*udph) + UDP_PAYLOAD_BYTES;
> +	first_len = sizeof(*eth) + sizeof(*iph) + sizeof(*udph);
> +	if (!num || num > MAX_TX_DESCS ||
> +	    (num > 1 && pkt_len - first_len < num - 1))
>  		return -1;
>  
> -	tx_desc = xsk_ring_prod__tx_desc(&xsk->tx, idx);
> -	tx_desc->addr = idx % (UMEM_NUM / 2) * UMEM_FRAME_SIZE + sizeof(struct xsk_tx_metadata);
> -	printf("%p: tx_desc[%u]->addr=%llx\n", xsk, idx, tx_desc->addr);
> -	data = xsk_umem__get_data(xsk->umem_area, tx_desc->addr);
> -
> -	meta = data - sizeof(struct xsk_tx_metadata);
> -	memset(meta, 0, sizeof(*meta));
> -	meta->flags = XDP_TXMD_FLAGS_TIMESTAMP;
> +	ret = xsk_ring_prod__reserve(&xsk->tx, num, &idx);
> +	if (!ASSERT_EQ(ret, num, "xsk_ring_prod__reserve"))
> +		return -1;
>  
> -	eth = data;
> +	memset(pkt, 0, sizeof(pkt));
> +	eth = (void *)pkt;
>  	iph = (void *)(eth + 1);
>  	udph = (void *)(iph + 1);
>  
> @@ -180,18 +187,43 @@ static int generate_packet(struct xsk *xsk, __u16 dst_port)
>  	udph->source = htons(UDP_SOURCE_PORT);
>  	udph->dest = htons(dst_port);
>  	udph->len = htons(sizeof(*udph) + UDP_PAYLOAD_BYTES);
> +	memset(udph + 1, 0xAA, UDP_PAYLOAD_BYTES);
>  	udph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
>  					 ntohs(udph->len), IPPROTO_UDP, 0);
>  
> -	memset(udph + 1, 0xAA, UDP_PAYLOAD_BYTES);
> -
> -	meta->flags |= XDP_TXMD_FLAGS_CHECKSUM;
> -	meta->request.csum_start = sizeof(*eth) + sizeof(*iph);
> -	meta->request.csum_offset = offsetof(struct udphdr, check);
> +	for (i = 0, offset = 0; i < num; i++) {
> +		u32 len;
> +
> +		if (num == 1)
> +			len = pkt_len;
> +		else if (!i)
> +			len = first_len;
> +		else if (i == num - 1)
> +			len = pkt_len - offset;
> +		else
> +			len = (pkt_len - first_len) / (num - 1);
> +
> +		addr = (idx + i) % (UMEM_NUM / 2) * UMEM_FRAME_SIZE;
> +		tx_desc = xsk_ring_prod__tx_desc(&xsk->tx, idx + i);
> +		tx_desc->addr = addr + sizeof(*meta);
> +		tx_desc->len = len;
> +		tx_desc->options = i ? 0 : XDP_TX_METADATA;
> +		if (i + 1 < num)
> +			tx_desc->options |= XDP_PKT_CONTD;
> +		printf("%p: tx_desc[%u]->addr=%llx\n", xsk, idx + i, tx_desc->addr);
> +		data = xsk_umem__get_data(xsk->umem_area, tx_desc->addr);
> +		if (!i) {
> +			meta = data - sizeof(*meta);
> +			memset(meta, 0, sizeof(*meta));
> +			meta->flags = XDP_TXMD_FLAGS_TIMESTAMP | XDP_TXMD_FLAGS_CHECKSUM;
> +			meta->request.csum_start = sizeof(*eth) + sizeof(*iph);
> +			meta->request.csum_offset = offsetof(struct udphdr, check);
> +		}
> +		memcpy(data, pkt + offset, len);
> +		offset += len;
> +	}
>  
> -	tx_desc->len = sizeof(*eth) + sizeof(*iph) + sizeof(*udph) + UDP_PAYLOAD_BYTES;
> -	tx_desc->options |= XDP_TX_METADATA;
> -	xsk_ring_prod__submit(&xsk->tx, 1);
> +	xsk_ring_prod__submit(&xsk->tx, num);
>  
>  	ret = sendto(xsk_socket__fd(xsk->socket), NULL, 0, MSG_DONTWAIT, NULL, 0);
>  	if (!ASSERT_GE(ret, 0, "sendto"))
> @@ -224,24 +256,27 @@ static int generate_packet_inet(void)
>  	return err;
>  }
>  
> -static void complete_tx(struct xsk *xsk)
> +static void complete_tx(struct xsk *xsk, u32 num)
>  {
>  	struct xsk_tx_metadata *meta;
>  	__u64 addr;
>  	void *data;
>  	__u32 idx;
> -
> -	if (ASSERT_EQ(xsk_ring_cons__peek(&xsk->comp, 1, &idx), 1, "xsk_ring_cons__peek")) {
> -		addr = *xsk_ring_cons__comp_addr(&xsk->comp, idx);
> -
> -		printf("%p: complete tx idx=%u addr=%llx\n", xsk, idx, addr);
> -
> -		data = xsk_umem__get_data(xsk->umem_area, addr);
> -		meta = data - sizeof(struct xsk_tx_metadata);
> -
> -		ASSERT_NEQ(meta->completion.tx_timestamp, 0, "tx_timestamp");
> -
> -		xsk_ring_cons__release(&xsk->comp, 1);
> +	__u32 i;
> +
> +	if (ASSERT_EQ(xsk_ring_cons__peek(&xsk->comp, num, &idx), num,
> +		      "xsk_ring_cons__peek")) {
> +		for (i = 0; i < num; i++) {
> +			addr = *xsk_ring_cons__comp_addr(&xsk->comp, idx + i);
> +			printf("%p: complete tx idx=%u addr=%llx\n", xsk, idx + i, addr);
> +
> +			if (i == 0) {
> +				data = xsk_umem__get_data(xsk->umem_area, addr);
> +				meta = data - sizeof(*meta);
> +				ASSERT_NEQ(meta->completion.tx_timestamp, 0, "tx_timestamp");
> +			}
> +		}
> +		xsk_ring_cons__release(&xsk->comp, num);
>  	}
>  }
>  
> @@ -325,7 +360,7 @@ static int verify_xsk_metadata(struct xsk *xsk, bool sent_from_af_xdp)
>  	ASSERT_EQ(meta->rx_hash_type, 0, "rx_hash_type");
>  
>  	/* checksum offload */
> -	ASSERT_EQ(udph->check, htons(0x721c), "csum");
> +	ASSERT_EQ(udph->check, htons(0x1c8f), "csum");
>  
>  done:
>  	xsk_ring_cons__release(&xsk->rx, 1);
> @@ -361,6 +396,7 @@ void test_xdp_metadata(void)
>  	int retries = 10;
>  	int rx_ifindex;
>  	int tx_ifindex;
> +	unsigned int i;
>  	int sock_fd;
>  	int ret;
>  
> @@ -398,7 +434,7 @@ void test_xdp_metadata(void)
>  
>  	/* Setup separate AF_XDP for RX interface. */
>  
> -	ret = open_xsk(rx_ifindex, &rx_xsk);
> +	ret = open_xsk(rx_ifindex, &rx_xsk, XDP_COPY);
>  	if (!ASSERT_OK(ret, "open_xsk(RX_NAME)"))
>  		goto out;
>  
> @@ -462,27 +498,28 @@ void test_xdp_metadata(void)
>  
>  	/* Setup separate AF_XDP for TX interface nad send packet to the RX socket. */
>  	tx_ifindex = if_nametoindex(TX_NAME);
> -	ret = open_xsk(tx_ifindex, &tx_xsk);
> +	ret = open_xsk(tx_ifindex, &tx_xsk, XDP_COPY | XDP_USE_SG);
>  	if (!ASSERT_OK(ret, "open_xsk(TX_NAME)"))
>  		goto out;
>  
> -	if (!ASSERT_GE(generate_packet(&tx_xsk, AF_XDP_CONSUMER_PORT), 0,
> -		       "generate AF_XDP_CONSUMER_PORT"))
> -		goto out;
> +	for (i = 1; i <= MAX_TX_DESCS; i++) {
> +		if (!ASSERT_GE(generate_packet(&tx_xsk, AF_XDP_CONSUMER_PORT, i), 0,
> +			       "generate AF_XDP packet"))
> +			goto out;
>  
> -	switch_ns_to_rx(&tok);
> -	if (!ASSERT_OK_PTR(tok, "setns rx"))
> -		goto out;
> +		switch_ns_to_rx(&tok);
> +		if (!ASSERT_OK_PTR(tok, "setns rx"))
> +			goto out;
>  
> -	/* Verify packet sent from AF_XDP has proper metadata. */
> -	if (!ASSERT_GE(verify_xsk_metadata(&rx_xsk, true), 0,
> -		       "verify_xsk_metadata"))
> -		goto out;
> +		if (!ASSERT_GE(verify_xsk_metadata(&rx_xsk, true), 0,
> +			       "verify xsk metadata"))
> +			goto out;
>  
> -	switch_ns_to_tx(&tok);
> -	if (!ASSERT_OK_PTR(tok, "setns tx"))
> -		goto out;
> -	complete_tx(&tx_xsk);
> +		switch_ns_to_tx(&tok);
> +		if (!ASSERT_OK_PTR(tok, "setns tx"))
> +			goto out;
> +		complete_tx(&tx_xsk, i);
> +	}
>  
>  	/* Now check metadata of packet, generated with network stack */
>  	if (!ASSERT_GE(generate_packet_inet(), 0, "generate UDP packet"))
> @@ -518,7 +555,7 @@ void test_xdp_metadata(void)
>  		goto out;
>  
>  	/* Send packet to trigger . */
> -	if (!ASSERT_GE(generate_packet(&tx_xsk, AF_XDP_CONSUMER_PORT), 0,
> +	if (!ASSERT_GE(generate_packet(&tx_xsk, AF_XDP_CONSUMER_PORT, 1), 0,
>  		       "generate freplace packet"))
>  		goto out;
>  
> -- 
> 2.39.5 (Apple Git-154)
> 

Re: [PATCH net v4 1/2] xsk: defer software checksum until the skb is complete

[Rongzheng Wang]

Thanks for the review, the suggestions, and the Reviewed-by on 2/2.

> Can you stop removing all the previous conversations and rather reply
> inline to related paragraphs?

Sorry, I'll reply inline from now on.

> I don't think v5 is necessary. And as Jason said you received 3 review/ack
> tags and none of them got included in v4.

You're right. v4 went out before Jason's tag arrived, and I missed the
other two tags from v3. Since you don't think v5 is necessary, I won't
send one; if there is another revision, I'll carry all the tags.

I learned a lot from this patch and from the review feedback. I'll be more
careful with future patches and wait longer for reviewers' feedback before
sending the next version.

Re: [PATCH net v4 2/2] selftests/bpf: cover AF_XDP multi-buffer TX software checksum

[Stanislav Fomichev]

On 10/30, Rongzheng Wang wrote:
> Extend test_xdp_metadata to send the same UDP packet using every descriptor
> count from 1 through MAX_TX_DESCS (MAX_SKB_FRAGS + 1), and to run the
> existing metadata and checksum verification for each case.
> 
> Build the packet once and write it into the TX descriptors in a loop
> instead of using a flat two-descriptor layout. Keep the first descriptor
> large enough to cover the checksum offset; the remaining payload is split
> across the continuation descriptors, with the last descriptor taking the
> remaining length.
> 
> Reuse the existing AF_XDP setup, packet generation, completion, and receive
> helpers in xdp_metadata.c instead of adding a separate test harness.
> 
> Signed-off-by: Rongzheng Wang <testwrzwrz@gmail.com>
> ---
>  .../selftests/bpf/prog_tests/xdp_metadata.c   | 147 +++++++++++-------
>  1 file changed, 92 insertions(+), 55 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
> index 5c31054ad4a4..17da0a145666 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
> @@ -18,12 +18,15 @@
>  #define TX_NAME "veTX"
>  #define RX_NAME "veRX"
>  
> -#define UDP_PAYLOAD_BYTES 4
> +#define UDP_PAYLOAD_BYTES 32
> +

[..]

> +#define MAX_SKB_FRAGS 17 /* minimum CONFIG_MAX_SKB_FRAGS */

Any reason you're not reading the real value from
/proc/sys/net/core/max_skb_frags ?