|WARNING| [v3] net/dpaa2: hash inner IP for tunnelled traffic

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-25

# DPDK Patch Review

## Patch Summary
This patch modifies the dpaa2 PMD to hash on both outer and inner IP headers for tunneled traffic, addressing the issue where tunneled flows with fixed outer headers all hashed to the same Rx queue.

---

## ERRORS

### 1. Resource limit checking - missing bounds validation
The code adds 6 extracts per RSS type (2 header indices x 3 IP fields) but does not verify that `i` stays within `DPKG_MAX_NUM_OF_EXTRACTS` (20) before writing to `kg_cfg->extracts[i]`.

**Impact:** Buffer overflow if the total number of requested RSS types causes `i` to exceed the extract limit.

**Fix:** Add bounds checking before each extract assignment:
```c
for (h = 0; h < RTE_DIM(ip_hdr_index); h++)
    for (f = 0; f < RTE_DIM(ip_fields); f++) {
        if (i >= DPKG_MAX_NUM_OF_EXTRACTS) {
            DPAA2_PMD_ERR("Exceeded extract limit");
            return -ENOSPC;
        }
        kg_cfg->extracts[i].extract.from_hdr.prot = NET_PROT_IP;
        /* ... */
        i++;
    }
```

The commit message states "worst case... is 16 extracts out of... 20" but this calculation is not enforced in code. The bounds check should be explicit.

---

## WARNINGS

### 1. Documentation does not fully match code behavior
The dpaa2.rst states "RSS hashes on both the outer and the inner IP header" but does not clarify the technical detail that inner IP hashing only occurs when multiple IP headers exist (tunneled traffic). 

**Suggestion:** Clarify in the documentation:
```rst
- RSS hashes on both the outer IP header (always) and the innermost IP 
  instance (tunneled traffic only). Plain frames with a single IP header 
  are hashed only on that header. Tunnelled flows that share the same 
  inner IP but differ in their outer IP may therefore be steered to 
  different Rx queues.
```

This is a minor clarity improvement, not an error.

---

## INFO

### 1. Code style - variable declarations
The `static const` arrays `ip_fields` and `ip_hdr_index` are declared inside the case block. While this is valid C99, DPDK style generally prefers declarations at the start of the function or file scope for driver code, especially for constants that could be shared.

**Consideration:** Moving these to file scope would make them available if other RSS types need similar patterns in future changes, though the current placement is acceptable.

### 2. Loop variable naming
Using single-letter loop variables `f` and `h` is acceptable for short loops, but more descriptive names (`field_idx`, `hdr_idx`) would improve readability given the nested structure.

---

## SUMMARY

**Critical Issues (Must Fix):**
- Missing bounds check before array write (buffer overflow risk)

**Recommended Changes:**
- Minor documentation clarification

The core logic of hashing on both outer and inner IP headers is sound and addresses the original problem correctly. The implementation uses `LAST_HDR_INDEX` appropriately to capture the innermost IP instance while preserving the outer IP hash for plain frames.