|WARNING| [v3,2/2] app/crypto-perf: support ML DSA

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-06-24

# DPDK Patch Review - series.patch

## Patch 1/2: app/crypto-perf: support ML KEM

### Errors

None found.

### Warnings

1. **Missing ML-KEM test data validation** (line 1179, cperf_options_parsing.c)
   ```c
   opts->mlkem_data = &mlkem_encap_perf_data[0];
   ```
   The default initialization assumes `mlkem_encap_perf_data[0]` exists, but there is no bounds check or validation. If the array is empty or the pointer is invalid, this will cause undefined behavior. Consider adding a check:
   ```c
   if (mlkem_encap_perf_data == NULL) {
       RTE_LOG(ERR, USER1, "ML-KEM test data not available\n");
       return -EINVAL;
   }
   opts->mlkem_data = &mlkem_encap_perf_data[0];
   ```

2. **Array indexing without bounds validation** (line 1680-1681, cperf_options_parsing.c)
   ```c
   if (options->asym_op_type == RTE_CRYPTO_ASYM_OP_ENCRYPT)
       options->mlkem_data = &mlkem_encap_perf_data[0];
   else if (options->asym_op_type == RTE_CRYPTO_ASYM_OP_DECRYPT)
       options->mlkem_data = &mlkem_decap_perf_data[0];
   ```
   Same concern as above - direct indexing into arrays without verifying they are non-empty.

3. **ML-KEM operation type validation occurs late** (line 520, main.c)
   The ML-KEM operation type validation in `cperf_verify_devices_capabilities()` happens after device initialization. The same validation logic is duplicated in `cperf_options_check()`. Consider removing the duplicate in `cperf_verify_devices_capabilities()` to avoid redundant error paths.

---

## Patch 2/2: app/crypto-perf: support ML DSA

### Errors

None found.

### Warnings

1. **Missing ML-DSA test data validation** (line 1184, cperf_options_parsing.c)
   ```c
   opts->mldsa_data = &mldsa_sign_perf_data[0];
   ```
   Same issue as ML-KEM patch - assumes array element exists without validation.

2. **Array indexing without bounds validation** (line 1718-1720, cperf_options_parsing.c)
   ```c
   if (options->asym_op_type == RTE_CRYPTO_ASYM_OP_SIGN)
       options->mldsa_data = &mldsa_sign_perf_data[0];
   else if (options->asym_op_type == RTE_CRYPTO_ASYM_OP_VERIFY)
       options->mldsa_data = &mldsa_verify_perf_data[0];
   ```
   Direct indexing without bounds checks.

3. **Duplicate operation type validation** (line 521, main.c)
   ML-DSA operation type validation in `cperf_verify_devices_capabilities()` duplicates logic in `cperf_options_check()`. Consider consolidating to avoid redundant code.

4. **Missing documentation for `sign_deterministic` field**
   The `sign_deterministic` field in `struct cperf_mldsa_test_data` (cperf_test_vectors.h:222) lacks documentation explaining when it should be true vs. false. Add a comment:
   ```c
   bool sign_deterministic;  /* true for deterministic signing (requires seed), false for randomized */
   ```

---

## General Observations

### Code Quality
- Both patches follow DPDK coding style conventions.
- Function and variable naming is consistent with existing patterns.
- The test vector data arrays are appropriately sized and structured.

### Architecture
- The implementation correctly extends the existing asymmetric crypto test framework.
- ML-KEM and ML-DSA operations are properly categorized as asymmetric operations.
- Error handling follows DPDK patterns (negative errno returns).

### Testing
- Both patches add test vectors but no mention of whether these have been validated against known-good implementations.
- Consider documenting the source of test vectors (e.g., NIST reference vectors) in commit messages.

---

## Summary

Both patches are well-structured and follow DPDK conventions. The primary concerns are:
1. Lack of bounds checking when indexing into test data arrays (could cause crashes if arrays are unexpectedly empty).
2. Minor code duplication in validation logic between `cperf_options_check()` and `cperf_verify_devices_capabilities()`.

These are quality-of-life improvements rather than critical bugs, but addressing them would make the code more robust.