|WARNING| [2/2] app/testpmd: fix runtime config of Rx split

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/166186

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-06-25

# DPDK Patch Review

## Patch 1/2: dts: simplify packet check in Rx split

### Correctness Issues

**Error: Resource leak on exception path**

In `_start_and_verify()`, if `testpmd.start()` raises an exception, the testpmd instance may be left in an inconsistent state. However, reviewing the context manager usage in the test methods, the testpmd instance is properly managed by `_create_testpmd()` context manager, so cleanup should occur. This is acceptable.

**No other correctness issues identified.**

### Code Style

**Warning: Type annotation uses union syntax without future import**

The function signature uses `Callable[[bytes], bytes] | bytes | int | None` which is Python 3.10+ union syntax. If DPDK DTS supports Python <3.10, this should use `Union[Callable[[bytes], bytes], bytes, int, None]` from typing, or add `from __future__ import annotations`. Verify the project's minimum Python version.

### API and Documentation

**Info: Docstring update recommended**

The `_start_and_verify()` method signature changed significantly but the docstring was not updated. Consider adding a parameter description for the new `expected` parameter explaining the different types it accepts (None for full packet, int for header length, bytes for exact match, Callable for custom transform).

**No release notes required** - This is a test-only change.

---

## Patch 2/2: app/testpmd: fix runtime config of Rx split

### Correctness Issues

**No correctness bugs identified.** The fix correctly removes the overly strict condition that prevented using multiple mempools without split configuration.

### Code Style

**No style issues identified.** The change is minimal and follows existing code patterns.

### Logic Analysis

The original code incorrectly required `multi_rx_mempool == 0 && (rx_pkt_nb_segs > 1 || mbuf_data_size_n > 1)` before entering the multi-segment configuration block. This prevented users from:
1. Starting with `--mbuf-size 512,0` (multiple mempools)
2. Running without split
3. Then later configuring split at runtime

The fix changes the condition to `multi_rx_mempool == 0 && rx_pkt_nb_segs > 1`, which correctly checks only whether split is actually configured, not whether multiple mempool sizes were specified. This allows the use case described in the commit message.

### API and Documentation

**Release notes required** - This fixes a regression (Bugzilla ID: 1956) and should be documented in the release notes under "Fixed Issues" or "Bug Fixes" section.

The commit message references the fix should go in release notes but does not show the actual release notes update. Verify that `doc/guides/rel_notes/release_26_03.rst` (or the appropriate current release file) includes this fix.

### Testing

**Good:** The patch adds a DTS test (`selective_rx_runtime_config`) that validates the fixed use case. This test:
- Starts with two mbuf pools but no split configured
- Verifies initial startup succeeds (tests the fix)
- Stops, configures split at runtime, restarts
- Verifies split works correctly after runtime config

---

## Summary

### Patch 1/2
- **Warnings:** Type annotation may need adjustment for Python version compatibility
- **Info:** Docstring could be more complete
- No errors or correctness bugs

### Patch 2/2
- **Warning:** Missing release notes update (commit message mentions it but no file shown)
- No errors or correctness bugs
- Logic fix is correct
- Test coverage is good

### Recommendations
1. Verify Python version compatibility for union type syntax in Patch 1
2. Update `_start_and_verify()` docstring to document the `expected` parameter
3. Confirm release notes were updated for Patch 2 (not shown in the diff)