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

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Mon Sep 14 16:41:04 CEST 2026


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

_AI Code Review_

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

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

## Summary
This patch fixes a resource leak in `ixgbe_flow_create()` where hardware filters were programmed before allocating the software flow object. If allocation failed, the hardware filter remained installed with no handle to destroy it. The fix reorders operations to allocate first, then program hardware, with proper cleanup on failure.

---

## Errors

### 1. Missing error propagation on FDIR cleanup
**File:** `drivers/net/intel/ixgbe/ixgbe_flow.c`  
**Lines:** Around FDIR rule handling (2937-2950)

When FDIR hardware programming fails after allocating `fdir_rule_ptr`, the code frees the pointer but does not roll back the global state modifications that may have occurred in `ixgbe_fdir_flow_program()`.

Specifically, if `first_mask` was set to `true` by `ixgbe_fdir_flow_program()` before it failed, the later cleanup code (lines 2949-2957) that updates `fdir_info->mask_added` and other globals will never execute, leaving stale state.

**Why it matters:** The driver's FDIR mask state becomes inconsistent with hardware, potentially causing subsequent FDIR operations to fail or behave incorrectly.

**Suggested fix:**
```c
ret = ixgbe_fdir_flow_program(dev, adapter, &fdir_rule,
	&first_mask, error);
if (ret) {
	/* Roll back mask state if it was added by this call */
	if (first_mask) {
		/* Attempt to clear the mask from hardware */
		ixgbe_fdir_filter_restore(dev);
	}
	rte_free(fdir_rule_ptr);
	goto out;
}
```

Alternatively, pass `first_mask` by value and only update global state after successful programming.

---

## Warnings

None identified. The patch correctly addresses the original issue and follows DPDK coding standards.

---

## Info

### 1. Consistent error code usage
The patch now sets `ret = -ENOMEM` on allocation failures, which is correct. Previously these paths fell through without setting `ret`, potentially returning success. The fix is appropriate.

### 2. L2 tunnel early return is correct
The patch changes L2 tunnel failures to return immediately instead of falling through to RSS parsing. This is the correct pattern matching the other filter types and prevents the RSS parser from overwriting the L2 tunnel error.

### 3. Pattern consistency
All six filter types now follow the same allocate-program-cleanup pattern, which improves maintainability and reduces the chance of future leaks.

---

## Additional Observations

**Positive aspects:**
- The core fix (allocate before program, cleanup on failure) is sound
- All allocation failures now correctly set `-ENOMEM`
- Each filter type's error path properly frees its allocated structure
- The patch eliminates the leak that occurred when `rte_zmalloc()` failed after hardware programming

**Testing recommendation:**
Verify that FDIR global state (`mask_added`, `fdir_conf`) is correctly synchronized when `ixgbe_fdir_flow_program()` fails partway through. This may require checking whether that function can leave partial state after returning an error.


More information about the test-report mailing list