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

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Tue Oct 27 02:23:49 CET 2026


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

_AI Code Review_

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

# DPDK DTS Patch Review

## ERRORS

None identified.

## WARNINGS

### 1. Conditional cryptodev binding logic depends on wrong flag

**File:** `dts/framework/test_run.py`  
**Line:** `if ctx.sut_node.cryptodevs and test_run.config.perf:`

The patch changes the condition for binding crypto devices from checking `config.crypto` to checking `config.perf`. This creates a logic error: crypto device binding should occur when crypto tests need to run, not based on whether performance tests are enabled. Performance tests and crypto tests serve different purposes.

**Why it matters:** This will bind crypto devices for all performance tests, including network performance tests that don't use crypto, and will fail to bind them if `perf: false` but crypto tests are requested through some other mechanism.

**Suggested fix:** Since the `crypto` config flag is being removed, the condition should check whether crypto devices exist and are needed, not whether `perf` is enabled:

```python
# Either check if cryptodevs should always be bound when present:
if ctx.sut_node.cryptodevs:
    ctx.topology.instantiate_crypto_ports()
    ctx.topology.bind_cryptodevs("dpdk")

# Or introduce a more specific check based on the test suites being run
```

### 2. Missing release notes update

The patch removes a user-visible configuration option (`crypto: false`) and changes test classification (crypto tests - performance tests). This affects how users configure and run the test framework.

**Why it matters:** Users upgrading to this version need to know that:
- The `crypto` configuration field has been removed
- Crypto tests are now classified as performance tests
- The no-link topology now supports crypto testing without needing a separate test type

**Suggested fix:** Add an entry to the release notes documenting:
- Removal of `crypto` test type
- Migration path (set `perf: true` to run crypto tests)
- New capability (crypto tests work in no-link topology)

## INFO

### Pre-existing: Empty NIC info file behavior

The patch adds logic to write an empty JSON array when no DUT NIC exists:

```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()
else:
    self.logger.info("No DUT NIC found, DUT NIC info file will contain an empty JSON list")
with open(f"{SETTINGS.output_dir}/dut_info.json", "w") as file:
    json.dump(testrun_nic_info, file, indent=3)
```

This is acceptable behavior and the log message clearly communicates the intent. The empty JSON list `[]` is valid JSON and won't break parsers expecting a list structure.

Review-Result: WARNING


More information about the test-report mailing list