net/ixgbe: fix leak of filters on flow create
PATCH 1/1 net/ixgbe: fix leak of filters on flow create
ixgbe_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/intel/ixgbe/ixgbe_flow.c | 164 +++++++++++++++------------ 1 file changed, 90 insertions(+), 74 deletions(-)
drivers/net/intel/ixgbe/ixgbe_flow.c +90 -74
@@ -2858,68 +2858,73 @@ ixgbe_flow_create(struct rte_eth_dev *dev, actions, &ntuple_filter, error);if (!ret) { + ntuple_filter_ptr =rte_zmalloc ("ixgbe_ntuple_filter" , +sizeof (struct ixgbe_ntuple_filter_ele ),0 ); +if (!ntuple_filter_ptr) { +PMD_DRV_LOG (ERR ,"failed to allocate memory" ); + ret = -ENOMEM ; + goto out; + } +memcpy (&ntuple_filter_ptr->filter_info, + &ntuple_filter, +sizeof (struct rte_eth_ntuple_filter )); ret =ixgbe_add_del_ntuple_filter (adapter, &ntuple_filter, TRUE); -if (!ret) { - ntuple_filter_ptr =rte_zmalloc ("ixgbe_ntuple_filter" , -sizeof (struct ixgbe_ntuple_filter_ele ),0 ); -if (!ntuple_filter_ptr) { -PMD_DRV_LOG (ERR ,"failed to allocate memory" ); - goto out; - } -memcpy (&ntuple_filter_ptr->filter_info, - &ntuple_filter, -sizeof (struct rte_eth_ntuple_filter )); - flow->rule = ntuple_filter_ptr; - flow->filter_type =RTE_ETH_FILTER_NTUPLE ; -return flow; +if (ret) { +rte_free (ntuple_filter_ptr); + goto out; } - goto out; + flow->rule = ntuple_filter_ptr; + flow->filter_type =RTE_ETH_FILTER_NTUPLE ; +return flow; }memset (ðertype_filter,0 ,sizeof (struct rte_eth_ethertype_filter )); ret =ixgbe_parse_ethertype_filter (dev, attr, pattern, actions, ðertype_filter, error);if (!ret) { + ethertype_filter_ptr =rte_zmalloc ("ixgbe_ethertype_filter" , +sizeof (struct ixgbe_ethertype_filter_ele ),0 ); +if (!ethertype_filter_ptr) { +PMD_DRV_LOG (ERR ,"failed to allocate memory" ); + ret = -ENOMEM ; + goto out; + } +memcpy (ðertype_filter_ptr->filter_info, + ðertype_filter, +sizeof (struct rte_eth_ethertype_filter )); ret =ixgbe_add_del_ethertype_filter (adapter, ðertype_filter, TRUE); -if (!ret) { - ethertype_filter_ptr =rte_zmalloc ( -"ixgbe_ethertype_filter" , -sizeof (struct ixgbe_ethertype_filter_ele ),0 ); -if (!ethertype_filter_ptr) { -PMD_DRV_LOG (ERR ,"failed to allocate memory" ); - goto out; - } -memcpy (ðertype_filter_ptr->filter_info, - ðertype_filter, -sizeof (struct rte_eth_ethertype_filter )); - 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; + 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 =ixgbe_parse_syn_filter (dev, attr, pattern, actions, &syn_filter, error);if (!ret) { + syn_filter_ptr =rte_zmalloc ("ixgbe_syn_filter" , +sizeof (struct ixgbe_eth_syn_filter_ele ),0 ); +if (!syn_filter_ptr) { +PMD_DRV_LOG (ERR ,"failed to allocate memory" ); + ret = -ENOMEM ; + goto out; + } +memcpy (&syn_filter_ptr->filter_info, + &syn_filter, +sizeof (struct rte_eth_syn_filter )); ret =ixgbe_syn_filter_set (adapter, &syn_filter, TRUE); -if (!ret) { - syn_filter_ptr =rte_zmalloc ("ixgbe_syn_filter" , -sizeof (struct ixgbe_eth_syn_filter_ele ),0 ); -if (!syn_filter_ptr) { -PMD_DRV_LOG (ERR ,"failed to allocate memory" ); - goto out; - } -memcpy (&syn_filter_ptr->filter_info, - &syn_filter, -sizeof (struct rte_eth_syn_filter )); - 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; + flow->rule = syn_filter_ptr; + flow->filter_type =RTE_ETH_FILTER_SYN ; +return flow; }memset (&fdir_rule,0 ,sizeof (struct ixgbe_fdir_rule ));
@@ -2929,17 +2934,21 @@ ixgbe_flow_create(struct rte_eth_dev *dev,struct rte_eth_fdir_conf *fdir_conf =IXGBE_DEV_FDIR_CONF (dev);bool first_mask = false; - ret =ixgbe_fdir_flow_program (dev, adapter, &fdir_rule, - &first_mask, error); -if (ret) - goto out; - fdir_rule_ptr =rte_zmalloc ("ixgbe_fdir_filter" ,sizeof (struct ixgbe_fdir_rule_ele ),0 );if (!fdir_rule_ptr) {PMD_DRV_LOG (ERR ,"failed to allocate memory" ); + ret = -ENOMEM ; + goto out; + } + + ret =ixgbe_fdir_flow_program (dev, adapter, &fdir_rule, + &first_mask, error); +if (ret) { +rte_free (fdir_rule_ptr); goto out; } +/* update global state */ if (first_mask) { fdir_info->mask_added = TRUE;
@@ -2961,41 +2970,48 @@ ixgbe_flow_create(struct rte_eth_dev *dev, ret =ixgbe_parse_l2_tn_filter (dev, attr, pattern, actions, &l2_tn_filter, error);if (!ret) { - ret =ixgbe_dev_l2_tunnel_filter_add (adapter, &l2_tn_filter, FALSE); -if (!ret) { - l2_tn_filter_ptr =rte_zmalloc ("ixgbe_l2_tn_filter" , -sizeof (struct ixgbe_eth_l2_tunnel_conf_ele ),0 ); -if (!l2_tn_filter_ptr) { -PMD_DRV_LOG (ERR ,"failed to allocate memory" ); - goto out; - } -memcpy (&l2_tn_filter_ptr->filter_info, - &l2_tn_filter, -sizeof (struct ixgbe_l2_tunnel_conf )); - flow->rule = l2_tn_filter_ptr; - flow->filter_type =RTE_ETH_FILTER_L2_TUNNEL ; -return flow; + l2_tn_filter_ptr =rte_zmalloc ("ixgbe_l2_tn_filter" , +sizeof (struct ixgbe_eth_l2_tunnel_conf_ele ),0 ); +if (!l2_tn_filter_ptr) { +PMD_DRV_LOG (ERR ,"failed to allocate memory" ); + ret = -ENOMEM ; + goto out; } +memcpy (&l2_tn_filter_ptr->filter_info, + &l2_tn_filter, +sizeof (struct ixgbe_l2_tunnel_conf )); + ret =ixgbe_dev_l2_tunnel_filter_add (adapter, &l2_tn_filter, + FALSE); +if (ret) { +rte_free (l2_tn_filter_ptr); + goto out; + } + flow->rule = l2_tn_filter_ptr; + flow->filter_type =RTE_ETH_FILTER_L2_TUNNEL ; +return flow; }memset (&rss_conf,0 ,sizeof (struct ixgbe_rte_flow_rss_conf )); ret =ixgbe_parse_rss_filter (dev, attr, actions, &rss_conf, error);if (!ret) { + rss_filter_ptr =rte_zmalloc ("ixgbe_rss_filter" , +sizeof (struct ixgbe_rss_conf_ele ),0 ); +if (!rss_filter_ptr) { +PMD_DRV_LOG (ERR ,"failed to allocate memory" ); + ret = -ENOMEM ; + goto out; + } ret =ixgbe_config_rss_filter (adapter, &rss_conf, TRUE); -if (!ret) { - rss_filter_ptr =rte_zmalloc ("ixgbe_rss_filter" , -sizeof (struct ixgbe_rss_conf_ele ),0 ); -if (!rss_filter_ptr) { -PMD_DRV_LOG (ERR ,"failed to allocate memory" ); - goto out; - } -ixgbe_rss_conf_init (&rss_filter_ptr->filter_info, - &rss_conf.conf); - flow->rule = rss_filter_ptr; - flow->filter_type =RTE_ETH_FILTER_HASH ; -return flow; +if (ret) { +rte_free (rss_filter_ptr); + goto out; } +ixgbe_rss_conf_init (&rss_filter_ptr->filter_info, + &rss_conf.conf); + flow->rule = rss_filter_ptr; + flow->filter_type =RTE_ETH_FILTER_HASH ; +return flow; } out:
Signature · 1 line
2.55.0
Quoted 13 lines · Zhang Tengfei
ixgbe_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 72c135a89f80 ("net/ixgbe: create consistent filter")Signed-off-by Zhang Tengfei <[email protected]>---
This patch needs rebase though. The memcpy calls in the driver have been replaced by struct assignments. Will try updating on apply.
Quoted 229 lines
drivers/net/intel/ixgbe/ixgbe_flow.c | 164 +++++++++++++++------------ 1 file changed, 90 insertions(+), 74 deletions(-)drivers/net/intel/ixgbe/ixgbe_flow.c +90
-74@@ -2858,68 +2858,73 @@ ixgbe_flow_create(struct rte_eth_dev *dev, actions, &ntuple_filter, error);if (!ret) { + ntuple_filter_ptr =rte_zmalloc ("ixgbe_ntuple_filter" , +sizeof (struct ixgbe_ntuple_filter_ele ),0 ); +if (!ntuple_filter_ptr) { +PMD_DRV_LOG (ERR ,"failed to allocate memory" ); + ret = -ENOMEM ; + goto out; + } +memcpy (&ntuple_filter_ptr->filter_info, + &ntuple_filter, +sizeof (struct rte_eth_ntuple_filter )); ret =ixgbe_add_del_ntuple_filter (adapter, &ntuple_filter, TRUE); -if (!ret) { - ntuple_filter_ptr =rte_zmalloc ("ixgbe_ntuple_filter" , -sizeof (struct ixgbe_ntuple_filter_ele ),0 ); -if (!ntuple_filter_ptr) { -PMD_DRV_LOG (ERR ,"failed to allocate memory" ); - goto out; - } -memcpy (&ntuple_filter_ptr->filter_info, - &ntuple_filter, -sizeof (struct rte_eth_ntuple_filter )); - flow->rule = ntuple_filter_ptr; - flow->filter_type =RTE_ETH_FILTER_NTUPLE ; -return flow; +if (ret) { +rte_free (ntuple_filter_ptr); + goto out; } - goto out; + flow->rule = ntuple_filter_ptr; + flow->filter_type =RTE_ETH_FILTER_NTUPLE ; +return flow; }memset (ðertype_filter,0 ,sizeof (struct rte_eth_ethertype_filter )); ret =ixgbe_parse_ethertype_filter (dev, attr, pattern, actions, ðertype_filter, error);if (!ret) { + ethertype_filter_ptr =rte_zmalloc ("ixgbe_ethertype_filter" , +sizeof (struct ixgbe_ethertype_filter_ele ),0 ); +if (!ethertype_filter_ptr) { +PMD_DRV_LOG (ERR ,"failed to allocate memory" ); + ret = -ENOMEM ; + goto out; + } +memcpy (ðertype_filter_ptr->filter_info, + ðertype_filter, +sizeof (struct rte_eth_ethertype_filter )); ret =ixgbe_add_del_ethertype_filter (adapter, ðertype_filter, TRUE); -if (!ret) { - ethertype_filter_ptr =rte_zmalloc ( -"ixgbe_ethertype_filter" , -sizeof (struct ixgbe_ethertype_filter_ele ),0 ); -if (!ethertype_filter_ptr) { -PMD_DRV_LOG (ERR ,"failed to allocate memory" ); - goto out; - } -memcpy (ðertype_filter_ptr->filter_info, - ðertype_filter, -sizeof (struct rte_eth_ethertype_filter )); - 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; + 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 =ixgbe_parse_syn_filter (dev, attr, pattern, actions, &syn_filter, error);if (!ret) { + syn_filter_ptr =rte_zmalloc ("ixgbe_syn_filter" , +sizeof (struct ixgbe_eth_syn_filter_ele ),0 ); +if (!syn_filter_ptr) { +PMD_DRV_LOG (ERR ,"failed to allocate memory" ); + ret = -ENOMEM ; + goto out; + } +memcpy (&syn_filter_ptr->filter_info, + &syn_filter, +sizeof (struct rte_eth_syn_filter )); ret =ixgbe_syn_filter_set (adapter, &syn_filter, TRUE); -if (!ret) { - syn_filter_ptr =rte_zmalloc ("ixgbe_syn_filter" , -sizeof (struct ixgbe_eth_syn_filter_ele ),0 ); -if (!syn_filter_ptr) { -PMD_DRV_LOG (ERR ,"failed to allocate memory" ); - goto out; - } -memcpy (&syn_filter_ptr->filter_info, - &syn_filter, -sizeof (struct rte_eth_syn_filter )); - 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; + flow->rule = syn_filter_ptr; + flow->filter_type =RTE_ETH_FILTER_SYN ; +return flow; }memset (&fdir_rule,0 ,sizeof (struct ixgbe_fdir_rule ));@@ -2929,17 +2934,21 @@ ixgbe_flow_create(struct rte_eth_dev *dev,struct rte_eth_fdir_conf *fdir_conf =IXGBE_DEV_FDIR_CONF (dev);bool first_mask = false; - ret =ixgbe_fdir_flow_program (dev, adapter, &fdir_rule, - &first_mask, error); -if (ret) - goto out; - fdir_rule_ptr =rte_zmalloc ("ixgbe_fdir_filter" ,sizeof (struct ixgbe_fdir_rule_ele ),0 );if (!fdir_rule_ptr) {PMD_DRV_LOG (ERR ,"failed to allocate memory" ); + ret = -ENOMEM ; + goto out; + } + + ret =ixgbe_fdir_flow_program (dev, adapter, &fdir_rule, + &first_mask, error); +if (ret) { +rte_free (fdir_rule_ptr); goto out; } +/* update global state */ if (first_mask) { fdir_info->mask_added = TRUE;@@ -2961,41 +2970,48 @@ ixgbe_flow_create(struct rte_eth_dev *dev, ret =ixgbe_parse_l2_tn_filter (dev, attr, pattern, actions, &l2_tn_filter, error);if (!ret) { - ret =ixgbe_dev_l2_tunnel_filter_add (adapter, &l2_tn_filter, FALSE); -if (!ret) { - l2_tn_filter_ptr =rte_zmalloc ("ixgbe_l2_tn_filter" , -sizeof (struct ixgbe_eth_l2_tunnel_conf_ele ),0 ); -if (!l2_tn_filter_ptr) { -PMD_DRV_LOG (ERR ,"failed to allocate memory" ); - goto out; - } -memcpy (&l2_tn_filter_ptr->filter_info, - &l2_tn_filter, -sizeof (struct ixgbe_l2_tunnel_conf )); - flow->rule = l2_tn_filter_ptr; - flow->filter_type =RTE_ETH_FILTER_L2_TUNNEL ; -return flow; + l2_tn_filter_ptr =rte_zmalloc ("ixgbe_l2_tn_filter" , +sizeof (struct ixgbe_eth_l2_tunnel_conf_ele ),0 ); +if (!l2_tn_filter_ptr) { +PMD_DRV_LOG (ERR ,"failed to allocate memory" ); + ret = -ENOMEM ; + goto out; } +memcpy (&l2_tn_filter_ptr->filter_info, + &l2_tn_filter, +sizeof (struct ixgbe_l2_tunnel_conf )); + ret =ixgbe_dev_l2_tunnel_filter_add (adapter, &l2_tn_filter, + FALSE); +if (ret) { +rte_free (l2_tn_filter_ptr); + goto out; + } + flow->rule = l2_tn_filter_ptr; + flow->filter_type =RTE_ETH_FILTER_L2_TUNNEL ; +return flow; }memset (&rss_conf,0 ,sizeof (struct ixgbe_rte_flow_rss_conf )); ret =ixgbe_parse_rss_filter (dev, attr, actions, &rss_conf, error);if (!ret) { + rss_filter_ptr =rte_zmalloc ("ixgbe_rss_filter" , +sizeof (struct ixgbe_rss_conf_ele ),0 ); +if (!rss_filter_ptr) { +PMD_DRV_LOG (ERR ,"failed to allocate memory" ); + ret = -ENOMEM ; + goto out; + } ret =ixgbe_config_rss_filter (adapter, &rss_conf, TRUE); -if (!ret) { - rss_filter_ptr =rte_zmalloc ("ixgbe_rss_filter" , -sizeof (struct ixgbe_rss_conf_ele ),0 ); -if (!rss_filter_ptr) { -PMD_DRV_LOG (ERR ,"failed to allocate memory" ); - goto out; - } -ixgbe_rss_conf_init (&rss_filter_ptr->filter_info, - &rss_conf.conf); - flow->rule = rss_filter_ptr; - flow->filter_type =RTE_ETH_FILTER_HASH ; -return flow; +if (ret) { +rte_free (rss_filter_ptr); + goto out; } +ixgbe_rss_conf_init (&rss_filter_ptr->filter_info, + &rss_conf.conf); + flow->rule = rss_filter_ptr; + flow->filter_type =RTE_ETH_FILTER_HASH ; +return flow; } out:Signature · 1 line
2.55.0
Quoted 17 lines · Bruce Richardson
Quoted 16 lines · Zhang Tengfei
ixgbe_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 72c135a89f80 ("net/ixgbe: create consistent filter")
Quoted 7 lines
Quoted 2 lines
Signed-off-by Zhang Tengfei <[email protected]>---
Acked-by Bruce Richardson <[email protected]>This patch needs rebase though. The memcpy calls in the driver have been replaced by struct assignments. Will try updating on apply.
Patch applied to dpdk-next-net-intel.
Thanks,
/Bruce