|WARNING| [v2] dts: remove crypto test type

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Tue Oct 20 01:27:30 CEST 2026


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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-19

# Review of DPDK DTS Crypto Test Type Removal Patch

## ERRORS

None.

## WARNINGS

**1. Empty JSON file written when no ports present** (Warning)

**File:** `dts/framework/test_run.py`, lines 363-368

The patch writes an empty JSON array when `ctx.topology.sut_ports` is empty:

```python
testrun_nic_info: list[dict[str, str]] = []
if ctx.topology.sut_ports:
    testrun_nic_info = self.test_run.ctx.sut_node.main_session.get_nic_info()
with open(f"{SETTINGS.output_dir}/dut_info.json", "w") as file:
    json.dump(testrun_nic_info, file, indent=3)
```

When no ports are present, this creates a `dut_info.json` file containing `[]`. While this is valid JSON, downstream consumers of this file may need to handle the empty case explicitly. The commit message mentions "an empty json file is written as a placeholder" but doesn't explain if this is intentional or the best approach.

**Suggestion:** Consider either:
- Documenting this behavior in code comments explaining that an empty array is written when no ports exist
- OR, omitting the file entirely when no ports are present (with a log message explaining why)

**2. Conditional topology setup may need verification** (Warning)

**File:** `dts/framework/testbed_model/topology.py`, lines 145-150

The devbind script preparation now runs when either NO_LINK topology with cryptodevs is present, or in normal link topologies:

```python
if self.type is not LinkTopology.NO_LINK or get_ctx().sut_node.cryptodevs:
    self._prepare_devbind_script()

if self.type is LinkTopology.NO_LINK:
    return

self._setup_ports("sut")
```

This changes the behavior such that `_prepare_devbind_script()` is called even in NO_LINK mode when cryptodevs exist, but `_setup_ports()` is still skipped.

**Suggestion:** Verify that cryptodev tests in NO_LINK mode don't require port setup. The logic appears correct, but the change should be confirmed through testing that cryptodev-only configurations (NO_LINK with cryptodevs) work as expected without calling `_setup_ports()`.

**3. Crypto test gating logic change** (Warning)

**File:** `dts/framework/test_run.py`, line 373

Changed from:
```python
if ctx.sut_node.cryptodevs and test_run.config.crypto:
```

To:
```python
if ctx.sut_node.cryptodevs and test_run.config.perf:
```

This means crypto device setup now runs whenever performance tests are enabled, not just when crypto tests are explicitly enabled. This is a behavior change: if someone runs with `perf=True` but doesn't intend to run crypto tests, they'll still get crypto port instantiation and binding.

**Suggestion:** Consider whether this coupling is intentional. If all crypto tests are now performance tests, this is correct. However, if non-crypto performance tests exist, they may unnecessarily trigger crypto device setup.

## INFO

**Test type removal is consistent**

The removal of `TestCaseType.CRYPTO` and the `crypto_test` decorator is thorough:
- Removed from enum definition
- Removed from decorator export
- Removed from filter logic
- All test cases updated to use `@perf_test` instead

**Configuration field removal**

The `crypto: bool` field is removed from `TestRunConfiguration`, and the filter logic updated to only check `perf` and `func` flags. This is consistent with treating crypto tests as a subset of performance tests.

**Import context in topology.py**

The addition of `from framework.context import get_ctx` inside the `setup()` method is unusual but likely done to avoid circular import issues. This is acceptable though unconventional.

---

Review-Result: WARNING


More information about the test-report mailing list