|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