|WARNING| [v2] crypto: add RSA-specific capability parameters
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Wed Aug 19 09:40:28 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/167990
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-19
# DPDK Patch Review
## Summary
This patch adds RSA-specific capability reporting to the cryptodev API. The implementation is generally sound, but there are several issues that need to be addressed.
---
## Errors
### 1. ABI Break Without Versioning
**Error**: Adding a new union member (`rsa_capa`) to an existing structure changes the union size and breaks ABI compatibility.
**Why it matters**: Applications compiled against the old structure definition will have incorrect size/layout assumptions.
**Suggested fix**: This is an ABI change that requires:
- Marking the new API as `__rte_experimental` if this is a new function/structure
- OR following ABI versioning procedures if modifying existing stable API
- OR scheduling the change via deprecation notice for the next LTS release
Since this modifies `struct rte_cryptodev_asymmetric_xform_capability` which is part of the stable API, you need to either:
1. Add a deprecation notice and defer to next ABI break window, OR
2. Use ABI versioning to maintain backward compatibility
---
### 2. Missing Release Notes
**Error**: API addition without release notes entry.
**Why it matters**: API changes must be documented for users.
**Suggested fix**: Add entry to `doc/guides/rel_notes/release_XX_YY.rst` (where XX_YY is the current release):
```rst
* **Added RSA-specific capability reporting to cryptodev.**
Added ``rte_crypto_rsa_capa`` structure to
``rte_cryptodev_asymmetric_xform_capability`` union to allow PMDs
to report RSA-specific parameters including padding schemes and
MGF1 hash algorithms.
```
---
## Warnings
### 1. Experimental API Not Marked
**Warning**: New API structure should be marked as experimental.
**Suggested fix**: If this is truly new API (not an ABI-breaking change to existing API), mark the new structure:
```c
/**
* RSA transform capability parameters.
* ...
*/
__rte_experimental
struct rte_crypto_rsa_capa {
```
However, since this is part of an existing union in a stable structure, this becomes an ABI compatibility issue (see Error #1 above).
---
### 2. Zero-Value Semantics May Be Confusing
**Warning**: The documentation states that zero values mean "not reported and PMD default may apply", but this is ambiguous.
**Consideration**: Applications cannot distinguish between:
- PMD doesn't support the feature
- PMD supports the feature but didn't report it
- PMD supports everything (default)
**Suggested improvement**: Consider either:
1. Requiring PMDs to explicitly report all capabilities (zero = not supported), OR
2. Adding a flags field to indicate whether capabilities are reported
Current documentation is acceptable if this ambiguity is intentional, but consider clarifying the expected behavior when capabilities are not reported.
---
### 3. Missing PMD Implementation
**Warning**: New API capability structure requires at least one PMD to implement it for validation.
**Note**: The guidelines state "New device API require at least one driver implementation." Ensure at least one crypto PMD is updated to populate `rsa_capa` in a follow-up patch or this series.
---
## Info
### 1. Documentation Quality
**Good**: The documentation clearly explains:
- When to use this structure
- Relationship to existing `hash_algos` field
- Zero-value semantics
- Bit positions for bitmask fields
### 2. Naming Consistency
**Good**: Structure naming follows DPDK conventions (`rte_crypto_rsa_capa`).
### 3. Field Semantics
**Observation**: The `pad_types` field uses `uint8_t` which limits support to 8 padding types. Currently adequate for `rte_crypto_rsa_padding_type`, but consider `uint32_t` for future extensibility if this is new API.
---
## Correctness Review
### No Issues Found:
- No resource leaks
- No use-after-free
- No NULL pointer dereferences
- No race conditions (structure is read-only capability data)
- No buffer overflows
- No forbidden tokens
- No deprecated API usage
### Style Compliance:
- Doxygen comments properly formatted
- Structure alignment appropriate
- No trailing whitespace visible
- Comment style consistent with DPDK standards
---
## Required Actions
**Must fix before merge:**
1. Address ABI compatibility (Error #1) - either:
- Add deprecation notice and defer, OR
- Implement ABI versioning
2. Add release notes entry (Error #2)
**Should fix:**
1. Ensure at least one PMD implementation exists to use this capability (Warning #3)
**Consider:**
1. Clarify zero-value semantics or make capability reporting mandatory
2. Evaluate `uint8_t` vs `uint32_t` for `pad_types` for future extensibility
More information about the test-report
mailing list