|WARNING| crypto/cnxk: changes to align with zuc256 v2 auth

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## ERRORS

**Error 1: Missing verification of `roc_model_is_cn20k()` return semantics**

Line: `if (key_len == 32 && roc_model_is_cn20k()) {`

The condition uses direct boolean check on `roc_model_is_cn20k()`, which is correct only if this function returns an actual `bool` type. However, the patch does not show the function definition, and if it returns `int` (0/1), the existing DPDK guidelines require explicit comparison: `if (key_len == 32 && roc_model_is_cn20k() != 0)`. Without seeing the function signature, I cannot definitively verify this is correct. If `roc_model_is_cn20k()` returns `int`, this violates the explicit comparison rule.

**Error 2: Missing bounds check on memory copy in `cpt_zuc_const_update()`**

Line: `memcpy(zuc_const, zuc_key256_v2, 16);`

The function receives `uint8_t *zuc_const` with no size parameter or validation that the destination buffer has at least 16 bytes available. The existing code paths copy 32 bytes (`memcpy(zuc_const, zuc_key128, 32)`) or 48 bytes (`memcpy(zuc_const, zuc_d, 48)`), but this new early-return path copies only 16 bytes. If callers always allocate a buffer sized for the maximum (48 bytes), this is safe, but there's no visible bounds enforcement in this function or validation that `zuc_const` points to a sufficiently sized buffer. This creates a risk if future code changes assumptions about buffer sizing.

**Error 3: Inconsistent control flow after CN20K special case**

Lines:
```c
if (key_len == 32 && roc_model_is_cn20k()) {
	memcpy(zuc_const, zuc_key256_v2, 16);
	return;
}

if (key_len == 16) {
	memcpy(zuc_const, zuc_key128, 32);
} else if (key_len == 32) {
```

The CN20K check adds an early return that changes control flow. After the return, the code proceeds to `if (key_len == 16)` and `else if (key_len == 32)`. The second `key_len == 32` branch is now only reachable when NOT on CN20K, which is correct behavior, but creates maintenance risk: if someone removes the early return or changes the condition, the logic becomes wrong. A better pattern would use explicit mutual exclusion in the conditions or restructure to make the CN20K case part of the key_len switch.

## WARNINGS

**Warning 1: Magic number without symbolic constant**

Lines:
```c
memcpy(zuc_const, zuc_key256_v2, 16);
sess->roc_se_ctx->pdcp_iv_offset = 16;
sess->roc_se_ctx->pdcp_iv_len = 32;
```

The values 16 and 32 appear to represent sizes or offsets specific to ZUC-256 V2 authentication parameters. These should be defined as named constants (e.g., `ZUC256_V2_CONST_SIZE`, `CN20K_PDCP_IV_OFFSET`) to improve code readability and maintainability. The existing code uses similar magic numbers, but new code should avoid adding more.

**Warning 2: Insufficient documentation for algorithm change**

The patch changes ZUC-256 authentication behavior specifically for CN20K models but provides no in-code comments explaining why V2 differs from the standard ZUC-256 implementation, or what "V2" means. Future maintainers will not understand why this model requires different handling. Add a comment explaining that CN20K uses ZUC-256 V2 microcode which requires different constant initialization.

**Warning 3: IV size validation inconsistency**

Lines:
```c
if (roc_model_is_cn20k())
	caps->sym.auth.iv_size.max = 16;
else
	caps->sym.auth.iv_size.max = 25;
```

The patch reduces `iv_size.max` from 25 to 16 for CN20K without explaining why the hardware limitation differs. This is a capability advertisement change that could break applications expecting the larger IV size. The release notes should document this as a behavioral change for CN20K platforms.

**Warning 4: Missing release notes entry**

The patch modifies crypto capabilities and adds new hardware-specific behavior (ZUC-256 V2 support for CN20K) but does not include updates to `doc/guides/rel_notes/`. This is a significant driver change affecting crypto operations and should be documented in the release notes.

## INFO

**Info 1: Existing code style - single-line conditionals**

The patch follows the existing style in this file of placing single-statement conditionals on separate lines with proper indentation, which is consistent with DPDK guidelines.

**Info 2: Related code context**

The function `cpt_zuc_const_update()` appears to handle multiple ZUC key lengths and MAC lengths. The patch adds CN20K-specific handling but doesn't validate the `mac_len` parameter in the new path, while the original code uses it in some branches (`else if (key_len == 32)`). Verify that ignoring `mac_len` in the CN20K path is intentional.

**Info 3: Potential test coverage gap**

The patch adds CN20K-specific code paths but does not show corresponding test additions. Verify that `app/test/` includes test cases covering ZUC-256 V2 authentication on CN20K hardware, particularly the new IV size limits and PDCP IV offset changes.

---

**FINAL CHECK APPLIED**

Pass 1 (Correctness): Verified that bounds checking, control flow consistency, and model-check semantics are flagged.

Pass 2 (Style/Process false positives): No items concluded with "actually acceptable" or "no issue" - all warnings represent genuine concerns.

Pass 3 (Evidence):
- Error 1: Based on line `if (key_len == 32 && roc_model_is_cn20k())` in added code
- Error 2: Based on line `memcpy(zuc_const, zuc_key256_v2, 16);` with no visible size parameter
- Error 3: Based on control flow in added `if (key_len == 32 && roc_model_is_cn20k()) { return; }` before existing `else if (key_len == 32)`
- All warnings based on quoted lines from the patch

Review-Result: ERROR