|WARNING| net/iavf: fix RSS rule overwriting an existing one

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## ERRORS

**Use-after-free / resource leak in error path:**

The patch allocates `meta` (a `struct virtchnl_rss_cfg *`) in `iavf_hash_parse_pattern_action()` before calling `iavf_hash_create()`. When `iavf_hash_ptype_in_use()` returns true, the function sets an error and returns `-rte_errno`, but `meta` (passed as the `rss_cfg` parameter) is never freed. This leaks the allocation on the new error path.

```c
/* BAD - meta/rss_cfg is allocated but not freed on this new error path */
if (iavf_hash_ptype_in_use(ad, &rss_cfg->proto_hdrs)) {
    PMD_DRV_LOG(ERR, "RSS rule for this packet type already exists");
    rte_flow_error_set(error, EEXIST,
               RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
               "RSS rule for this packet type already exists");
    return -rte_errno;
}
```

**Fix:** Free `rss_cfg` before returning on the duplicate check error path:

```c
if (iavf_hash_ptype_in_use(ad, &rss_cfg->proto_hdrs)) {
    PMD_DRV_LOG(ERR, "RSS rule for this packet type already exists");
    rte_flow_error_set(error, EEXIST,
               RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
               "RSS rule for this packet type already exists");
    rte_free(rss_cfg);
    return -rte_errno;
}
```

**Spinlock held during list traversal without dropping it on early return:**

`iavf_hash_ptype_in_use()` acquires `vf->flow_ops_lock`, but if the raw-pattern early return (`if (hdrs->count == 0)`) is taken, the lock is never released. This causes a deadlock on the next attempt to acquire the lock.

```c
/* BAD - lock acquired but not released on early return */
if (hdrs->count == 0)
    return false;

rte_spinlock_lock(&vf->flow_ops_lock);
```

**Fix:** Either move the raw-pattern check before acquiring the lock (preferred), or unlock before returning:

```c
/* GOOD - check before lock */
if (hdrs->count == 0)
    return false;

rte_spinlock_lock(&vf->flow_ops_lock);
/* ... */
rte_spinlock_unlock(&vf->flow_ops_lock);
return found;
```

---

## WARNINGS

**Missing release notes for new error behavior:**

The patch introduces a new error case (EEXIST when creating a duplicate RSS rule) that applications will now encounter. This is a user-visible API behavior change and should be documented in the release notes under `doc/guides/rel_notes/release_26_11.rst` (or the appropriate current release file).

**Spinlock usage in error path:**

The spinlock `vf->flow_ops_lock` is used to protect the flow list traversal, but the lock is held across the entire TAILQ_FOREACH. If the list is long or the loop body is slow, this may cause latency spikes for other threads trying to access flow state. Consider whether the traversal must hold the lock for the entire duration, or whether a reference-counted or RCU approach would be safer. (This is a design suggestion, not a correctness bug, since the current locking is not incorrect per se.)

---

## INFO

**Early return placement (pre-existing observation):**

The raw-pattern check `if (hdrs->count == 0)` in `iavf_hash_ptype_in_use()` could be hoisted before the lock acquisition to avoid unnecessary lock overhead when the condition is always false for raw patterns. This does not affect correctness but is a minor optimization opportunity.

---

Review-Result: ERROR