net/txgbe: fix leak of filters on flow create
PATCH 1/1 net/txgbe: fix leak of filters on flow create
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.
--- drivers/net/txgbe/txgbe_flow.c | 227 ++++++++++++++++++--------------- 1 file changed, 122 insertions(+), 105 deletions(-)
drivers/net/txgbe/txgbe_flow.c +122 -105
@@ -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, ðertype_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 (ðertype_filter_ptr->filter_info, + ðertype_filter, +sizeof (struct rte_eth_ethertype_filter )); ret =txgbe_add_del_ethertype_filter (dev, ðertype_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 (ðertype_filter_ptr->filter_info, - ðertype_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:
Signature · 1 line
2.53.0
Quoted 13 lines · On Tue, 15 Sep 2026 23:24:36 +0800 Zhang Tengfei <[email protected]> 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.
Quoted 5 lines
Fixes 5c2352b9ece6 ("net/txgbe: support creating consistent filter")Signed-off-by Zhang Tengfei <[email protected]>---
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:
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.
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/[email protected]/
I will send a v2 for txgbe.
Quoted 94 lines · Stephen Hemminger
Quoted 13 lines · On Tue, 15 Sep 2026 23:24:36 +0800 Zhang Tengfei <[email protected]> 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.
Quoted 5 lines
Fixes 5c2352b9ece6 ("net/txgbe: support creating consistent filter")Signed-off-by Zhang Tengfei <[email protected]>---
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: 169627Applied 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_addednot 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_maskis TRUE andfdir_info->mask_addedhas been set beforerte_zmalloc()runs. On allocation failure the code jumps to out without clearingmask_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_ptrallocation above the mask block (free it on the mask error paths), or clearmask_addedwhenfirst_maskis 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: theflex_bytes_offset/flex_relativemismatch, and the trailing goto out for an FDIR rule withoutb_spec. Thememcmp()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.chas the same program-then- allocate pattern inixgbe_flow_create(). txgbe was derived from it, the same fix applies there.
v2 cover net/txgbe: fix flow create errors
In reply to Zhang Tengfeiv2:
- 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(-)
Signature · 1 line
2.53.0
PATCH v2 1/3 net/txgbe: fix L2 tunnel error on flow create
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.
--- drivers/net/txgbe/txgbe_flow.c | 1 + 1 file changed, 1 insertion(+)
drivers/net/txgbe/txgbe_flow.c +1 -0
@@ -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 ));
Signature · 1 line
2.53.0
PATCH v2 2/3 net/txgbe: fix leak of filters on flow create
In reply to Zhang Tengfeitxgbe_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.
--- drivers/net/txgbe/txgbe_flow.c | 233 +++++++++++++++++---------------- 1 file changed, 121 insertions(+), 112 deletions(-)
drivers/net/txgbe/txgbe_flow.c +121 -112
@@ -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, ðertype_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, ðertype_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 (ðertype_filter_ptr->filter_info, - ðertype_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:
Signature · 1 line
2.53.0
PATCH v2 3/3 net/txgbe: fix flow create error codes
In reply to Zhang Tengfeitxgbe_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.
--- drivers/net/txgbe/txgbe_flow.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/net/txgbe/txgbe_flow.c +5 -3
@@ -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; }
Signature · 1 line
2.53.0
Re: [PATCH v2 0/3] net/txgbe: fix flow create errors
In reply to Zhang TengfeiQuoted 11 lines · On Wed, 16 Sep 2026 21:11:02 +0800 Zhang Tengfei <[email protected]> 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-EINVALon remaining FDIR goto out paths with bad ret
- use struct assignment instead ofrte_memcpyfor filter copies1/3 fix L2 tunnel error on flow create
2/3 fix leak of filters on flow create
3/3 fix flow create error codes
Quoted 8 lines
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 codesdrivers/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 <[email protected]>
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.
v3 cover net/txgbe: fix flow create errors
In reply to Zhang Tengfeiv3:
- 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(-)
Signature · 1 line
2.53.0
PATCH v3 1/3 net/txgbe: fix L2 tunnel error on flow create
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.
--- drivers/net/txgbe/txgbe_flow.c | 1 + 1 file changed, 1 insertion(+)
drivers/net/txgbe/txgbe_flow.c +1 -0
@@ -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 ));
Signature · 1 line
2.53.0
PATCH v3 2/3 net/txgbe: fix leak of filters on flow create
In reply to Zhang Tengfeitxgbe_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.
--- drivers/net/txgbe/txgbe_flow.c | 233 +++++++++++++++++---------------- 1 file changed, 121 insertions(+), 112 deletions(-)
drivers/net/txgbe/txgbe_flow.c +121 -112
@@ -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, ðertype_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, ðertype_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 (ðertype_filter_ptr->filter_info, - ðertype_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:
Signature · 1 line
2.53.0
PATCH v3 3/3 net/txgbe: fix FDIR error handling on flow create
In reply to Zhang TengfeiOn 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.
--- drivers/net/txgbe/txgbe_flow.c | 56 ++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 26 deletions(-)
drivers/net/txgbe/txgbe_flow.c +30 -26
@@ -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 ));
Signature · 1 line
2.53.0
Re: [PATCH v3 0/3] net/txgbe: fix flow create errors
In reply to Zhang TengfeiQuoted 16 lines · On Thu, 17 Sep 2026 23:32:39 +0800 Zhang Tengfei <[email protected]> wrote:
v3:
- reject mask-only FDIR before programming the global mask
- log flex offset mismatch
- no code change in 1/3 and 2/3v2:
- 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-EINVALon remaining FDIR goto out paths with bad ret
- use struct assignment instead ofrte_memcpyfor filter copies1/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
Quoted 8 lines
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 createdrivers/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 <[email protected]>
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.
Re: [PATCH v3 0/3] net/txgbe: fix flow create errors
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
Quoted 70 lines · Stephen Hemminger
Quoted 16 lines · On Thu, 17 Sep 2026 23:32:39 +0800 Zhang Tengfei <[email protected]> wrote:
v3:
- reject mask-only FDIR before programming the global mask
- log flex offset mismatch
- no code change in 1/3 and 2/3v2:
- 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-EINVALon remaining FDIR goto out paths with bad ret
- use struct assignment instead ofrte_memcpyfor filter copies1/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 createQuoted 8 lines
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 createdrivers/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 <[email protected]>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_addedset. With that check in place theb_specblock 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_speccheck sits after therte_zmalloc(), so it allocates
and immediately frees on the reject path. Moving it above the allocation drops therte_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()propagatesTXGBE_ERR_FEATURE_NOT_SUPPORTED (-292) andTXGBE_ERR_NOSUPPfromtxgbevf_set_fdir(), sorte_flow_error_set(error, -ret)gets 292, not an errno. The PF paths checked out:txgbe_fdir_set_input_mask()andtxgbe_fdir_filter_program()return-ENOTSUP,-EINVAL,-ENOMEMor-ETIMEDOUT. Converting the VF codes is a separate patch.Reviewed-by Stephen Hemminger <[email protected]>
v4 cover net/txgbe: fix flow create errors
In reply to Zhang Tengfeiv4:
- 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(-)
Signature · 1 line
2.53.0
PATCH v4 1/3 net/txgbe: fix L2 tunnel error on flow create
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.
--- drivers/net/txgbe/txgbe_flow.c | 1 + 1 file changed, 1 insertion(+)
drivers/net/txgbe/txgbe_flow.c +1 -0
@@ -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 ));
Signature · 1 line
2.53.0
PATCH v4 2/3 net/txgbe: fix leak of filters on flow create
In reply to Zhang Tengfeitxgbe_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.
--- drivers/net/txgbe/txgbe_flow.c | 233 +++++++++++++++++---------------- 1 file changed, 121 insertions(+), 112 deletions(-)
drivers/net/txgbe/txgbe_flow.c +121 -112
@@ -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, ðertype_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, ðertype_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 (ðertype_filter_ptr->filter_info, - ðertype_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:
Signature · 1 line
2.53.0
PATCH v4 3/3 net/txgbe: fix FDIR error handling on flow create
In reply to Zhang TengfeiOn 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.
--- drivers/net/txgbe/txgbe_flow.c | 55 ++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 26 deletions(-)
drivers/net/txgbe/txgbe_flow.c +29 -26
@@ -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 ));
Signature · 1 line
2.53.0