|WARNING| [2/2] crypto/ipsec_mb: Add SHAKE support.
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Mon Oct 5 14:54:05 CEST 2026
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
**1. Documentation formatting could use definition lists**
In `doc/guides/cryptodevs/aesni_mb.rst`, the hash algorithms list uses bullet points where each item is just an algorithm name. This is acceptable for simple lists without descriptions, so this is actually fine as-is.
**2. Array size comment accuracy**
File: `drivers/crypto/ipsec_mb/pmd_aesni_mb_priv.h`, lines 1251-1259
The comment states:
```c
/* *< HMAC Authentication pads -
* allocating space for the maximum pad
* size supported which is 144 bytes for
* SHA3-224
*/
```
While technically correct that SHA3-224 has a 144-byte block size (the largest among the supported algorithms), this is somewhat misleading because SHA3 HMAC does not use traditional "inner/outer pads" in the same way as other HMAC constructions. SHA3 HMAC uses the KMAC construction which processes the key differently. However, the buffer size increase from 128 to 144 bytes is correct and necessary.
Suggested clarification:
```c
/* *< HMAC Authentication state -
* allocating space for the maximum block
* size supported which is 144 bytes for
* SHA3-224 (and its HMAC variant)
*/
```
This is a minor documentation clarity issue, not a functional error.
### Info
**1. Consistent algorithm naming in arrays**
The patch fixes naming consistency in the `auth_blocksize` and `auth_truncated_digest_byte_lengths` arrays by changing:
- `IMB_AUTH_ZUC_EIA3_BITLEN` - `IMB_AUTH_ZUC_EIA3`
- `IMB_AUTH_SNOW3G_UIA2_BITLEN` - `IMB_AUTH_SNOW3G_UIA2`
These changes appear to be corrections to match the current Intel Multi-Buffer library naming conventions, which is good.
**2. SHA3 block sizes are correct**
The block sizes specified match the SHA3 specification:
- SHA3-224: 144 bytes (1152 bits / 8)
- SHA3-256: 136 bytes (1088 bits / 8)
- SHA3-384: 104 bytes (832 bits / 8)
- SHA3-512: 72 bytes (576 bits / 8)
**3. Digest sizes are correct**
All digest sizes match the SHA3 specification for truncated outputs.
---
## PATCH 2/2: crypto/ipsec_mb: Add SHAKE support
### Errors
None identified.
### Warnings
**1. SHAKE digest size specification**
File: `drivers/crypto/ipsec_mb/pmd_aesni_mb_priv.h`, lines 954-955 and 973-974
SHAKE is an Extendable Output Function (XOF), not a traditional hash with a fixed digest size. The capability advertisement shows:
```c
.digest_size = {
.min = 1,
.max = 65535,
.increment = 1
}
```
This is technically correct for SHAKE's variable-length output capability. However, the truncated and full digest sizes in the arrays are set to 64 bytes:
```c
[IMB_AUTH_SHAKE128] = 64,
[IMB_AUTH_SHAKE256] = 64
```
This is reasonable as a default/common output length, but it's worth noting that:
- SHAKE128 security strength saturates at 128 bits (16 bytes) of output
- SHAKE256 security strength saturates at 256 bits (32 bytes) of output
The 64-byte value appears to be chosen for consistency with other hash functions in the table. This is acceptable but could be documented in a comment explaining that SHAKE supports variable-length output and this is a default/maximum for internal bookkeeping.
**2. Block sizes are correct**
- SHAKE-128: 168 bytes (1344 bits / 8, rate for capacity 256)
- SHAKE-256: 136 bytes (1088 bits / 8, rate for capacity 512)
These match the SHAKE specification.
---
## General Observations
**Positive aspects:**
1. Both patches properly guard new functionality with `#if IMB_VERSION(3, 0, 0) <= IMB_VERSION_NUM`
2. Release notes are updated appropriately in both patches
3. Documentation files (RST and INI) are updated consistently
4. The `auth_precompute = 0` setting for non-HMAC variants is correct (plain SHA3/SHAKE don't need key preprocessing)
5. No forbidden tokens or coding style violations detected
6. No correctness bugs identified (no resource leaks, use-after-free, missing error checks)
**Note:** The changes to existing array entries (ZUC, SNOW3G naming) in patch 1 are cleanup/corrections, not related to the SHA3 addition, but are acceptable to include in the same patch since they're in the same context.
More information about the test-report
mailing list