|WARNING| [RFC] dts: add eventdev pipeline performance testsuite

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-09-08

# DPDK Patch Review

## Summary
This patch adds a new DTS (DPDK Test Suite) test for eventdev pipeline performance. The code is Python (not C), so C-specific style rules don't apply. However, there are some logic and design issues to address.

---

## Errors

### 1. Incorrect resource cleanup on test failure
**File:** `dts/tests/TestSuite_eventdev_pipeline_perf.py:112-131`

The test creates an interactive shell (`eventdev_app`) but does not explicitly verify it started successfully before transmitting packets. If the application fails to start or crashes during execution, the context manager will clean up, but the test continues to call `_transmit()` which assumes the SUT is running the eventdev application.

**Impact:** The test could produce misleading results if the application is not actually running when traffic is sent.

**Suggested fix:**
Add a check after shell creation to verify the application started successfully, or handle potential failures in `_transmit()`. Consider adding application output validation.

```python
with self.sut_node.create_interactive_shell(
    DPDKShell,
    app_name="dpdk-test-eventdev",
    app_params=eventdev_params,
    eal_params=Params(vdev="event_sw0"),
    privileged=True,
) as eventdev_app:
    # Verify application started successfully
    # (implementation depends on DPDKShell interface)
    
    for frame_size in self.config.frame_sizes:
        # ... rest of test
```

---

## Warnings

### 1. Magic number repetitions parameter
**File:** `dts/tests/TestSuite_eventdev_pipeline_perf.py:114`

The `_transmit()` call uses `repetitions=5` hardcoded, while the method signature already defaults to 5. This redundant parameter makes the code harder to maintain if the default changes.

**Suggested fix:**
Remove the explicit `repetitions=5` argument or make it a configuration parameter:

```python
measured_mpps = round(self._transmit(frame_size=frame_size), 2)
```

Or add to Config class if repetitions need to vary:
```python
class Config(BaseConfig):
    # ... existing fields
    test_repetitions: int = 5
```

### 2. Results printed before verification
**File:** `dts/tests/TestSuite_eventdev_pipeline_perf.py:128-136`

The `_produce_stats_table()` function prints results and writes performance JSON before the `verify()` calls run. If any verification fails, the output may be misleading (showing results for a failed test).

**Suggested fix:**
Move the stats table generation after all verifications pass, or clearly indicate test pass/fail status in the output:

```python
self._produce_stats_table(results)

# Verify all results
for result in results:
    expected_baseline = result["expected_mpps"] * (1 - self.delta_tolerance)
    verify(
        result["pass"],
        f"Measured MPPS ({result['measured_mpps']:.2f}) for frame size {result['frame_size']} "
        f"is below expected baseline ({expected_baseline:.2f}).",
    )
```

Alternatively, update `_produce_stats_table` to include pass/fail status:
```python
header = f"{'Frame Size':>12} | {'TXD/RXD':>12} | {'Real MPPS':>12} | {'Expected MPPS':>14} | {'Status':>8}"
# ... in loop:
status = "PASS" if params['pass'] else "FAIL"
print(f"... | {status:>8}")
```

### 3. Missing documentation for delta_tolerance
**File:** `dts/tests/TestSuite_eventdev_pipeline_perf.py:42`

The `delta_tolerance` config field lacks a comment explaining what it represents (acceptable performance variance as a fraction).

**Suggested fix:**
```python
delta_tolerance: float = 0.05  # Acceptable variance (5%) from expected performance
```

### 4. Unused num_descriptors in test
**File:** `dts/tests/TestSuite_eventdev_pipeline_perf.py:122`

The `num_descriptors` field is added to results but never actually used or configured in the test. The test doesn't pass any descriptor-related parameters to the eventdev application.

**Suggested fix:**
Either remove `num_descriptors` from results if it's not configurable in this test, or add it as a parameter to `EventDevParams` if the application supports it:

```python
# If not used, remove from results dict:
results.append({
    "frame_size": frame_size,
    # Remove: "num_descriptors": self.config.num_descriptors,
    "measured_mpps": measured_mpps,
    "expected_mpps": expected_mpps,
    "pass": passed,
})
```

---

## Info

### 1. Expected performance values hardcoded
The `expected_mpps` dictionary in the Config class contains hardcoded performance baselines. Consider documenting the test platform/configuration these values were measured on, as performance varies significantly between systems.

**Suggestion:** Add a comment explaining the baseline system:
```python
# Expected MPPS values measured on [system description]
# e.g., "Intel Xeon Gold 6230, 2.1GHz, dual-socket"
expected_mpps: dict[int, float] = {
    64: 14.8,
    # ...
}
```

### 2. Consider parameterizing scheduling modes
The current test only covers the `"pipeline_atq"` test mode with atomic scheduling (`"a"`). The test name suggests future expansion to other modes. Consider making these parameters to enable testing parallel, ordered, and other scheduling modes without duplicating test code.

---

## Positive Observations

1. **Good use of dataclasses** for configuration management
2. **Proper use of context manager** for shell lifecycle
3. **Clear separation** of concerns (transmit, stats table, verification)
4. **Appropriate use of type hints** throughout

---

## Overall Assessment

The patch introduces a well-structured performance test suite. The main concerns are around error handling (application startup verification) and minor issues with output ordering and hardcoded values. No critical correctness bugs were found, but the error-level item should be addressed before merging.