Re: [PATCH v4 net 6/6] selftests/xsk: account reclaimed invalid Tx descriptors
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-07-23 16:29:57
Also in:
bpf
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. --- selftests/xsk: account reclaimed invalid Tx descriptors This patch updates the AF_XDP selftests to count every Tx descriptor submitted (including invalid ones) in valid_frags so the completion ring accounting matches the kernel's new behavior of returning invalid Tx descriptors through the completion ring. It also adjusts verbatim fragment counting and adds explicit end fragments after invalid middle descriptors.
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c index de17dd48f176..38ce6060b8fa 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -665,11 +665,11 @@ static struct pkt_stream *__pkt_stream_generate_custom(struct ifobject *ifobj, s if (!frame->valid || !pkt_continues(frame->options)) payload++; } else { - if (frame->valid) + if (frame->valid) { len += frame->len; - if (frame->valid && pkt_continues(frame->options)) - continue; - + if (pkt_continues(frame->options)) + continue; + } pkt->pkt_nb = pkt_nb; pkt->len = len; pkt->valid = frame->valid;
[Low] The commit message says "Update custom stream generation so an invalid middle fragment terminates the generated Rx packet while Tx completion accounting still covers the complete invalid packet." Does this hunk actually change any behavior? Walking through the four (valid, pkt_continues) combinations for the non-verbatim branch shows the same control flow before and after: valid=1, continues=1 -> len += frame->len; continue valid=1, continues=0 -> len += frame->len; fall through to finalize valid=0, continues=1 -> fall through to finalize valid=0, continues=0 -> fall through to finalize The Rx-packet termination described in the commit message appears to be driven by the pkts[] additions and by removing the !pkt->valid early exit in pkt_nb_frags(), not by this hunk. Would it be clearer to describe this change as a pure refactor of __pkt_stream_generate_custom(), or drop it from this commit? [ ... ]
quoted hunk ↗ jump to hunk
@@ -2099,13 +2098,16 @@ int testapp_invalid_desc_mb(struct test_spec *test) {0, 0, 0, false, 0}, /* Invalid address in the second frame */ {0, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD}, - {umem_sz, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD}, + {umem_sz * 2, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD}, + {0, MIN_PKT_SIZE, 0, false, 0},
[Low] Would it be worth calling out the umem_sz -> umem_sz * 2 change in the commit message? In shared_umem mode the UMEM size is doubled before registration: umem_sz *= 2; so a descriptor address of umem_sz can fall inside the valid UMEM range for the second socket. Bumping this to umem_sz * 2 ensures the address is out of range regardless of shared_umem, which is a nice correctness tweak, but bundling it with the accounting change makes the intent less obvious and complicates bisection.
/* Invalid len in the middle */
{0, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
{0, XSK_UMEM__INVALID_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
+ {0, MIN_PKT_SIZE, 0, false, 0},
/* Invalid options in the middle */
{0, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
{0, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XSK_DESC__INVALID_OPTION},
+ {0, MIN_PKT_SIZE, 0, false, 0},
/* Transmit 2 frags, receive 3 */
{0, XSK_UMEM__MAX_FRAME_SIZE, 0, true, XDP_PKT_CONTD},
{0, XSK_UMEM__MAX_FRAME_SIZE, 0, true, 0},[ ... ]