|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