[PATCH v5 02/12] net/ixgbe: fix memory leak in security flows

[Bruce Richardson]

On Thu, Feb 12, 2026 at 12:53:25PM +0000, Anatoly Burakov wrote:
> Currently, security flows are implemented as a special case and do not go
> through the normal flow create/destroy infrastructure. However, because of
> that, it is impossible to destroy such flows once created. Fix it by adding
> a flag to rte_flow indicating that it is a security flow, so that it can be
> destroyed later.
> 
> Additionally, security flows return pointer to allocated `rte_flow` struct
> unconditionally, even though the underlying call to ipsec code might have
> failed. Fix that by checking the return value from the filter function
> before returning.
> 
> Fixes: 9a0752f498d2 ("net/ixgbe: enable inline IPsec")
> Cc: radu.nicolau at intel.com
> Cc: stable at dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov at intel.com>

Not an expert in these security flows but the patch looks ok to me. One
minor nit inline below which you can take if you do a respin.

Signed-off-by: Bruce Richardson <bruce.richardson at intel.com>


> ---
>  drivers/net/intel/ixgbe/ixgbe_ethdev.h |  2 ++
>  drivers/net/intel/ixgbe/ixgbe_flow.c   | 13 ++++++++++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/intel/ixgbe/ixgbe_ethdev.h b/drivers/net/intel/ixgbe/ixgbe_ethdev.h
> index 5393c81363..5dbd659941 100644
> --- a/drivers/net/intel/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/intel/ixgbe/ixgbe_ethdev.h
> @@ -359,6 +359,8 @@ struct ixgbe_l2_tn_info {
>  
>  struct rte_flow {
>  	enum rte_filter_type filter_type;
> +	/* security flows are not rte_filter_type */
> +	bool is_security;
>  	void *rule;
>  };
>  
> diff --git a/drivers/net/intel/ixgbe/ixgbe_flow.c b/drivers/net/intel/ixgbe/ixgbe_flow.c
> index 27d2ba1132..90072e757e 100644
> --- a/drivers/net/intel/ixgbe/ixgbe_flow.c
> +++ b/drivers/net/intel/ixgbe/ixgbe_flow.c
> @@ -3062,8 +3062,12 @@ ixgbe_flow_create(struct rte_eth_dev *dev,
>  
>  #ifdef RTE_LIB_SECURITY
>  	/* ESP flow not really a flow*/
> -	if (ntuple_filter.proto == IPPROTO_ESP)
> +	if (ntuple_filter.proto == IPPROTO_ESP) {
> +		if (ret != 0)
> +			goto out;
> +		flow->is_security = true;
>  		return flow;
> +	}
>  #endif
>  
>  	if (!ret) {
> @@ -3350,6 +3354,12 @@ ixgbe_flow_destroy(struct rte_eth_dev *dev,
>  		IXGBE_DEV_PRIVATE_TO_FDIR_INFO(dev->data->dev_private);
>  	struct ixgbe_rss_conf_ele *rss_filter_ptr;
>  
> +	/* Special case for SECURITY flows */
> +	if (flow->is_security) {
> +		ret = 0;

Rather than assigning ret explicitly here, I think it might be better just
to set it = 0  at definition, and leaving this as a simple goto free. [It
would also head off any future compiler warnings about ret being
uninitialized :-)]

> +		goto free;
> +	}
> +
>  	switch (filter_type) {
>  	case RTE_ETH_FILTER_NTUPLE:
>  		ntuple_filter_ptr = (struct ixgbe_ntuple_filter_ele *)
> @@ -3442,6 +3452,7 @@ ixgbe_flow_destroy(struct rte_eth_dev *dev,
>  		return ret;
>  	}
>  
> +free:
>  	TAILQ_FOREACH(ixgbe_flow_mem_ptr, &ixgbe_flow_list, entries) {
>  		if (ixgbe_flow_mem_ptr->flow == pmd_flow) {
>  			TAILQ_REMOVE(&ixgbe_flow_list,
> -- 
> 2.47.3
>