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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/bpf: generate_packet() copies ASCII MAC strings into ethhdr
--

commit 84305185f8b937f13eaa038de90ba93344b90447
Author: Rongzheng Wang <testwrzwrz@gmail.com>

selftests/bpf: cover AF_XDP multi-buffer TX software checksum

This commit extends test_xdp_metadata to send the test UDP packet both as a
single TX descriptor and split across two TX descriptors, verifying AF_XDP
metadata and software checksum calculation for both cases.

> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
> index 5c31054ad4a4c..1cf70fb800fba 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
[ ... ]
> @@ -133,36 +135,36 @@ 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, u16 split_len)
>  {
>  	struct xsk_tx_metadata *meta;
>  	struct xdp_desc *tx_desc;
>  	struct udphdr *udph;
>  	struct ethhdr *eth;
>  	struct iphdr *iph;
> +	u16 pkt_len;
> +	u32 num, addr;
>  	void *data;
>  	__u32 idx;
> +	u8 pkt[256];
>  	int ret;
>  
> -	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;
> +	if (!ASSERT_LT(split_len, pkt_len, "split_len"))
>  		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;
> +	num = split_len ? 2 : 1;
> +	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);
>  
> -	memcpy(eth->h_dest, "\x00\x00\x00\x00\x00\x02", ETH_ALEN);
> -	memcpy(eth->h_source, "\x00\x00\x00\x00\x00\x01", ETH_ALEN);
> +	memcpy(eth->h_dest, RX_MAC, ETH_ALEN);
> +	memcpy(eth->h_source, TX_MAC, ETH_ALEN);

[Severity: Medium]
Does this populate eth->h_dest and eth->h_source with ASCII characters
instead of the binary MAC addresses?

In tools/testing/selftests/bpf/prog_tests/xdp_metadata.c, TX_MAC and RX_MAC
are defined as colon-separated ASCII strings for iproute2 commands:

#define TX_MAC "00:00:00:00:00:01"
#define RX_MAC "00:00:00:00:00:02"

When test_xdp_metadata() calls generate_packet(), copying ETH_ALEN (6)
bytes from RX_MAC and TX_MAC writes the literal string "00:00:"
(30:30:3a:30:30:3a) into both eth->h_dest and eth->h_source, rather than
the 6-byte binary MAC addresses "\x00\x00\x00\x00\x00\x02" and
"\x00\x00\x00\x00\x00\x01".

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024141157.59301-1-testwrzwrz@gmail.com?part=2