Re: [PATCH v2 0/3] net/txgbe: fix flow create errors

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 16 Sep 2026 21:11:02 +0800
Zhang Tengfei <zhtfdev@gmail.com> wrote:

> v2:
> - split L2 tunnel add-failure goto out into its own patch
> - allocate FDIR object before installing the global mask
> - add Fixes tag for the VF FDIR path
> - set -EINVAL on remaining FDIR goto out paths with bad ret
> - use struct assignment instead of rte_memcpy for filter copies
> 
> 1/3 fix L2 tunnel error on flow create
> 2/3 fix leak of filters on flow create
> 3/3 fix flow create error codes
> 
> Zhang Tengfei (3):
>   net/txgbe: fix L2 tunnel error on flow create
>   net/txgbe: fix leak of filters on flow create
>   net/txgbe: fix flow create error codes
> 
>  drivers/net/txgbe/txgbe_flow.c | 240 +++++++++++++++++----------------
>  1 file changed, 126 insertions(+), 114 deletions(-)
> 

Looks good still some small items found by AI review:

Review: [PATCH v2 0/3] net/txgbe: flow create fixes
Author: Zhang Tengfei <zhtfdev@gmail.com>

Series applies cleanly to main; each commit builds with -Dwerror=true.
Fixes: tags resolve (5c2352b9ece6 in 21.02, 7eef71080e16 in 25.11),
so Cc: stable is correct.

Patch 2/3 checked for early copies: ntuple, ethertype, SYN and L2
tunnel add helpers do not modify their input, so copying filter_info
before programming is safe. FDIR (PF and VF) copies after programming.
Element types match the old rte_memcpy sizes, struct assignment is
equivalent.


Patch 3/3: net/txgbe: fix flow create error codes

Warning: mask-only FDIR rule fails but leaves global mask committed

  With b_mask set and b_spec clear on the first FDIR rule,
  txgbe_flow_create() programs the input mask via
  txgbe_fdir_set_input_mask(), sets fdir_info->mask_added = TRUE,
  then falls to the final path which now returns -EINVAL.

  The application gets a failed create, holds no handle, yet the
  global mask stays in hardware and mask_added stays set. Any later
  rule with a different mask is rejected with "only support one
  global mask" even though no FDIR flow exists. The b_spec failure
  path already clears mask_added when first_mask is set; this path
  does not.

  The parser reaches this state: txgbe_parse_fdir_filter_normal() sets
  b_mask on item->mask and b_spec only on item->spec.

  Since this patch now declares the path an error, reject it before
  touching hardware, e.g. right after the rte_zmalloc():

	if (!fdir_rule.b_spec) {
		rte_free(fdir_rule_ptr);
		ret = -EINVAL;
		goto out;
	}

  and drop the trailing free/-EINVAL block.

Info: commit message says memcmp stores a "positive result". memcmp()
  returns any nonzero value of either sign, so -ret could be a random
  positive or negative number. Reword to say the value is not an
  errno.

Info: flex offset mismatch path returns -EINVAL with no log, unlike
  the mask mismatch path right above it. Add a PMD_DRV_LOG(ERR, ...)
  so the two rejections are distinguishable.