|WARNING| [2/2] crypto/ipsec_mb: Add SHAKE support.

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Patch 1/2: crypto/ipsec_mb: Add SHA3 support

### Errors

None.

### Warnings

**Inconsistent auth algorithm enum names (pmd_aesni_mb_priv.h:1065-1067)**

The patch changes existing array entries from `IMB_AUTH_ZUC_EIA3_BITLEN`, `IMB_AUTH_SNOW3G_UIA2_BITLEN` to `IMB_AUTH_ZUC_EIA3`, `IMB_AUTH_SNOW3G_UIA2` without explanation. This appears to be fixing existing code rather than adding SHA3 support. If these are intentional corrections, they should be in a separate patch with explanation in the commit message. If they are unintentional, revert them.

```c
/* Changed without explanation */
[IMB_AUTH_ZUC_EIA3]		= 16,      /* was IMB_AUTH_ZUC_EIA3_BITLEN */
[IMB_AUTH_SNOW3G_UIA2]	= 16,      /* was IMB_AUTH_SNOW3G_UIA2_BITLEN */
```

**Misleading comment about maximum pad size (pmd_aesni_mb_priv.h:1256-1259)**

The comment states the maximum pad size is 144 bytes for SHA3-224, but the arrays are sized to 144 bytes. While this is correct for SHA3-224 (which has a 144-byte rate/block size), the comment incorrectly implies SHA3-224 requires the largest HMAC pads in the driver. In fact, SHA3-224 has the largest *block size* among the hash functions in this driver, but HMAC key padding is more complex. The comment should clarify this is the maximum *block size* needed for HMAC preprocessing, not simply "pad size."

```c
/* Current comment is unclear */
/* *< HMAC Authentication pads -
 * allocating space for the maximum pad
 * size supported which is 144 bytes for
 * SHA3-224
 */

/* Suggested clarification */
/* *< HMAC Authentication pads -
 * allocating space for the maximum block
 * size supported which is 144 bytes for
 * SHA3-224
 */
```

### Info

**Block size values (pmd_aesni_mb_priv.h:769+)**

The SHA3 block sizes (144, 136, 104, 72) represent the rate parameter (r) of the Keccak sponge construction in bytes. These values are correct for the respective SHA3 variants. They are not Errors or Warnings, just noting for review completeness.

**Digest size ranges (pmd_aesni_mb_priv.h:785+, 827+, 869+, 911+)**

The SHA3 and HMAC-SHA3 capability entries allow truncated digests from 1 byte up to the full digest length (28, 32, 48, or 64 bytes). This matches common practice for HMAC truncation and is acceptable.

---

## Patch 2/2: crypto/ipsec_mb: Add SHAKE support

### Errors

None.

### Warnings

**SHAKE truncated digest defaults appear arbitrary (pmd_aesni_mb_priv.h:1164-1165)**

SHAKE is an extendable-output function (XOF) that can produce arbitrary-length output. The patch sets the "truncated digest" length to 64 bytes for both SHAKE-128 and SHAKE-256, but these are arbitrary choices not specified by any standard. The `auth_truncated_digest_byte_lengths` array name suggests these are maximum safe truncation lengths, but there's no standard defining a "truncated" output for SHAKE. Either document why 64 bytes was chosen (e.g., matching typical usage or library limitation), or reconsider whether SHAKE belongs in this array at all given its arbitrary-length nature.

```c
[IMB_AUTH_SHAKE128]		= 64,
[IMB_AUTH_SHAKE256]		= 64
```

**SHAKE "digest size" defaults also arbitrary (pmd_aesni_mb_priv.h:1214-1215)**

Similar issue: `auth_digest_byte_lengths` sets SHAKE-128 and SHAKE-256 to 64 bytes. For SHAKE-128, NIST SP 800-185 suggests a minimum of 256 bits (32 bytes) for 128-bit security, and for SHAKE-256, 512 bits (64 bytes) for 256-bit security. Setting both to 64 may be conservative but reduces SHAKE-128's throughput benefit. Document the rationale or consider 32 bytes for SHAKE-128.

```c
/* Default may not match NIST recommendations */
[IMB_AUTH_SHAKE128]		= 64,  /* NIST suggests 32 bytes minimum */
[IMB_AUTH_SHAKE256]		= 64   /* NIST suggests 64 bytes minimum */
```

### Info

**SHAKE digest_size capabilities allow arbitrary lengths (pmd_aesni_mb_priv.h:952, 973)**

The capability entries correctly advertise that SHAKE can produce outputs from 1 to 65535 bytes, matching its XOF design. This is acceptable and gives applications flexibility.

---

## General Observations

### Release Notes

Both patches update `doc/guides/rel_notes/release_26_11.rst` with appropriate new feature descriptions. The updates are clear and correctly placed in the "New Features" section.

### Documentation Consistency

The `aesni_mb.rst` documentation and `aesni_mb.ini` feature matrix are updated consistently with the code capabilities. The algorithm names match the RTE_CRYPTO_AUTH_* enum values.

### Version Gating

Both patches correctly gate the new features with `#if IMB_VERSION(3, 0, 0) <= IMB_VERSION_NUM`, ensuring they only compile with intel-ipsec-mb library version 3.0.0 or later.

### No Functional Tests Visible

Neither patch includes updates to `app/test` or adds test vectors for the new algorithms. While not strictly required in the driver patch itself, the contributor should ensure functional tests exist or are submitted separately.

---

## Summary

**Patch 1/2:** Acceptable with minor issues. The unexplained change to ZUC/SNOW3G enum names should be clarified or separated. The comment about pad size should be reworded for clarity.

**Patch 2/2:** Acceptable with documentation needed. The arbitrary default digest lengths for SHAKE should be documented or reconsidered based on NIST guidance.

Both patches are otherwise correct and follow DPDK coding standards. No correctness bugs identified.