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

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

[Zhang Tengfei]

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 <zhtfdev@gmail.com>
---
 drivers/net/txgbe/txgbe_flow.c | 227 ++++++++++++++++++---------------
 1 file changed, 122 insertions(+), 105 deletions(-)

diff --git a/drivers/net/txgbe/txgbe_flow.c b/drivers/net/txgbe/txgbe_flow.c
index eaeb973c91..a3e6f8cb83 100644
--- a/drivers/net/txgbe/txgbe_flow.c
+++ b/drivers/net/txgbe/txgbe_flow.c
@@ -3,6 +3,7 @@
  * Copyright(c) 2010-2017 Intel Corporation
  */
 
+#include <errno.h>
 #include <sys/queue.h>
 #include <bus_pci_driver.h>
 #include <rte_malloc.h>
@@ -3246,26 +3247,28 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 #endif
 
 	if (!ret) {
+		ntuple_filter_ptr = rte_zmalloc("txgbe_ntuple_filter",
+			sizeof(struct txgbe_ntuple_filter_ele), 0);
+		if (!ntuple_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		rte_memcpy(&ntuple_filter_ptr->filter_info,
+			&ntuple_filter,
+			sizeof(struct rte_eth_ntuple_filter));
 		ret = txgbe_add_del_ntuple_filter(dev, &ntuple_filter, TRUE);
-		if (!ret) {
-			ntuple_filter_ptr = rte_zmalloc("txgbe_ntuple_filter",
-				sizeof(struct txgbe_ntuple_filter_ele), 0);
-			if (!ntuple_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&ntuple_filter_ptr->filter_info,
-				&ntuple_filter,
-				sizeof(struct rte_eth_ntuple_filter));
-			TAILQ_INSERT_TAIL(&filter_ntuple_list,
-				ntuple_filter_ptr, entries);
-			flow->rule = ntuple_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_NTUPLE;
-			return flow;
-		} else if (filter_info->ntuple_is_full) {
-			goto next;
+		if (ret) {
+			rte_free(ntuple_filter_ptr);
+			if (filter_info->ntuple_is_full)
+				goto next;
+			goto out;
 		}
-		goto out;
+		TAILQ_INSERT_TAIL(&filter_ntuple_list,
+			ntuple_filter_ptr, entries);
+		flow->rule = ntuple_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_NTUPLE;
+		return flow;
 	}
 
 next:
@@ -3273,51 +3276,53 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 	ret = txgbe_parse_ethertype_filter(dev, attr, pattern,
 				actions, &ethertype_filter, error);
 	if (!ret) {
+		ethertype_filter_ptr = rte_zmalloc("txgbe_ethertype_filter",
+			sizeof(struct txgbe_ethertype_filter_ele), 0);
+		if (!ethertype_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		rte_memcpy(&ethertype_filter_ptr->filter_info,
+			&ethertype_filter,
+			sizeof(struct rte_eth_ethertype_filter));
 		ret = txgbe_add_del_ethertype_filter(dev,
 				&ethertype_filter, TRUE);
-		if (!ret) {
-			ethertype_filter_ptr =
-				rte_zmalloc("txgbe_ethertype_filter",
-				sizeof(struct txgbe_ethertype_filter_ele), 0);
-			if (!ethertype_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&ethertype_filter_ptr->filter_info,
-				&ethertype_filter,
-				sizeof(struct rte_eth_ethertype_filter));
-			TAILQ_INSERT_TAIL(&filter_ethertype_list,
-				ethertype_filter_ptr, entries);
-			flow->rule = ethertype_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_ETHERTYPE;
-			return flow;
+		if (ret) {
+			rte_free(ethertype_filter_ptr);
+			goto out;
 		}
-		goto out;
+		TAILQ_INSERT_TAIL(&filter_ethertype_list,
+			ethertype_filter_ptr, entries);
+		flow->rule = ethertype_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_ETHERTYPE;
+		return flow;
 	}
 
 	memset(&syn_filter, 0, sizeof(struct rte_eth_syn_filter));
 	ret = txgbe_parse_syn_filter(dev, attr, pattern,
 				actions, &syn_filter, error);
 	if (!ret) {
+		syn_filter_ptr = rte_zmalloc("txgbe_syn_filter",
+			sizeof(struct txgbe_eth_syn_filter_ele), 0);
+		if (!syn_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		rte_memcpy(&syn_filter_ptr->filter_info,
+			&syn_filter,
+			sizeof(struct rte_eth_syn_filter));
 		ret = txgbe_syn_filter_set(dev, &syn_filter, TRUE);
-		if (!ret) {
-			syn_filter_ptr = rte_zmalloc("txgbe_syn_filter",
-				sizeof(struct txgbe_eth_syn_filter_ele), 0);
-			if (!syn_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&syn_filter_ptr->filter_info,
-				&syn_filter,
-				sizeof(struct rte_eth_syn_filter));
-			TAILQ_INSERT_TAIL(&filter_syn_list,
-				syn_filter_ptr,
-				entries);
-			flow->rule = syn_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_SYN;
-			return flow;
+		if (ret) {
+			rte_free(syn_filter_ptr);
+			goto out;
 		}
-		goto out;
+		TAILQ_INSERT_TAIL(&filter_syn_list,
+			syn_filter_ptr, entries);
+		flow->rule = syn_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_SYN;
+		return flow;
 	}
 
 	memset(&fdir_rule, 0, sizeof(struct txgbe_fdir_rule));
@@ -3325,16 +3330,21 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 				actions, &fdir_rule, error);
 	if (!ret) {
 		if (!txgbe_is_pf(TXGBE_DEV_HW(dev))) {
-			ret = txgbevf_fdir_filter_program(dev, &fdir_rule, FALSE);
-			if (ret < 0)
-				goto out;
-
 			fdir_rule_ptr = rte_zmalloc("txgbe_fdir_filter",
-					    sizeof(struct txgbe_fdir_rule_ele), 0);
+					sizeof(struct txgbe_fdir_rule_ele), 0);
 			if (!fdir_rule_ptr) {
 				PMD_DRV_LOG(ERR, "failed to allocate memory");
+				ret = -ENOMEM;
 				goto out;
 			}
+
+			ret = txgbevf_fdir_filter_program(dev, &fdir_rule,
+							  FALSE);
+			if (ret < 0) {
+				rte_free(fdir_rule_ptr);
+				goto out;
+			}
+
 			rte_memcpy(&fdir_rule_ptr->filter_info,
 				   &fdir_rule,
 				   sizeof(struct txgbe_fdir_rule));
@@ -3393,28 +3403,19 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 		}
 
 		if (fdir_rule.b_spec) {
-			ret = txgbe_fdir_filter_program(dev, &fdir_rule,
-					FALSE, FALSE);
-			if (!ret) {
-				fdir_rule_ptr = rte_zmalloc("txgbe_fdir_filter",
+			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");
-					goto out;
-				}
-				rte_memcpy(&fdir_rule_ptr->filter_info,
-					&fdir_rule,
-					sizeof(struct txgbe_fdir_rule));
-				TAILQ_INSERT_TAIL(&filter_fdir_list,
-					fdir_rule_ptr, entries);
-				flow->rule = fdir_rule_ptr;
-				flow->filter_type = RTE_ETH_FILTER_FDIR;
-
-				return flow;
+			if (!fdir_rule_ptr) {
+				PMD_DRV_LOG(ERR,
+					"failed to allocate memory");
+				ret = -ENOMEM;
+				goto out;
 			}
 
+			ret = txgbe_fdir_filter_program(dev, &fdir_rule,
+					FALSE, FALSE);
 			if (ret) {
+				rte_free(fdir_rule_ptr);
 				/**
 				 * clean the mask_added flag if fail to
 				 * program
@@ -3423,6 +3424,16 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 					fdir_info->mask_added = FALSE;
 				goto out;
 			}
+
+			rte_memcpy(&fdir_rule_ptr->filter_info,
+				&fdir_rule,
+				sizeof(struct txgbe_fdir_rule));
+			TAILQ_INSERT_TAIL(&filter_fdir_list,
+				fdir_rule_ptr, entries);
+			flow->rule = fdir_rule_ptr;
+			flow->filter_type = RTE_ETH_FILTER_FDIR;
+
+			return flow;
 		}
 
 		goto out;
@@ -3432,45 +3443,51 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 	ret = txgbe_parse_l2_tn_filter(dev, attr, pattern,
 					actions, &l2_tn_filter, error);
 	if (!ret) {
+		l2_tn_filter_ptr = rte_zmalloc("txgbe_l2_tn_filter",
+			sizeof(struct txgbe_eth_l2_tunnel_conf_ele), 0);
+		if (!l2_tn_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		rte_memcpy(&l2_tn_filter_ptr->filter_info,
+			&l2_tn_filter,
+			sizeof(struct txgbe_l2_tunnel_conf));
 		ret = txgbe_dev_l2_tunnel_filter_add(dev, &l2_tn_filter, FALSE);
-		if (!ret) {
-			l2_tn_filter_ptr = rte_zmalloc("txgbe_l2_tn_filter",
-				sizeof(struct txgbe_eth_l2_tunnel_conf_ele), 0);
-			if (!l2_tn_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&l2_tn_filter_ptr->filter_info,
-				&l2_tn_filter,
-				sizeof(struct txgbe_l2_tunnel_conf));
-			TAILQ_INSERT_TAIL(&filter_l2_tunnel_list,
-				l2_tn_filter_ptr, entries);
-			flow->rule = l2_tn_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_L2_TUNNEL;
-			return flow;
+		if (ret) {
+			rte_free(l2_tn_filter_ptr);
+			goto out;
 		}
+		TAILQ_INSERT_TAIL(&filter_l2_tunnel_list,
+			l2_tn_filter_ptr, entries);
+		flow->rule = l2_tn_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_L2_TUNNEL;
+		return flow;
 	}
 
 	memset(&rss_conf, 0, sizeof(struct txgbe_rte_flow_rss_conf));
 	ret = txgbe_parse_rss_filter(dev, attr,
 					actions, &rss_conf, error);
 	if (!ret) {
-		ret = txgbe_config_rss_filter(dev, &rss_conf, TRUE);
-		if (!ret) {
-			rss_filter_ptr = rte_zmalloc("txgbe_rss_filter",
-				sizeof(struct txgbe_rss_conf_ele), 0);
-			if (!rss_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			txgbe_rss_conf_init(&rss_filter_ptr->filter_info,
-					    &rss_conf.conf);
-			TAILQ_INSERT_TAIL(&filter_rss_list,
-				rss_filter_ptr, entries);
-			flow->rule = rss_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_HASH;
-			return flow;
+		rss_filter_ptr = rte_zmalloc("txgbe_rss_filter",
+			sizeof(struct txgbe_rss_conf_ele), 0);
+		if (!rss_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
 		}
+		ret = txgbe_config_rss_filter(dev, &rss_conf, TRUE);
+		if (ret) {
+			rte_free(rss_filter_ptr);
+			goto out;
+		}
+		txgbe_rss_conf_init(&rss_filter_ptr->filter_info,
+				    &rss_conf.conf);
+		TAILQ_INSERT_TAIL(&filter_rss_list,
+			rss_filter_ptr, entries);
+		flow->rule = rss_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_HASH;
+		return flow;
 	}
 
 out:
-- 
2.53.0

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

[Stephen Hemminger]

On Tue, 15 Sep 2026 23:24:36 +0800
Zhang Tengfei <zhtfdev@gmail.com> wrote:

> 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 <zhtfdev@gmail.com>
> ---

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.

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

[Zhang Tengfei]

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 <zhtfdev@gmail.com> wrote:
>
>> 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 <zhtfdev@gmail.com>
>> ---
> 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.

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

[Zhang Tengfei]

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(-)

-- 
2.53.0

[PATCH v2 1/3] net/txgbe: fix L2 tunnel error on flow create

[Zhang Tengfei]

txgbe_flow_create() continues to RSS parsing when L2 tunnel filter
add fails. An E-tag VF/PF rule cannot match RSS, so the original
error is overwritten.

Return immediately on L2 add failure, same as ntuple, ethertype and
SYN.

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

Signed-off-by: Zhang Tengfei <zhtfdev@gmail.com>
---
 drivers/net/txgbe/txgbe_flow.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/txgbe/txgbe_flow.c b/drivers/net/txgbe/txgbe_flow.c
index eaeb973c91..76191a7c2d 100644
--- a/drivers/net/txgbe/txgbe_flow.c
+++ b/drivers/net/txgbe/txgbe_flow.c
@@ -3449,6 +3449,7 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 			flow->filter_type = RTE_ETH_FILTER_L2_TUNNEL;
 			return flow;
 		}
+		goto out;
 	}
 
 	memset(&rss_conf, 0, sizeof(struct txgbe_rte_flow_rss_conf));
-- 
2.53.0

[PATCH v2 2/3] net/txgbe: fix leak of filters on flow create

[Zhang Tengfei]

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.
Allocate the FDIR object before installing the global mask so an
allocation failure cannot leave mask_added set with no rule.

Fixes: 5c2352b9ece6 ("net/txgbe: support creating consistent filter")
Fixes: 7eef71080e16 ("net/txgbe: switch to FDIR on VF")
Cc: stable@dpdk.org

Signed-off-by: Zhang Tengfei <zhtfdev@gmail.com>
---
 drivers/net/txgbe/txgbe_flow.c | 233 +++++++++++++++++----------------
 1 file changed, 121 insertions(+), 112 deletions(-)

diff --git a/drivers/net/txgbe/txgbe_flow.c b/drivers/net/txgbe/txgbe_flow.c
index 76191a7c2d..a1a497fa22 100644
--- a/drivers/net/txgbe/txgbe_flow.c
+++ b/drivers/net/txgbe/txgbe_flow.c
@@ -3,6 +3,7 @@
  * Copyright(c) 2010-2017 Intel Corporation
  */
 
+#include <errno.h>
 #include <sys/queue.h>
 #include <bus_pci_driver.h>
 #include <rte_malloc.h>
@@ -3246,26 +3247,26 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 #endif
 
 	if (!ret) {
+		ntuple_filter_ptr = rte_zmalloc("txgbe_ntuple_filter",
+			sizeof(struct txgbe_ntuple_filter_ele), 0);
+		if (!ntuple_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		ntuple_filter_ptr->filter_info = ntuple_filter;
 		ret = txgbe_add_del_ntuple_filter(dev, &ntuple_filter, TRUE);
-		if (!ret) {
-			ntuple_filter_ptr = rte_zmalloc("txgbe_ntuple_filter",
-				sizeof(struct txgbe_ntuple_filter_ele), 0);
-			if (!ntuple_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&ntuple_filter_ptr->filter_info,
-				&ntuple_filter,
-				sizeof(struct rte_eth_ntuple_filter));
-			TAILQ_INSERT_TAIL(&filter_ntuple_list,
-				ntuple_filter_ptr, entries);
-			flow->rule = ntuple_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_NTUPLE;
-			return flow;
-		} else if (filter_info->ntuple_is_full) {
-			goto next;
+		if (ret) {
+			rte_free(ntuple_filter_ptr);
+			if (filter_info->ntuple_is_full)
+				goto next;
+			goto out;
 		}
-		goto out;
+		TAILQ_INSERT_TAIL(&filter_ntuple_list,
+			ntuple_filter_ptr, entries);
+		flow->rule = ntuple_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_NTUPLE;
+		return flow;
 	}
 
 next:
@@ -3273,51 +3274,49 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 	ret = txgbe_parse_ethertype_filter(dev, attr, pattern,
 				actions, &ethertype_filter, error);
 	if (!ret) {
+		ethertype_filter_ptr = rte_zmalloc("txgbe_ethertype_filter",
+			sizeof(struct txgbe_ethertype_filter_ele), 0);
+		if (!ethertype_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		ethertype_filter_ptr->filter_info = ethertype_filter;
 		ret = txgbe_add_del_ethertype_filter(dev,
 				&ethertype_filter, TRUE);
-		if (!ret) {
-			ethertype_filter_ptr =
-				rte_zmalloc("txgbe_ethertype_filter",
-				sizeof(struct txgbe_ethertype_filter_ele), 0);
-			if (!ethertype_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&ethertype_filter_ptr->filter_info,
-				&ethertype_filter,
-				sizeof(struct rte_eth_ethertype_filter));
-			TAILQ_INSERT_TAIL(&filter_ethertype_list,
-				ethertype_filter_ptr, entries);
-			flow->rule = ethertype_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_ETHERTYPE;
-			return flow;
+		if (ret) {
+			rte_free(ethertype_filter_ptr);
+			goto out;
 		}
-		goto out;
+		TAILQ_INSERT_TAIL(&filter_ethertype_list,
+			ethertype_filter_ptr, entries);
+		flow->rule = ethertype_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_ETHERTYPE;
+		return flow;
 	}
 
 	memset(&syn_filter, 0, sizeof(struct rte_eth_syn_filter));
 	ret = txgbe_parse_syn_filter(dev, attr, pattern,
 				actions, &syn_filter, error);
 	if (!ret) {
+		syn_filter_ptr = rte_zmalloc("txgbe_syn_filter",
+			sizeof(struct txgbe_eth_syn_filter_ele), 0);
+		if (!syn_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		syn_filter_ptr->filter_info = syn_filter;
 		ret = txgbe_syn_filter_set(dev, &syn_filter, TRUE);
-		if (!ret) {
-			syn_filter_ptr = rte_zmalloc("txgbe_syn_filter",
-				sizeof(struct txgbe_eth_syn_filter_ele), 0);
-			if (!syn_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&syn_filter_ptr->filter_info,
-				&syn_filter,
-				sizeof(struct rte_eth_syn_filter));
-			TAILQ_INSERT_TAIL(&filter_syn_list,
-				syn_filter_ptr,
-				entries);
-			flow->rule = syn_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_SYN;
-			return flow;
+		if (ret) {
+			rte_free(syn_filter_ptr);
+			goto out;
 		}
-		goto out;
+		TAILQ_INSERT_TAIL(&filter_syn_list,
+			syn_filter_ptr, entries);
+		flow->rule = syn_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_SYN;
+		return flow;
 	}
 
 	memset(&fdir_rule, 0, sizeof(struct txgbe_fdir_rule));
@@ -3325,19 +3324,22 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 				actions, &fdir_rule, error);
 	if (!ret) {
 		if (!txgbe_is_pf(TXGBE_DEV_HW(dev))) {
-			ret = txgbevf_fdir_filter_program(dev, &fdir_rule, FALSE);
-			if (ret < 0)
-				goto out;
-
 			fdir_rule_ptr = rte_zmalloc("txgbe_fdir_filter",
-					    sizeof(struct txgbe_fdir_rule_ele), 0);
+					sizeof(struct txgbe_fdir_rule_ele), 0);
 			if (!fdir_rule_ptr) {
 				PMD_DRV_LOG(ERR, "failed to allocate memory");
+				ret = -ENOMEM;
 				goto out;
 			}
-			rte_memcpy(&fdir_rule_ptr->filter_info,
-				   &fdir_rule,
-				   sizeof(struct txgbe_fdir_rule));
+
+			ret = txgbevf_fdir_filter_program(dev, &fdir_rule,
+							  FALSE);
+			if (ret < 0) {
+				rte_free(fdir_rule_ptr);
+				goto out;
+			}
+
+			fdir_rule_ptr->filter_info = fdir_rule;
 			TAILQ_INSERT_TAIL(&filter_fdir_list,
 					  fdir_rule_ptr, entries);
 			flow->rule = fdir_rule_ptr;
@@ -3345,6 +3347,14 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 			return flow;
 		}
 
+		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;
+			goto out;
+		}
+
 		/* A mask cannot be deleted. */
 		if (fdir_rule.b_mask) {
 			if (!fdir_info->mask_added) {
@@ -3366,8 +3376,10 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 				fdir_info->mask.pkt_type_mask =
 					fdir_rule.mask.pkt_type_mask;
 				ret = txgbe_fdir_set_input_mask(dev);
-				if (ret)
+				if (ret) {
+					rte_free(fdir_rule_ptr);
 					goto out;
+				}
 
 				fdir_info->mask_added = TRUE;
 				first_mask = TRUE;
@@ -3381,40 +3393,25 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 					sizeof(struct txgbe_hw_fdir_mask));
 				if (ret) {
 					PMD_DRV_LOG(ERR, "only support one global mask");
+					rte_free(fdir_rule_ptr);
 					goto out;
 				}
 
 				if (fdir_info->flex_bytes_offset !=
 				    fdir_rule.flex_bytes_offset ||
 				    fdir_info->flex_relative !=
-				    fdir_rule.flex_relative)
+				    fdir_rule.flex_relative) {
+					rte_free(fdir_rule_ptr);
 					goto out;
+				}
 			}
 		}
 
 		if (fdir_rule.b_spec) {
 			ret = txgbe_fdir_filter_program(dev, &fdir_rule,
 					FALSE, FALSE);
-			if (!ret) {
-				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");
-					goto out;
-				}
-				rte_memcpy(&fdir_rule_ptr->filter_info,
-					&fdir_rule,
-					sizeof(struct txgbe_fdir_rule));
-				TAILQ_INSERT_TAIL(&filter_fdir_list,
-					fdir_rule_ptr, entries);
-				flow->rule = fdir_rule_ptr;
-				flow->filter_type = RTE_ETH_FILTER_FDIR;
-
-				return flow;
-			}
-
 			if (ret) {
+				rte_free(fdir_rule_ptr);
 				/**
 				 * clean the mask_added flag if fail to
 				 * program
@@ -3423,8 +3420,17 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 					fdir_info->mask_added = FALSE;
 				goto out;
 			}
+
+			fdir_rule_ptr->filter_info = fdir_rule;
+			TAILQ_INSERT_TAIL(&filter_fdir_list,
+				fdir_rule_ptr, entries);
+			flow->rule = fdir_rule_ptr;
+			flow->filter_type = RTE_ETH_FILTER_FDIR;
+
+			return flow;
 		}
 
+		rte_free(fdir_rule_ptr);
 		goto out;
 	}
 
@@ -3432,46 +3438,49 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 	ret = txgbe_parse_l2_tn_filter(dev, attr, pattern,
 					actions, &l2_tn_filter, error);
 	if (!ret) {
+		l2_tn_filter_ptr = rte_zmalloc("txgbe_l2_tn_filter",
+			sizeof(struct txgbe_eth_l2_tunnel_conf_ele), 0);
+		if (!l2_tn_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		l2_tn_filter_ptr->filter_info = l2_tn_filter;
 		ret = txgbe_dev_l2_tunnel_filter_add(dev, &l2_tn_filter, FALSE);
-		if (!ret) {
-			l2_tn_filter_ptr = rte_zmalloc("txgbe_l2_tn_filter",
-				sizeof(struct txgbe_eth_l2_tunnel_conf_ele), 0);
-			if (!l2_tn_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&l2_tn_filter_ptr->filter_info,
-				&l2_tn_filter,
-				sizeof(struct txgbe_l2_tunnel_conf));
-			TAILQ_INSERT_TAIL(&filter_l2_tunnel_list,
-				l2_tn_filter_ptr, entries);
-			flow->rule = l2_tn_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_L2_TUNNEL;
-			return flow;
+		if (ret) {
+			rte_free(l2_tn_filter_ptr);
+			goto out;
 		}
-		goto out;
+		TAILQ_INSERT_TAIL(&filter_l2_tunnel_list,
+			l2_tn_filter_ptr, entries);
+		flow->rule = l2_tn_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_L2_TUNNEL;
+		return flow;
 	}
 
 	memset(&rss_conf, 0, sizeof(struct txgbe_rte_flow_rss_conf));
 	ret = txgbe_parse_rss_filter(dev, attr,
 					actions, &rss_conf, error);
 	if (!ret) {
-		ret = txgbe_config_rss_filter(dev, &rss_conf, TRUE);
-		if (!ret) {
-			rss_filter_ptr = rte_zmalloc("txgbe_rss_filter",
-				sizeof(struct txgbe_rss_conf_ele), 0);
-			if (!rss_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			txgbe_rss_conf_init(&rss_filter_ptr->filter_info,
-					    &rss_conf.conf);
-			TAILQ_INSERT_TAIL(&filter_rss_list,
-				rss_filter_ptr, entries);
-			flow->rule = rss_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_HASH;
-			return flow;
+		rss_filter_ptr = rte_zmalloc("txgbe_rss_filter",
+			sizeof(struct txgbe_rss_conf_ele), 0);
+		if (!rss_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
 		}
+		ret = txgbe_config_rss_filter(dev, &rss_conf, TRUE);
+		if (ret) {
+			rte_free(rss_filter_ptr);
+			goto out;
+		}
+		txgbe_rss_conf_init(&rss_filter_ptr->filter_info,
+				    &rss_conf.conf);
+		TAILQ_INSERT_TAIL(&filter_rss_list,
+			rss_filter_ptr, entries);
+		flow->rule = rss_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_HASH;
+		return flow;
 	}
 
 out:
-- 
2.53.0

[PATCH v2 3/3] net/txgbe: fix flow create error codes

[Zhang Tengfei]

txgbe_flow_create() reports failures with rte_flow_error_set(error, -ret).
The FDIR flex offset mismatch and mask-only paths jump to that handler
with ret == 0, so the application sees a failed create and errno 0. The
global mask memcmp path stores memcmp's positive result in ret, so -ret
is negative.

Set -EINVAL on these paths.

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

Signed-off-by: Zhang Tengfei <zhtfdev@gmail.com>
---
 drivers/net/txgbe/txgbe_flow.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/txgbe/txgbe_flow.c b/drivers/net/txgbe/txgbe_flow.c
index a1a497fa22..084b1eec1d 100644
--- a/drivers/net/txgbe/txgbe_flow.c
+++ b/drivers/net/txgbe/txgbe_flow.c
@@ -3388,12 +3388,12 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 				 * Only support one global mask,
 				 * all the masks should be the same.
 				 */
-				ret = memcmp(&fdir_info->mask,
+				if (memcmp(&fdir_info->mask,
 					&fdir_rule.mask,
-					sizeof(struct txgbe_hw_fdir_mask));
-				if (ret) {
+					sizeof(struct txgbe_hw_fdir_mask)) != 0) {
 					PMD_DRV_LOG(ERR, "only support one global mask");
 					rte_free(fdir_rule_ptr);
+					ret = -EINVAL;
 					goto out;
 				}
 
@@ -3402,6 +3402,7 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 				    fdir_info->flex_relative !=
 				    fdir_rule.flex_relative) {
 					rte_free(fdir_rule_ptr);
+					ret = -EINVAL;
 					goto out;
 				}
 			}
@@ -3431,6 +3432,7 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 		}
 
 		rte_free(fdir_rule_ptr);
+		ret = -EINVAL;
 		goto out;
 	}
 
-- 
2.53.0

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

[Stephen Hemminger]

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.

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

[Zhang Tengfei]

v3:
- reject mask-only FDIR before programming the global mask
- log flex offset mismatch
- no code change in 1/3 and 2/3

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 FDIR error handling on flow create

Zhang Tengfei (3):
  net/txgbe: fix L2 tunnel error on flow create
  net/txgbe: fix leak of filters on flow create
  net/txgbe: fix FDIR error handling on flow create

 drivers/net/txgbe/txgbe_flow.c | 266 +++++++++++++++++----------------
 1 file changed, 140 insertions(+), 126 deletions(-)

-- 
2.53.0

[PATCH v3 1/3] net/txgbe: fix L2 tunnel error on flow create

[Zhang Tengfei]

txgbe_flow_create() continues to RSS parsing when L2 tunnel filter
add fails. An E-tag VF/PF rule cannot match RSS, so the original
error is overwritten.

Fail the create on L2 add failure, same as ethertype and SYN.

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

Signed-off-by: Zhang Tengfei <zhtfdev@gmail.com>
---
 drivers/net/txgbe/txgbe_flow.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/txgbe/txgbe_flow.c b/drivers/net/txgbe/txgbe_flow.c
index eaeb973c91..76191a7c2d 100644
--- a/drivers/net/txgbe/txgbe_flow.c
+++ b/drivers/net/txgbe/txgbe_flow.c
@@ -3449,6 +3449,7 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 			flow->filter_type = RTE_ETH_FILTER_L2_TUNNEL;
 			return flow;
 		}
+		goto out;
 	}
 
 	memset(&rss_conf, 0, sizeof(struct txgbe_rte_flow_rss_conf));
-- 
2.53.0

[PATCH v3 2/3] net/txgbe: fix leak of filters on flow create

[Zhang Tengfei]

txgbe_flow_create() programs ntuple, ethertype, SYN, FDIR, L2 tunnel and
RSS filters into hardware before allocating the software copy.
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.
Allocate the FDIR object before installing the global mask so an
allocation failure cannot leave mask_added set with no rule.

Fixes: 5c2352b9ece6 ("net/txgbe: support creating consistent filter")
Fixes: 7eef71080e16 ("net/txgbe: switch to FDIR on VF")
Cc: stable@dpdk.org

Signed-off-by: Zhang Tengfei <zhtfdev@gmail.com>
---
 drivers/net/txgbe/txgbe_flow.c | 233 +++++++++++++++++----------------
 1 file changed, 121 insertions(+), 112 deletions(-)

diff --git a/drivers/net/txgbe/txgbe_flow.c b/drivers/net/txgbe/txgbe_flow.c
index 76191a7c2d..a1a497fa22 100644
--- a/drivers/net/txgbe/txgbe_flow.c
+++ b/drivers/net/txgbe/txgbe_flow.c
@@ -3,6 +3,7 @@
  * Copyright(c) 2010-2017 Intel Corporation
  */
 
+#include <errno.h>
 #include <sys/queue.h>
 #include <bus_pci_driver.h>
 #include <rte_malloc.h>
@@ -3246,26 +3247,26 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 #endif
 
 	if (!ret) {
+		ntuple_filter_ptr = rte_zmalloc("txgbe_ntuple_filter",
+			sizeof(struct txgbe_ntuple_filter_ele), 0);
+		if (!ntuple_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		ntuple_filter_ptr->filter_info = ntuple_filter;
 		ret = txgbe_add_del_ntuple_filter(dev, &ntuple_filter, TRUE);
-		if (!ret) {
-			ntuple_filter_ptr = rte_zmalloc("txgbe_ntuple_filter",
-				sizeof(struct txgbe_ntuple_filter_ele), 0);
-			if (!ntuple_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&ntuple_filter_ptr->filter_info,
-				&ntuple_filter,
-				sizeof(struct rte_eth_ntuple_filter));
-			TAILQ_INSERT_TAIL(&filter_ntuple_list,
-				ntuple_filter_ptr, entries);
-			flow->rule = ntuple_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_NTUPLE;
-			return flow;
-		} else if (filter_info->ntuple_is_full) {
-			goto next;
+		if (ret) {
+			rte_free(ntuple_filter_ptr);
+			if (filter_info->ntuple_is_full)
+				goto next;
+			goto out;
 		}
-		goto out;
+		TAILQ_INSERT_TAIL(&filter_ntuple_list,
+			ntuple_filter_ptr, entries);
+		flow->rule = ntuple_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_NTUPLE;
+		return flow;
 	}
 
 next:
@@ -3273,51 +3274,49 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 	ret = txgbe_parse_ethertype_filter(dev, attr, pattern,
 				actions, &ethertype_filter, error);
 	if (!ret) {
+		ethertype_filter_ptr = rte_zmalloc("txgbe_ethertype_filter",
+			sizeof(struct txgbe_ethertype_filter_ele), 0);
+		if (!ethertype_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		ethertype_filter_ptr->filter_info = ethertype_filter;
 		ret = txgbe_add_del_ethertype_filter(dev,
 				&ethertype_filter, TRUE);
-		if (!ret) {
-			ethertype_filter_ptr =
-				rte_zmalloc("txgbe_ethertype_filter",
-				sizeof(struct txgbe_ethertype_filter_ele), 0);
-			if (!ethertype_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&ethertype_filter_ptr->filter_info,
-				&ethertype_filter,
-				sizeof(struct rte_eth_ethertype_filter));
-			TAILQ_INSERT_TAIL(&filter_ethertype_list,
-				ethertype_filter_ptr, entries);
-			flow->rule = ethertype_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_ETHERTYPE;
-			return flow;
+		if (ret) {
+			rte_free(ethertype_filter_ptr);
+			goto out;
 		}
-		goto out;
+		TAILQ_INSERT_TAIL(&filter_ethertype_list,
+			ethertype_filter_ptr, entries);
+		flow->rule = ethertype_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_ETHERTYPE;
+		return flow;
 	}
 
 	memset(&syn_filter, 0, sizeof(struct rte_eth_syn_filter));
 	ret = txgbe_parse_syn_filter(dev, attr, pattern,
 				actions, &syn_filter, error);
 	if (!ret) {
+		syn_filter_ptr = rte_zmalloc("txgbe_syn_filter",
+			sizeof(struct txgbe_eth_syn_filter_ele), 0);
+		if (!syn_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		syn_filter_ptr->filter_info = syn_filter;
 		ret = txgbe_syn_filter_set(dev, &syn_filter, TRUE);
-		if (!ret) {
-			syn_filter_ptr = rte_zmalloc("txgbe_syn_filter",
-				sizeof(struct txgbe_eth_syn_filter_ele), 0);
-			if (!syn_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&syn_filter_ptr->filter_info,
-				&syn_filter,
-				sizeof(struct rte_eth_syn_filter));
-			TAILQ_INSERT_TAIL(&filter_syn_list,
-				syn_filter_ptr,
-				entries);
-			flow->rule = syn_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_SYN;
-			return flow;
+		if (ret) {
+			rte_free(syn_filter_ptr);
+			goto out;
 		}
-		goto out;
+		TAILQ_INSERT_TAIL(&filter_syn_list,
+			syn_filter_ptr, entries);
+		flow->rule = syn_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_SYN;
+		return flow;
 	}
 
 	memset(&fdir_rule, 0, sizeof(struct txgbe_fdir_rule));
@@ -3325,19 +3324,22 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 				actions, &fdir_rule, error);
 	if (!ret) {
 		if (!txgbe_is_pf(TXGBE_DEV_HW(dev))) {
-			ret = txgbevf_fdir_filter_program(dev, &fdir_rule, FALSE);
-			if (ret < 0)
-				goto out;
-
 			fdir_rule_ptr = rte_zmalloc("txgbe_fdir_filter",
-					    sizeof(struct txgbe_fdir_rule_ele), 0);
+					sizeof(struct txgbe_fdir_rule_ele), 0);
 			if (!fdir_rule_ptr) {
 				PMD_DRV_LOG(ERR, "failed to allocate memory");
+				ret = -ENOMEM;
 				goto out;
 			}
-			rte_memcpy(&fdir_rule_ptr->filter_info,
-				   &fdir_rule,
-				   sizeof(struct txgbe_fdir_rule));
+
+			ret = txgbevf_fdir_filter_program(dev, &fdir_rule,
+							  FALSE);
+			if (ret < 0) {
+				rte_free(fdir_rule_ptr);
+				goto out;
+			}
+
+			fdir_rule_ptr->filter_info = fdir_rule;
 			TAILQ_INSERT_TAIL(&filter_fdir_list,
 					  fdir_rule_ptr, entries);
 			flow->rule = fdir_rule_ptr;
@@ -3345,6 +3347,14 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 			return flow;
 		}
 
+		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;
+			goto out;
+		}
+
 		/* A mask cannot be deleted. */
 		if (fdir_rule.b_mask) {
 			if (!fdir_info->mask_added) {
@@ -3366,8 +3376,10 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 				fdir_info->mask.pkt_type_mask =
 					fdir_rule.mask.pkt_type_mask;
 				ret = txgbe_fdir_set_input_mask(dev);
-				if (ret)
+				if (ret) {
+					rte_free(fdir_rule_ptr);
 					goto out;
+				}
 
 				fdir_info->mask_added = TRUE;
 				first_mask = TRUE;
@@ -3381,40 +3393,25 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 					sizeof(struct txgbe_hw_fdir_mask));
 				if (ret) {
 					PMD_DRV_LOG(ERR, "only support one global mask");
+					rte_free(fdir_rule_ptr);
 					goto out;
 				}
 
 				if (fdir_info->flex_bytes_offset !=
 				    fdir_rule.flex_bytes_offset ||
 				    fdir_info->flex_relative !=
-				    fdir_rule.flex_relative)
+				    fdir_rule.flex_relative) {
+					rte_free(fdir_rule_ptr);
 					goto out;
+				}
 			}
 		}
 
 		if (fdir_rule.b_spec) {
 			ret = txgbe_fdir_filter_program(dev, &fdir_rule,
 					FALSE, FALSE);
-			if (!ret) {
-				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");
-					goto out;
-				}
-				rte_memcpy(&fdir_rule_ptr->filter_info,
-					&fdir_rule,
-					sizeof(struct txgbe_fdir_rule));
-				TAILQ_INSERT_TAIL(&filter_fdir_list,
-					fdir_rule_ptr, entries);
-				flow->rule = fdir_rule_ptr;
-				flow->filter_type = RTE_ETH_FILTER_FDIR;
-
-				return flow;
-			}
-
 			if (ret) {
+				rte_free(fdir_rule_ptr);
 				/**
 				 * clean the mask_added flag if fail to
 				 * program
@@ -3423,8 +3420,17 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 					fdir_info->mask_added = FALSE;
 				goto out;
 			}
+
+			fdir_rule_ptr->filter_info = fdir_rule;
+			TAILQ_INSERT_TAIL(&filter_fdir_list,
+				fdir_rule_ptr, entries);
+			flow->rule = fdir_rule_ptr;
+			flow->filter_type = RTE_ETH_FILTER_FDIR;
+
+			return flow;
 		}
 
+		rte_free(fdir_rule_ptr);
 		goto out;
 	}
 
@@ -3432,46 +3438,49 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 	ret = txgbe_parse_l2_tn_filter(dev, attr, pattern,
 					actions, &l2_tn_filter, error);
 	if (!ret) {
+		l2_tn_filter_ptr = rte_zmalloc("txgbe_l2_tn_filter",
+			sizeof(struct txgbe_eth_l2_tunnel_conf_ele), 0);
+		if (!l2_tn_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		l2_tn_filter_ptr->filter_info = l2_tn_filter;
 		ret = txgbe_dev_l2_tunnel_filter_add(dev, &l2_tn_filter, FALSE);
-		if (!ret) {
-			l2_tn_filter_ptr = rte_zmalloc("txgbe_l2_tn_filter",
-				sizeof(struct txgbe_eth_l2_tunnel_conf_ele), 0);
-			if (!l2_tn_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&l2_tn_filter_ptr->filter_info,
-				&l2_tn_filter,
-				sizeof(struct txgbe_l2_tunnel_conf));
-			TAILQ_INSERT_TAIL(&filter_l2_tunnel_list,
-				l2_tn_filter_ptr, entries);
-			flow->rule = l2_tn_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_L2_TUNNEL;
-			return flow;
+		if (ret) {
+			rte_free(l2_tn_filter_ptr);
+			goto out;
 		}
-		goto out;
+		TAILQ_INSERT_TAIL(&filter_l2_tunnel_list,
+			l2_tn_filter_ptr, entries);
+		flow->rule = l2_tn_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_L2_TUNNEL;
+		return flow;
 	}
 
 	memset(&rss_conf, 0, sizeof(struct txgbe_rte_flow_rss_conf));
 	ret = txgbe_parse_rss_filter(dev, attr,
 					actions, &rss_conf, error);
 	if (!ret) {
-		ret = txgbe_config_rss_filter(dev, &rss_conf, TRUE);
-		if (!ret) {
-			rss_filter_ptr = rte_zmalloc("txgbe_rss_filter",
-				sizeof(struct txgbe_rss_conf_ele), 0);
-			if (!rss_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			txgbe_rss_conf_init(&rss_filter_ptr->filter_info,
-					    &rss_conf.conf);
-			TAILQ_INSERT_TAIL(&filter_rss_list,
-				rss_filter_ptr, entries);
-			flow->rule = rss_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_HASH;
-			return flow;
+		rss_filter_ptr = rte_zmalloc("txgbe_rss_filter",
+			sizeof(struct txgbe_rss_conf_ele), 0);
+		if (!rss_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
 		}
+		ret = txgbe_config_rss_filter(dev, &rss_conf, TRUE);
+		if (ret) {
+			rte_free(rss_filter_ptr);
+			goto out;
+		}
+		txgbe_rss_conf_init(&rss_filter_ptr->filter_info,
+				    &rss_conf.conf);
+		TAILQ_INSERT_TAIL(&filter_rss_list,
+			rss_filter_ptr, entries);
+		flow->rule = rss_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_HASH;
+		return flow;
 	}
 
 out:
-- 
2.53.0

[PATCH v3 3/3] net/txgbe: fix FDIR error handling on flow create

[Zhang Tengfei]

On failure, txgbe_flow_create() calls rte_flow_error_set(error, -ret),
so ret must be a negative errno. The FDIR flex offset mismatch path
leaves ret at 0, and the application sees errno 0. The global mask
memcmp path stores memcmp's return value in ret, which is not an
errno. Set -EINVAL on both paths.

Reject a mask-only FDIR rule before programming the global input mask,
so a failed create cannot leave the mask committed.

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

Signed-off-by: Zhang Tengfei <zhtfdev@gmail.com>
---
 drivers/net/txgbe/txgbe_flow.c | 56 ++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/drivers/net/txgbe/txgbe_flow.c b/drivers/net/txgbe/txgbe_flow.c
index a1a497fa22..b4138cdc4e 100644
--- a/drivers/net/txgbe/txgbe_flow.c
+++ b/drivers/net/txgbe/txgbe_flow.c
@@ -3355,6 +3355,12 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 			goto out;
 		}
 
+		if (!fdir_rule.b_spec) {
+			rte_free(fdir_rule_ptr);
+			ret = -EINVAL;
+			goto out;
+		}
+
 		/* A mask cannot be deleted. */
 		if (fdir_rule.b_mask) {
 			if (!fdir_info->mask_added) {
@@ -3388,12 +3394,12 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 				 * Only support one global mask,
 				 * all the masks should be the same.
 				 */
-				ret = memcmp(&fdir_info->mask,
+				if (memcmp(&fdir_info->mask,
 					&fdir_rule.mask,
-					sizeof(struct txgbe_hw_fdir_mask));
-				if (ret) {
+					sizeof(struct txgbe_hw_fdir_mask)) != 0) {
 					PMD_DRV_LOG(ERR, "only support one global mask");
 					rte_free(fdir_rule_ptr);
+					ret = -EINVAL;
 					goto out;
 				}
 
@@ -3401,37 +3407,35 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 				    fdir_rule.flex_bytes_offset ||
 				    fdir_info->flex_relative !=
 				    fdir_rule.flex_relative) {
+					PMD_DRV_LOG(ERR,
+						"flex bytes offset mismatch");
 					rte_free(fdir_rule_ptr);
+					ret = -EINVAL;
 					goto out;
 				}
 			}
 		}
 
-		if (fdir_rule.b_spec) {
-			ret = txgbe_fdir_filter_program(dev, &fdir_rule,
-					FALSE, FALSE);
-			if (ret) {
-				rte_free(fdir_rule_ptr);
-				/**
-				 * clean the mask_added flag if fail to
-				 * program
-				 **/
-				if (first_mask)
-					fdir_info->mask_added = FALSE;
-				goto out;
-			}
-
-			fdir_rule_ptr->filter_info = fdir_rule;
-			TAILQ_INSERT_TAIL(&filter_fdir_list,
-				fdir_rule_ptr, entries);
-			flow->rule = fdir_rule_ptr;
-			flow->filter_type = RTE_ETH_FILTER_FDIR;
-
-			return flow;
+		ret = txgbe_fdir_filter_program(dev, &fdir_rule,
+				FALSE, FALSE);
+		if (ret) {
+			rte_free(fdir_rule_ptr);
+			/**
+			 * clean the mask_added flag if fail to
+			 * program
+			 **/
+			if (first_mask)
+				fdir_info->mask_added = FALSE;
+			goto out;
 		}
 
-		rte_free(fdir_rule_ptr);
-		goto out;
+		fdir_rule_ptr->filter_info = fdir_rule;
+		TAILQ_INSERT_TAIL(&filter_fdir_list,
+			fdir_rule_ptr, entries);
+		flow->rule = fdir_rule_ptr;
+		flow->filter_type = RTE_ETH_FILTER_FDIR;
+
+		return flow;
 	}
 
 	memset(&l2_tn_filter, 0, sizeof(struct txgbe_l2_tunnel_conf));
-- 
2.53.0

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

[Stephen Hemminger]

On Thu, 17 Sep 2026 23:32:39 +0800
Zhang Tengfei <zhtfdev@gmail.com> wrote:

> v3:
> - reject mask-only FDIR before programming the global mask
> - log flex offset mismatch
> - no code change in 1/3 and 2/3
> 
> 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 FDIR error handling on flow create
> 
> Zhang Tengfei (3):
>   net/txgbe: fix L2 tunnel error on flow create
>   net/txgbe: fix leak of filters on flow create
>   net/txgbe: fix FDIR error handling on flow create
> 
>  drivers/net/txgbe/txgbe_flow.c | 266 +++++++++++++++++----------------
>  1 file changed, 140 insertions(+), 126 deletions(-)
> 

I applied this to next-net.

AI review had some Info level observations, if you want to send
a new version, I will replace the one in next-net.

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

Applies to main, each of the three commits builds with -Dwerror=true.
Fixes tags resolve; both referenced commits are in released versions,
so Cc: stable is right.

Patches 1 and 2 are unchanged from v2 apart from commit message
wording. The v2 warning is addressed: mask-only FDIR rules are now
rejected before the global input mask is programmed, so a failed
create no longer leaves mask_added set. With that check in place the
b_spec block is unconditional, which reads better than the old
nesting. The memcmp and flex offset paths now set -EINVAL, and the
flex mismatch has a log message.


Patch 3/3: net/txgbe: fix FDIR error handling on flow create

Info: the b_spec check sits after the rte_zmalloc(), so it allocates
  and immediately frees on the reject path. Moving it above the
  allocation drops the rte_free() and one level of churn:

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

	fdir_rule_ptr = rte_zmalloc(...);

Info: the VF path still reaches "out" with a base driver status code.
  txgbevf_fdir_filter_program() propagates TXGBE_ERR_FEATURE_NOT_
  SUPPORTED (-292) and TXGBE_ERR_NOSUPP from txgbevf_set_fdir(), so
  rte_flow_error_set(error, -ret) gets 292, not an errno. The PF
  paths checked out: txgbe_fdir_set_input_mask() and
  txgbe_fdir_filter_program() return -ENOTSUP, -EINVAL, -ENOMEM or
  -ETIMEDOUT. Converting the VF codes is a separate patch.

Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>

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

[Zhang Tengfei]

Thanks for the review.

Moving the b_spec check above the allocation is better. I will send a v4
with that change.

The VF status-to-errno conversion will be a separate patch.

Out of curiosity, which AI review tool did you use? Mine did not flag
these Infos.

Thanks

On 9/19/26 00:06, Stephen Hemminger wrote:
> On Thu, 17 Sep 2026 23:32:39 +0800
> Zhang Tengfei <zhtfdev@gmail.com> wrote:
>
>> v3:
>> - reject mask-only FDIR before programming the global mask
>> - log flex offset mismatch
>> - no code change in 1/3 and 2/3
>>
>> 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 FDIR error handling on flow create
>>
>> Zhang Tengfei (3):
>>    net/txgbe: fix L2 tunnel error on flow create
>>    net/txgbe: fix leak of filters on flow create
>>    net/txgbe: fix FDIR error handling on flow create
>>
>>   drivers/net/txgbe/txgbe_flow.c | 266 +++++++++++++++++----------------
>>   1 file changed, 140 insertions(+), 126 deletions(-)
>>
> I applied this to next-net.
>
> AI review had some Info level observations, if you want to send
> a new version, I will replace the one in next-net.
>
> Review: [PATCH v3 0/3] net/txgbe: flow create fixes
> Author: Zhang Tengfei <zhtfdev@gmail.com>
>
> Applies to main, each of the three commits builds with -Dwerror=true.
> Fixes tags resolve; both referenced commits are in released versions,
> so Cc: stable is right.
>
> Patches 1 and 2 are unchanged from v2 apart from commit message
> wording. The v2 warning is addressed: mask-only FDIR rules are now
> rejected before the global input mask is programmed, so a failed
> create no longer leaves mask_added set. With that check in place the
> b_spec block is unconditional, which reads better than the old
> nesting. The memcmp and flex offset paths now set -EINVAL, and the
> flex mismatch has a log message.
>
>
> Patch 3/3: net/txgbe: fix FDIR error handling on flow create
>
> Info: the b_spec check sits after the rte_zmalloc(), so it allocates
>    and immediately frees on the reject path. Moving it above the
>    allocation drops the rte_free() and one level of churn:
>
> 	if (!fdir_rule.b_spec) {
> 		ret = -EINVAL;
> 		goto out;
> 	}
>
> 	fdir_rule_ptr = rte_zmalloc(...);
>
> Info: the VF path still reaches "out" with a base driver status code.
>    txgbevf_fdir_filter_program() propagates TXGBE_ERR_FEATURE_NOT_
>    SUPPORTED (-292) and TXGBE_ERR_NOSUPP from txgbevf_set_fdir(), so
>    rte_flow_error_set(error, -ret) gets 292, not an errno. The PF
>    paths checked out: txgbe_fdir_set_input_mask() and
>    txgbe_fdir_filter_program() return -ENOTSUP, -EINVAL, -ENOMEM or
>    -ETIMEDOUT. Converting the VF codes is a separate patch.
>
> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>

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

[Zhang Tengfei]

v4:
- reject mask-only FDIR before allocating the software object

v3:
- reject mask-only FDIR before programming the global mask
- log flex offset mismatch

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

1/3 fix L2 tunnel error on flow create
2/3 fix leak of filters on flow create
3/3 fix FDIR error handling on flow create

Zhang Tengfei (3):
  net/txgbe: fix L2 tunnel error on flow create
  net/txgbe: fix leak of filters on flow create
  net/txgbe: fix FDIR error handling on flow create

 drivers/net/txgbe/txgbe_flow.c | 265 +++++++++++++++++----------------
 1 file changed, 139 insertions(+), 126 deletions(-)

-- 
2.53.0

[PATCH v4 1/3] net/txgbe: fix L2 tunnel error on flow create

[Zhang Tengfei]

txgbe_flow_create() continues to RSS parsing when L2 tunnel filter
add fails. An E-tag VF/PF rule cannot match RSS, so the original
error is overwritten.

Fail the create on L2 add failure, same as ethertype and SYN.

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

Signed-off-by: Zhang Tengfei <zhtfdev@gmail.com>
---
 drivers/net/txgbe/txgbe_flow.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/txgbe/txgbe_flow.c b/drivers/net/txgbe/txgbe_flow.c
index eaeb973c91..76191a7c2d 100644
--- a/drivers/net/txgbe/txgbe_flow.c
+++ b/drivers/net/txgbe/txgbe_flow.c
@@ -3449,6 +3449,7 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 			flow->filter_type = RTE_ETH_FILTER_L2_TUNNEL;
 			return flow;
 		}
+		goto out;
 	}
 
 	memset(&rss_conf, 0, sizeof(struct txgbe_rte_flow_rss_conf));
-- 
2.53.0

[PATCH v4 2/3] net/txgbe: fix leak of filters on flow create

[Zhang Tengfei]

txgbe_flow_create() programs ntuple, ethertype, SYN, FDIR, L2 tunnel and
RSS filters into hardware before allocating the software copy.
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.
Allocate the FDIR object before installing the global mask so an
allocation failure cannot leave mask_added set with no rule.

Fixes: 5c2352b9ece6 ("net/txgbe: support creating consistent filter")
Fixes: 7eef71080e16 ("net/txgbe: switch to FDIR on VF")
Cc: stable@dpdk.org

Signed-off-by: Zhang Tengfei <zhtfdev@gmail.com>
---
 drivers/net/txgbe/txgbe_flow.c | 233 +++++++++++++++++----------------
 1 file changed, 121 insertions(+), 112 deletions(-)

diff --git a/drivers/net/txgbe/txgbe_flow.c b/drivers/net/txgbe/txgbe_flow.c
index 76191a7c2d..a1a497fa22 100644
--- a/drivers/net/txgbe/txgbe_flow.c
+++ b/drivers/net/txgbe/txgbe_flow.c
@@ -3,6 +3,7 @@
  * Copyright(c) 2010-2017 Intel Corporation
  */
 
+#include <errno.h>
 #include <sys/queue.h>
 #include <bus_pci_driver.h>
 #include <rte_malloc.h>
@@ -3246,26 +3247,26 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 #endif
 
 	if (!ret) {
+		ntuple_filter_ptr = rte_zmalloc("txgbe_ntuple_filter",
+			sizeof(struct txgbe_ntuple_filter_ele), 0);
+		if (!ntuple_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		ntuple_filter_ptr->filter_info = ntuple_filter;
 		ret = txgbe_add_del_ntuple_filter(dev, &ntuple_filter, TRUE);
-		if (!ret) {
-			ntuple_filter_ptr = rte_zmalloc("txgbe_ntuple_filter",
-				sizeof(struct txgbe_ntuple_filter_ele), 0);
-			if (!ntuple_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&ntuple_filter_ptr->filter_info,
-				&ntuple_filter,
-				sizeof(struct rte_eth_ntuple_filter));
-			TAILQ_INSERT_TAIL(&filter_ntuple_list,
-				ntuple_filter_ptr, entries);
-			flow->rule = ntuple_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_NTUPLE;
-			return flow;
-		} else if (filter_info->ntuple_is_full) {
-			goto next;
+		if (ret) {
+			rte_free(ntuple_filter_ptr);
+			if (filter_info->ntuple_is_full)
+				goto next;
+			goto out;
 		}
-		goto out;
+		TAILQ_INSERT_TAIL(&filter_ntuple_list,
+			ntuple_filter_ptr, entries);
+		flow->rule = ntuple_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_NTUPLE;
+		return flow;
 	}
 
 next:
@@ -3273,51 +3274,49 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 	ret = txgbe_parse_ethertype_filter(dev, attr, pattern,
 				actions, &ethertype_filter, error);
 	if (!ret) {
+		ethertype_filter_ptr = rte_zmalloc("txgbe_ethertype_filter",
+			sizeof(struct txgbe_ethertype_filter_ele), 0);
+		if (!ethertype_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		ethertype_filter_ptr->filter_info = ethertype_filter;
 		ret = txgbe_add_del_ethertype_filter(dev,
 				&ethertype_filter, TRUE);
-		if (!ret) {
-			ethertype_filter_ptr =
-				rte_zmalloc("txgbe_ethertype_filter",
-				sizeof(struct txgbe_ethertype_filter_ele), 0);
-			if (!ethertype_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&ethertype_filter_ptr->filter_info,
-				&ethertype_filter,
-				sizeof(struct rte_eth_ethertype_filter));
-			TAILQ_INSERT_TAIL(&filter_ethertype_list,
-				ethertype_filter_ptr, entries);
-			flow->rule = ethertype_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_ETHERTYPE;
-			return flow;
+		if (ret) {
+			rte_free(ethertype_filter_ptr);
+			goto out;
 		}
-		goto out;
+		TAILQ_INSERT_TAIL(&filter_ethertype_list,
+			ethertype_filter_ptr, entries);
+		flow->rule = ethertype_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_ETHERTYPE;
+		return flow;
 	}
 
 	memset(&syn_filter, 0, sizeof(struct rte_eth_syn_filter));
 	ret = txgbe_parse_syn_filter(dev, attr, pattern,
 				actions, &syn_filter, error);
 	if (!ret) {
+		syn_filter_ptr = rte_zmalloc("txgbe_syn_filter",
+			sizeof(struct txgbe_eth_syn_filter_ele), 0);
+		if (!syn_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		syn_filter_ptr->filter_info = syn_filter;
 		ret = txgbe_syn_filter_set(dev, &syn_filter, TRUE);
-		if (!ret) {
-			syn_filter_ptr = rte_zmalloc("txgbe_syn_filter",
-				sizeof(struct txgbe_eth_syn_filter_ele), 0);
-			if (!syn_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&syn_filter_ptr->filter_info,
-				&syn_filter,
-				sizeof(struct rte_eth_syn_filter));
-			TAILQ_INSERT_TAIL(&filter_syn_list,
-				syn_filter_ptr,
-				entries);
-			flow->rule = syn_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_SYN;
-			return flow;
+		if (ret) {
+			rte_free(syn_filter_ptr);
+			goto out;
 		}
-		goto out;
+		TAILQ_INSERT_TAIL(&filter_syn_list,
+			syn_filter_ptr, entries);
+		flow->rule = syn_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_SYN;
+		return flow;
 	}
 
 	memset(&fdir_rule, 0, sizeof(struct txgbe_fdir_rule));
@@ -3325,19 +3324,22 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 				actions, &fdir_rule, error);
 	if (!ret) {
 		if (!txgbe_is_pf(TXGBE_DEV_HW(dev))) {
-			ret = txgbevf_fdir_filter_program(dev, &fdir_rule, FALSE);
-			if (ret < 0)
-				goto out;
-
 			fdir_rule_ptr = rte_zmalloc("txgbe_fdir_filter",
-					    sizeof(struct txgbe_fdir_rule_ele), 0);
+					sizeof(struct txgbe_fdir_rule_ele), 0);
 			if (!fdir_rule_ptr) {
 				PMD_DRV_LOG(ERR, "failed to allocate memory");
+				ret = -ENOMEM;
 				goto out;
 			}
-			rte_memcpy(&fdir_rule_ptr->filter_info,
-				   &fdir_rule,
-				   sizeof(struct txgbe_fdir_rule));
+
+			ret = txgbevf_fdir_filter_program(dev, &fdir_rule,
+							  FALSE);
+			if (ret < 0) {
+				rte_free(fdir_rule_ptr);
+				goto out;
+			}
+
+			fdir_rule_ptr->filter_info = fdir_rule;
 			TAILQ_INSERT_TAIL(&filter_fdir_list,
 					  fdir_rule_ptr, entries);
 			flow->rule = fdir_rule_ptr;
@@ -3345,6 +3347,14 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 			return flow;
 		}
 
+		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;
+			goto out;
+		}
+
 		/* A mask cannot be deleted. */
 		if (fdir_rule.b_mask) {
 			if (!fdir_info->mask_added) {
@@ -3366,8 +3376,10 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 				fdir_info->mask.pkt_type_mask =
 					fdir_rule.mask.pkt_type_mask;
 				ret = txgbe_fdir_set_input_mask(dev);
-				if (ret)
+				if (ret) {
+					rte_free(fdir_rule_ptr);
 					goto out;
+				}
 
 				fdir_info->mask_added = TRUE;
 				first_mask = TRUE;
@@ -3381,40 +3393,25 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 					sizeof(struct txgbe_hw_fdir_mask));
 				if (ret) {
 					PMD_DRV_LOG(ERR, "only support one global mask");
+					rte_free(fdir_rule_ptr);
 					goto out;
 				}
 
 				if (fdir_info->flex_bytes_offset !=
 				    fdir_rule.flex_bytes_offset ||
 				    fdir_info->flex_relative !=
-				    fdir_rule.flex_relative)
+				    fdir_rule.flex_relative) {
+					rte_free(fdir_rule_ptr);
 					goto out;
+				}
 			}
 		}
 
 		if (fdir_rule.b_spec) {
 			ret = txgbe_fdir_filter_program(dev, &fdir_rule,
 					FALSE, FALSE);
-			if (!ret) {
-				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");
-					goto out;
-				}
-				rte_memcpy(&fdir_rule_ptr->filter_info,
-					&fdir_rule,
-					sizeof(struct txgbe_fdir_rule));
-				TAILQ_INSERT_TAIL(&filter_fdir_list,
-					fdir_rule_ptr, entries);
-				flow->rule = fdir_rule_ptr;
-				flow->filter_type = RTE_ETH_FILTER_FDIR;
-
-				return flow;
-			}
-
 			if (ret) {
+				rte_free(fdir_rule_ptr);
 				/**
 				 * clean the mask_added flag if fail to
 				 * program
@@ -3423,8 +3420,17 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 					fdir_info->mask_added = FALSE;
 				goto out;
 			}
+
+			fdir_rule_ptr->filter_info = fdir_rule;
+			TAILQ_INSERT_TAIL(&filter_fdir_list,
+				fdir_rule_ptr, entries);
+			flow->rule = fdir_rule_ptr;
+			flow->filter_type = RTE_ETH_FILTER_FDIR;
+
+			return flow;
 		}
 
+		rte_free(fdir_rule_ptr);
 		goto out;
 	}
 
@@ -3432,46 +3438,49 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 	ret = txgbe_parse_l2_tn_filter(dev, attr, pattern,
 					actions, &l2_tn_filter, error);
 	if (!ret) {
+		l2_tn_filter_ptr = rte_zmalloc("txgbe_l2_tn_filter",
+			sizeof(struct txgbe_eth_l2_tunnel_conf_ele), 0);
+		if (!l2_tn_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
+		}
+		l2_tn_filter_ptr->filter_info = l2_tn_filter;
 		ret = txgbe_dev_l2_tunnel_filter_add(dev, &l2_tn_filter, FALSE);
-		if (!ret) {
-			l2_tn_filter_ptr = rte_zmalloc("txgbe_l2_tn_filter",
-				sizeof(struct txgbe_eth_l2_tunnel_conf_ele), 0);
-			if (!l2_tn_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			rte_memcpy(&l2_tn_filter_ptr->filter_info,
-				&l2_tn_filter,
-				sizeof(struct txgbe_l2_tunnel_conf));
-			TAILQ_INSERT_TAIL(&filter_l2_tunnel_list,
-				l2_tn_filter_ptr, entries);
-			flow->rule = l2_tn_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_L2_TUNNEL;
-			return flow;
+		if (ret) {
+			rte_free(l2_tn_filter_ptr);
+			goto out;
 		}
-		goto out;
+		TAILQ_INSERT_TAIL(&filter_l2_tunnel_list,
+			l2_tn_filter_ptr, entries);
+		flow->rule = l2_tn_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_L2_TUNNEL;
+		return flow;
 	}
 
 	memset(&rss_conf, 0, sizeof(struct txgbe_rte_flow_rss_conf));
 	ret = txgbe_parse_rss_filter(dev, attr,
 					actions, &rss_conf, error);
 	if (!ret) {
-		ret = txgbe_config_rss_filter(dev, &rss_conf, TRUE);
-		if (!ret) {
-			rss_filter_ptr = rte_zmalloc("txgbe_rss_filter",
-				sizeof(struct txgbe_rss_conf_ele), 0);
-			if (!rss_filter_ptr) {
-				PMD_DRV_LOG(ERR, "failed to allocate memory");
-				goto out;
-			}
-			txgbe_rss_conf_init(&rss_filter_ptr->filter_info,
-					    &rss_conf.conf);
-			TAILQ_INSERT_TAIL(&filter_rss_list,
-				rss_filter_ptr, entries);
-			flow->rule = rss_filter_ptr;
-			flow->filter_type = RTE_ETH_FILTER_HASH;
-			return flow;
+		rss_filter_ptr = rte_zmalloc("txgbe_rss_filter",
+			sizeof(struct txgbe_rss_conf_ele), 0);
+		if (!rss_filter_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			ret = -ENOMEM;
+			goto out;
 		}
+		ret = txgbe_config_rss_filter(dev, &rss_conf, TRUE);
+		if (ret) {
+			rte_free(rss_filter_ptr);
+			goto out;
+		}
+		txgbe_rss_conf_init(&rss_filter_ptr->filter_info,
+				    &rss_conf.conf);
+		TAILQ_INSERT_TAIL(&filter_rss_list,
+			rss_filter_ptr, entries);
+		flow->rule = rss_filter_ptr;
+		flow->filter_type = RTE_ETH_FILTER_HASH;
+		return flow;
 	}
 
 out:
-- 
2.53.0

[PATCH v4 3/3] net/txgbe: fix FDIR error handling on flow create

[Zhang Tengfei]

On failure, txgbe_flow_create() calls rte_flow_error_set(error, -ret),
so ret must be a negative errno. The FDIR flex offset mismatch path
leaves ret at 0, and the application sees errno 0. The global mask
memcmp path stores memcmp's return value in ret, which is not an
errno. Set -EINVAL on both paths.

Reject a mask-only FDIR rule before allocating the software object or
programming the global input mask, so a failed create cannot leave the
mask committed.

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

Signed-off-by: Zhang Tengfei <zhtfdev@gmail.com>
---
 drivers/net/txgbe/txgbe_flow.c | 55 ++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/drivers/net/txgbe/txgbe_flow.c b/drivers/net/txgbe/txgbe_flow.c
index a1a497fa22..f8d8c6850d 100644
--- a/drivers/net/txgbe/txgbe_flow.c
+++ b/drivers/net/txgbe/txgbe_flow.c
@@ -3347,6 +3347,11 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 			return flow;
 		}
 
+		if (!fdir_rule.b_spec) {
+			ret = -EINVAL;
+			goto out;
+		}
+
 		fdir_rule_ptr = rte_zmalloc("txgbe_fdir_filter",
 				sizeof(struct txgbe_fdir_rule_ele), 0);
 		if (!fdir_rule_ptr) {
@@ -3388,12 +3393,12 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 				 * Only support one global mask,
 				 * all the masks should be the same.
 				 */
-				ret = memcmp(&fdir_info->mask,
+				if (memcmp(&fdir_info->mask,
 					&fdir_rule.mask,
-					sizeof(struct txgbe_hw_fdir_mask));
-				if (ret) {
+					sizeof(struct txgbe_hw_fdir_mask)) != 0) {
 					PMD_DRV_LOG(ERR, "only support one global mask");
 					rte_free(fdir_rule_ptr);
+					ret = -EINVAL;
 					goto out;
 				}
 
@@ -3401,37 +3406,35 @@ txgbe_flow_create(struct rte_eth_dev *dev,
 				    fdir_rule.flex_bytes_offset ||
 				    fdir_info->flex_relative !=
 				    fdir_rule.flex_relative) {
+					PMD_DRV_LOG(ERR,
+						"flex bytes offset mismatch");
 					rte_free(fdir_rule_ptr);
+					ret = -EINVAL;
 					goto out;
 				}
 			}
 		}
 
-		if (fdir_rule.b_spec) {
-			ret = txgbe_fdir_filter_program(dev, &fdir_rule,
-					FALSE, FALSE);
-			if (ret) {
-				rte_free(fdir_rule_ptr);
-				/**
-				 * clean the mask_added flag if fail to
-				 * program
-				 **/
-				if (first_mask)
-					fdir_info->mask_added = FALSE;
-				goto out;
-			}
-
-			fdir_rule_ptr->filter_info = fdir_rule;
-			TAILQ_INSERT_TAIL(&filter_fdir_list,
-				fdir_rule_ptr, entries);
-			flow->rule = fdir_rule_ptr;
-			flow->filter_type = RTE_ETH_FILTER_FDIR;
-
-			return flow;
+		ret = txgbe_fdir_filter_program(dev, &fdir_rule,
+				FALSE, FALSE);
+		if (ret) {
+			rte_free(fdir_rule_ptr);
+			/**
+			 * clean the mask_added flag if fail to
+			 * program
+			 **/
+			if (first_mask)
+				fdir_info->mask_added = FALSE;
+			goto out;
 		}
 
-		rte_free(fdir_rule_ptr);
-		goto out;
+		fdir_rule_ptr->filter_info = fdir_rule;
+		TAILQ_INSERT_TAIL(&filter_fdir_list,
+			fdir_rule_ptr, entries);
+		flow->rule = fdir_rule_ptr;
+		flow->filter_type = RTE_ETH_FILTER_FDIR;
+
+		return flow;
 	}
 
 	memset(&l2_tn_filter, 0, sizeof(struct txgbe_l2_tunnel_conf));
-- 
2.53.0