|WARNING| vhost: make cryptodev support optional

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Wed Oct 14 19:16:28 CEST 2026


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

_AI Code Review_

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

# DPDK Patch Review

## Errors

None.

## Warnings

**1. Missing release notes update**

The patch introduces a significant behavior change -- the vhost crypto feature becomes conditional on `RTE_LIB_CRYPTODEV`.
This is an API-level change that users need to know about: applications using `rte_vhost_crypto.h` may now fail to build if cryptodev is disabled in the build configuration.
A release note in `doc/guides/rel_notes/release_*.rst` is required documenting:
- The feature is now optional
- Build-time dependency on `RTE_LIB_CRYPTODEV`
- Impact on applications using vhost crypto API

**2. Incomplete conditional compilation**

The patch makes the cryptodev dependency optional in the build system,
but does not guard the vhost crypto API declarations in `rte_vhost.h` or related headers.
If `rte_vhost.h` includes or references types/functions from `rte_vhost_crypto.h`,
those references will fail to compile when cryptodev is disabled.

Verify that:
- No installed vhost header includes `rte_vhost_crypto.h` unconditionally
- No vhost API function signature or struct definition depends on cryptodev types when cryptodev is disabled
- If such dependencies exist, they must be wrapped in `#ifdef RTE_LIB_CRYPTODEV` guards

**3. Hash library dependency unclear**

The `hash` library is moved from the unconditional `deps` list to the cryptodev-conditional block.
If the vhost library uses the hash library for non-crypto purposes
(e.g., in `vhost.c`, `vhost_user.c`, or `virtio_net.c`),
removing it from the base dependencies will break the build when cryptodev is disabled.

Verify that the hash library is used ONLY in `vhost_crypto.c` and nowhere else in the vhost sources.
If it is used elsewhere, it must remain in the base `deps` list.

## Info

None.

---

**Summary:**
The patch implements a reasonable mechanism to make vhost crypto optional,
but is missing documentation and may have incomplete API guards.
The hash dependency change needs verification.

Review-Result: WARNING


More information about the test-report mailing list