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: [email protected]
> Cc: [email protected]
>
> Signed-off-by: Anatoly Burakov <[email protected]>
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 <[email protected]>
> ---
> 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
>
