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

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Feb 13, 2026 at 09:44:49AM +0100, Burakov, Anatoly wrote:
> On 2/12/2026 6:10 PM, Bruce Richardson wrote:
> > 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@intel.com
> > > Cc: stable@dpdk.org
> > > 
> > > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> <snip>
> 
> > > @@ -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 :-)]
> > 
> 
> I actually remember a lot of commits *removing* that sort of thing, with the
> idea being that we *want* to have these warnings to make sure every path is
> covered. Additionally, I personally prefer it this way for clarity (i.e.
> explicitly indicating success).
> 
> I can still fix it if you have strong feelings on it, but I'd rather leave
> it as is.
> 

True. It was just fresh in my mind having had to fix an issue with the hash
gfni code where the compiler had failed to realise that the variable had
to have been initialized and so was giving a false positive warning.
If you prefer it this way, I'm ok to keep as-is.

/Bruce