Re: [PATCH v8 9/9] dts: add selective Rx tests

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu,  4 Jun 2026 21:31:01 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> Add TestSuite_rx_split with 7 test cases:
> - 3 positive: headers only, payload only, two non-contiguous segments
> - 4 negative: missing offload flag, out-of-range, overlap, all-discard
> 
> Add selective Rx capability detection via testpmd "show port info".
> 
> The test suite could be completed later for the basic buffer split
> configuration based on offsets or protocols.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---

AI review found:
Patch 9 (dts: add selective Rx tests)

selective_rx_out_of_range expects a rejection that never happens, so the
negative test will fail. It configures a real segment plus an oversized
discard segment:

	rx_segments_length=[ETHER_IP_HDR_LEN, 20000],
	mbuf_size=[256, 0],

and expects start_all_ports() to fail. But an over-range length on a discard
segment is not rejected anywhere: rte_eth_rx_queue_check_split() does
"continue" for mp == NULL segments, so it never length-checks them, and
mlx5_rxq_new() clamps it:

	if (seg_len > tail_len)
		seg_len = qs_seg->mp != NULL ? buf_len - offset : tail_len;

The discard seg_len becomes the remaining frame length, the queue is built,
the port starts, and the test hits its fail().

Clamping an over-long discard to "the rest" is harmless (the bytes are
discarded anyway), so the cleanest fix is probably to drop or rework this
test rather than add a rejection path. If rejection is the intended
contract, it would have to be added for discard segments in patch 2 or
patch 6 -- a behavior choice, not a correctness requirement.

Minor: expressing a leading discard as --mbuf-size=0,... puts 0 at index 0,
and testpmd treats mbuf_data_size[0] as the primary pool size elsewhere (the
max_rx_pkt_len > mbuf_data_size[0] check, the default mbuf_pool_find(socket,
0)). Only bites an unusual config, but it is a latent foot-gun.