Re: [PATCH] net/txgbe: fix leak of filters on flow create

Thanks for the review.
Item 6: the ixgbe counterpart is already on the list:
   [PATCH] net/ixgbe: fix leak of filters on flow create
https://patches.dpdk.org/project/dpdk/patch/20260914133836.14644-1-zhtfdev@gmail.com/

I will send a v2 for txgbe.

On 9/15/26 23:38, Stephen Hemminger wrote:

On Tue, 15 Sep 2026 23:24:36 +0800
Zhang Tengfei [off-list ref] wrote:

quoted

txgbe_flow_create() programs ntuple, ethertype, SYN, FDIR, L2 tunnel and
RSS filters into hardware before allocating the software flow object.
If that allocation fails, create returns an error but leaves the
hardware filter installed. The application has no handle to destroy it.

Allocate the software copy first, then program the hardware. On a
programming failure, free the copy. Set ENOMEM when allocation fails
so the error path does not report success.

L2 tunnel add failures now return immediately instead of falling
through to RSS parsing, which cannot succeed for a VF/PF E-tag rule
and overwrote the original error.

Fixes: 5c2352b9ece6 ("net/txgbe: support creating consistent filter")
Cc: stable@dpdk.org

Signed-off-by: Zhang Tengfei <redacted>
---

Patch looks good, I was going to merge but AI had a couple of small items
that should be addressed first.

Review: [PATCH] net/txgbe: fix leak of filters on flow create
Patchwork: 169627

Applied to main (f43632a) and built with -Dwerror=true, no warnings.

The alloc-before-program reordering is the right fix. The add helpers
(ntuple, ethertype, syn, l2 tunnel) do not modify their input, so
copying the filter into the software object before programming is
equivalent to the old copy-after.

Warning

1. PF FDIR: mask_added not unwound on the new ENOMEM path.

    The allocation now sits after the "A mask cannot be deleted" block.
    When this rule is the first to set the mask, first_mask is TRUE and
    fdir_info->mask_added has been set before rte_zmalloc() runs. On
    allocation failure the code jumps to out without clearing
    mask_added, unlike the program-failure path right below it.
    A later rule with a different mask is then rejected with "only
    support one global mask" even though no rule is using the mask.

    Either move the fdir_rule_ptr allocation above the mask block
    (free it on the mask error paths), or clear mask_added when
    first_mask is set on allocation failure:

	if (fdir_rule_ptr == NULL) {
		PMD_DRV_LOG(ERR, "failed to allocate memory");
		if (first_mask)
			fdir_info->mask_added = FALSE;
		ret = -ENOMEM;
		goto out;
	}

    Allocating first is cleaner.

2. Missing Fixes tag for the VF FDIR path.

    The txgbevf_fdir_filter_program() branch was added later by:

    Fixes: 7eef71080e ("net/txgbe: switch to FDIR on VF")

    Add it (12-char hash) alongside the existing tag so stable
    maintainers know the VF hunk only applies to 25.11 and later.

Info

3. The commit message says ENOMEM is set "so the error path does not
    report success". Other goto out paths in the same function still
    reach rte_flow_error_set() with ret == 0: the flex_bytes_offset /
    flex_relative mismatch, and the trailing goto out for an FDIR rule
    without b_spec. The memcmp() mismatch path passes a positive ret,
    so -ret is negative. Not introduced here, but either fix them in a
    follow-up or narrow the wording.

4. The L2 tunnel early return is a separate behavior change (errno
    reported to the caller changes from the RSS parse error to the
    real add error). It is correct, but belongs in its own patch so
    it can be backported or reverted independently.

5. Lines being moved anyway can drop rte_memcpy() for plain struct
    assignment:

	ntuple_filter_ptr->filter_info = ntuple_filter;

    Same for ethertype, syn, fdir and l2 tunnel.

6. drivers/net/intel/ixgbe/ixgbe_flow.c has the same program-then-
    allocate pattern in ixgbe_flow_create(). txgbe was derived from it,
    the same fix applies there.