|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 identified.

### Warnings

None identified.

### Info

**Documentation and feature matrix are complete:**
The patch properly updates:
- Algorithm documentation in `aesni_mb.rst`
- Feature matrix in `aesni_mb.ini`
- Release notes in `release_26_11.rst`
- Code matches documentation for all 8 new algorithms (SHA3-224/256/384/512 and their HMAC variants)

**Code structure is correct:**
- Version guard `#if IMB_VERSION(3, 0, 0) <= IMB_VERSION_NUM` properly wraps all SHA3 additions
- Capabilities structures follow existing pattern with correct block sizes and digest ranges per NIST FIPS 202
- Array updates (`auth_blocksize`, `auth_truncated_digest_byte_lengths`, `auth_digest_byte_lengths`) include all new entries
- `auth_precompute = 0` correctly set for plain SHA3 (no key expansion needed)

**Session structure change is justified:**
The increase of `inner`/`outer` pad arrays from 128 to 144 bytes is correct -- SHA3-224 has a 144-byte block size (1152 bits), larger than SHA-512's 128 bytes. The comment update accurately reflects this.

---

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

### Errors

**1. SHAKE digest sizes are variable-length XOFs, not fixed 64 bytes**

SHAKE-128 and SHAKE-256 are extendable-output functions (XOFs) that can produce arbitrary-length output. The capabilities advertise `digest_size` range `{.min = 1, .max = 65535, .increment = 1}`, which is correct. However, the `auth_truncated_digest_byte_lengths` and `auth_digest_byte_lengths` arrays hardcode both to 64 bytes:

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

This is misleading. SHAKE does not have a single canonical digest length like SHA3. The 64-byte value may be Intel IPsec-MB's default or maximum, but it contradicts the advertised capability range. Either:
- The capability `max` should match these arrays (64 bytes), or
- These arrays should document that 64 is a conventional default, not the only valid length, or
- The implementation must handle variable-length output correctly at runtime

**Suggested fix:** Verify Intel IPsec-MB library's actual SHAKE output behavior. If it supports arbitrary lengths up to 65535 bytes, document why 64 appears in these arrays (e.g., "default/recommended length"). If the library only supports up to 64 bytes, change the capabilities to `{.min = 1, .max = 64, .increment = 1}`.

**2. Comment in session structure not updated for SHAKE's larger block size**

In patch 1/2, the comment was updated to reflect SHA3-224's 144-byte block requirement:
```c
/* *< HMAC Authentication pads -
 * allocating space for the maximum pad
 * size supported which is 144 bytes for
 * SHA3-224
 */
```

Patch 2/2 adds SHAKE-128 with a 168-byte block size, which now exceeds the 144-byte allocation. The arrays are updated correctly (`[IMB_AUTH_SHAKE128] = 168`), but the pad buffer size (`alignas(16) uint8_t inner[144]`) and comment were not adjusted.

**Suggested fix:**
```c
alignas(16) uint8_t inner[168];
alignas(16) uint8_t outer[168];
/* *< HMAC Authentication pads -
 * allocating space for the maximum pad
 * size supported which is 168 bytes for
 * SHAKE-128
 */
```

Note: SHAKE does not have an HMAC mode in NIST standards (HMAC is defined for hash functions with fixed output, not XOFs), so this may be an Intel IPsec-MB-specific construct. Verify the library actually uses these pads for SHAKE.

### Warnings

None identified beyond the errors above.

### Info

**Documentation updates are complete:**
- `aesni_mb.rst` lists both algorithms
- `aesni_mb.ini` feature matrix updated
- Release notes amended

**Block sizes match NIST FIPS 202:**
- SHAKE-128: 168 bytes (1344 bits) 
- SHAKE-256: 136 bytes (1088 bits) 

---

## Series-Level Observations

**Positive:**
- Both patches guard new code with `#if IMB_VERSION(3, 0, 0) <= IMB_VERSION_NUM`, ensuring compatibility
- No forbidden tokens introduced
- Code style consistent with existing driver
- All 10 algorithms properly integrated into capabilities, block size, and digest length arrays

**Recommendations:**
1. Add tests to `app/test` for the new algorithms (or document if Intel IPsec-MB has its own test suite that covers these)
2. Verify Intel IPsec-MB 3.0.0+ actually supports all these algorithms at the API level (`IMB_AUTH_SHA3_*`, `IMB_AUTH_SHAKE*` constants exist)
3. Confirm SHAKE's variable-length output is handled correctly by the PMD at session setup and job submission

---

## Summary

**Patch 1/2:** Clean implementation, no issues found.

**Patch 2/2:** Two **Errors** requiring fixes:
1. SHAKE digest length discrepancy between advertised capability (up to 65535 bytes) and hardcoded array values (64 bytes)
2. Session pad buffer allocation (144 bytes) insufficient for SHAKE-128's 168-byte block size; comment also outdated