|WARNING| net/txgbe: fix leak of filters on flow create

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-09-15

# DPDK Patch Review: net/txgbe: fix leak of filters on flow create

## Summary

This patch fixes a resource leak in `txgbe_flow_create()` where hardware filters were programmed before allocating the software flow object. The reordering is correct and addresses a real bug. However, there is one **Error**-level correctness issue that must be fixed.

---

## Errors

### 1. Missing error handling cleanup in FDIR PF code path

**Location:** Lines 3403-3437 (FDIR filter for PF)

**Issue:** After the `rte_zmalloc()` for `fdir_rule_ptr` succeeds (line 3406), if `txgbe_fdir_filter_program()` fails (line 3415-3424), the code frees `fdir_rule_ptr` but does not clean up the FDIR mask that was added earlier at line 3366-3397.

The mask addition code (lines 3366-3397) allocates `mask_rule` and inserts it into `filter_fdir_list` if `txgbe_fdir_filter_program()` succeeds. But if `txgbe_fdir_filter_program()` fails at line 3415, the cleanup at line 3418 only frees `fdir_rule_ptr` and sets `fdir_info->mask_added = FALSE` (line 3423). It does NOT remove the `mask_rule` that was successfully added at line 3394 (`TAILQ_INSERT_TAIL(&filter_fdir_list, mask_rule, entries)`).

This leaves a dangling `mask_rule` in the list with `fdir_info->mask_added = FALSE`, causing a resource leak and list corruption.

**Why it matters:** If `txgbe_fdir_filter_program()` succeeds for the mask but fails for the actual filter rule, the mask remains installed in the hardware and the list, but `fdir_info->mask_added` is reset to `FALSE`. This leaks the `mask_rule` allocation and corrupts the state.

**Fix:** The error path at line 3418-3424 must also clean up the mask if it was added. Since the mask addition code is complex and occurs before the allocation you're moving, you need to either:
1. Keep track of whether the mask was successfully added in this function call, and clean it up on failure; OR
2. Restructure so the mask allocation/programming also happens after the `rte_zmalloc()` check

Suggested fix (track mask addition):

```c
bool mask_added_here = false;

/* ... mask addition code ... */
if (/* mask addition succeeded */) {
	TAILQ_INSERT_TAIL(&filter_fdir_list, mask_rule, entries);
	mask_added_here = true;
}

/* ... later ... */

fdir_rule_ptr = rte_zmalloc("txgbe_fdir_filter",
			sizeof(struct txgbe_fdir_rule_ele), 0);
if (!fdir_rule_ptr) {
	PMD_DRV_LOG(ERR, "failed to allocate memory");
	ret = -ENOMEM;
	if (mask_added_here) {
		/* Clean up the mask we just added */
		TAILQ_REMOVE(&filter_fdir_list, mask_rule, entries);
		rte_free(mask_rule);
		fdir_info->mask_added = FALSE;
	}
	goto out;
}

ret = txgbe_fdir_filter_program(dev, &fdir_rule, FALSE, FALSE);
if (ret) {
	rte_free(fdir_rule_ptr);
	if (mask_added_here) {
		/* Clean up the mask we added */
		TAILQ_REMOVE(&filter_fdir_list, mask_rule, entries);
		rte_free(mask_rule);
		fdir_info->mask_added = FALSE;
	}
	goto out;
}
```

**Note:** The exact cleanup depends on whether `mask_rule` allocation can be moved. If you cannot track it cleanly, consider restructuring the entire mask+rule allocation to happen together before any hardware programming.

---

## Warnings

None.

---

## Info

### 1. Consistent errno assignment

The patch sets `ret = -ENOMEM` on allocation failures throughout, which is correct. The original code silently returned success when `rte_zmalloc()` failed. This is now fixed.

### 2. L2 tunnel error handling improvement

The patch changes L2 tunnel failures to return immediately instead of falling through to RSS parsing. This is correct -- a VF/PF E-tag rule cannot be an RSS rule, so the original fallthrough was wrong and would overwrite the real error.

### 3. Code structure improvement

The reordering (allocate software copy first, then program hardware, free copy on hardware failure) is the correct pattern and matches DPDK best practices for error handling.

---

## Positive Findings

- All allocation sites now have proper error handling
- All hardware programming failures now free the allocated software copy
- Error codes are consistently propagated
- The VF FDIR path (lines 3330-3348) is correctly ordered: allocate, program, free-on-error
- The SYN, ethertype, ntuple, L2 tunnel, and RSS filter paths all follow the correct pattern

---

## Conclusion

This patch fixes a real resource leak bug. The approach is correct. However, the **FDIR PF code path has an error path resource leak** where the mask allocation is not cleaned up when `txgbe_fdir_filter_program()` fails after the mask was successfully added. This must be fixed before merging.