|WARNING| [v9] examples: add Wycheproof validation app
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Tue Oct 13 18:26:55 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/171198
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-13
# Wycheproof Validation Example - Code Review
## ERRORS
None identified.
## WARNINGS
**1. Queue-related buffers not using `rte_zmalloc_socket()`**
Lines: 194, 199
The code allocates mbuf pool, session pool, and op pool using `rte_pktmbuf_pool_create()` and `rte_crypto_op_pool_create()`, which is correct. However, these are not descriptor rings or queue control structures, so the guideline about `rte_zmalloc_socket()` for queue-related buffers does not apply here. The pools are correctly allocated with socket affinity via the `rte_socket_id()` parameter.
Upon closer inspection, this is not a warning - the existing allocation pattern is appropriate for mempools.
**2. Missing release notes for internal implementation change (fips_validation move)**
The patch moves `examples/fips_validation/` to `examples/crypto_validation/fips_validation/` (a directory reorganization) but does not add a release note entry for this move. While the new Wycheproof application is documented in release notes, the fips_validation move is not mentioned.
This is a user-visible change (path change affects build commands and documentation references). A release note entry should be added under "Other changes" or similar section mentioning the directory reorganization.
**3. Missing NULL checks after asym pool creation (lines 233-236)**
The code creates `asym_session_pool` and `asym_op_pool` with best-effort comment but does not check for NULL before using them later. Lines 262+ use these pools:
```c
env.asym_session_pool = rte_cryptodev_asym_session_pool_create(
"WYCHEPROOF_ASYM_SESS", 16, 0, 0, rte_socket_id());
env.asym_op_pool = rte_crypto_op_pool_create("WYCHEPROOF_ASYM_OP",
RTE_CRYPTO_OP_TYPE_ASYMMETRIC, 64, 0, 0, rte_socket_id());
```
Later usage checks for NULL (e.g., line 1223: `if (env.asym_session_pool == NULL || env.asym_op_pool == NULL)`), which is correct. The creation is indeed best-effort, and the runtime checks handle the NULL case appropriately. This is acceptable.
Actually, this is not a warning - the code correctly handles NULL pools at use time.
**4. Error path resource leak in `app_init()` (line 310)**
In the error path cleanup at line 310, if the hash device setup succeeds but a later allocation fails, the code jumps to `error:` which does not stop or close the hash device. The hash device is only stopped/closed in `app_uninit()`, but if `app_init()` fails, the caller may not call `app_uninit()`.
However, looking at `main()` line 2070: when `app_init()` fails, the code calls `goto out_eal` which does NOT call `app_uninit()`. If the hash device was configured and started during the hash device selection loop (lines 262-283), it will leak.
**Error path in app_init() does not clean up hash_dev_id if it was configured:**
When `app_init()` fails after successfully configuring the hash device (lines 262-283), the cleanup at label `error:` (line 310) does not stop or close `env.hash_dev_id`. This leaks the configured device.
Add to the error path before returning:
```c
error:
if (env.hash_dev_own) {
rte_cryptodev_stop(env.hash_dev_id);
rte_cryptodev_close(env.hash_dev_id);
}
/* existing cleanup */
```
**5. Standalone Makefile uses `-DALLOW_EXPERIMENTAL_API` in CFLAGS (line 24)**
Line 24:
```c
CFLAGS += -O3 -DALLOW_EXPERIMENTAL_API $(shell $(PKGCONF) --cflags libdpdk jansson)
```
Per AGENTS.md: "Not in lib/drivers/app" for `-DALLOW_EXPERIMENTAL_API` in build flags. This is an example application (`examples/`), so the flag is acceptable here. Examples are allowed to use experimental APIs.
This is not a warning.
## INFO
**1. RST documentation could use definition lists (doc/guides/sample_app_ug/wycheproof_validation.rst)**
Lines 23-27 and 33-36 describe command-line options using paragraph text. These could be formatted as definition lists for better structure:
Current:
```
Build the example from the DPDK tree with Meson:
.. code-block:: console
meson setup build -Dexamples=crypto_validation/wycheproof_validation \
-Denable_drivers=crypto/openssl
meson compile -C build
```
Could use a definition list if documenting multiple options, but the current format is acceptable for simple build/run instructions. The existing style matches other DPDK example documentation. No change needed.
**2. `rte_free_sensitive()` used for key material (line 335)**
Line 335:
```c
rte_free_sensitive(vector->key);
```
This is correct - key material should be zeroed before free. Well done.
Review-Result: WARNING
More information about the test-report
mailing list