|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