Re: [PATCH v3 15/19] net/i40e: refactor FDIR engine infrastructure

["Medvedkin, Vladimir" <vladimir.medvedkin@intel.com>]

On 9/16/2026 1:18 PM, Anatoly Burakov wrote:
> Currently, there are multiple problems with how i40e flow directory feature
> is implemented, both in terms of how it works with rte_flow, and how it
> integrates with the PMD-specific packet template API.
>
> For one, these two subsystems, while using shared infrastructure, do not
> really interact or cooperate, and are built on top of special cases in FDIR
> path. More specifically, the packet template code does not store its
> packet in the hash map, and has a different hashing scheme, yet it still
> registers itself in FDIR flow list, hash table, and hash map. This list
> is then used by `dev_start` to restore FDIR filters that user has
> inserted into the list. These filters, as written, cannot be reprogrammed
> that way because the information a filter restore function would need is
> lost on insert (the packet pointer is not added to the hash map).
>
> Another issue is that while rte_flow FDIR code does lazy FDIR init on first
> added flow, the packet template API does not, even though it too relies on
> the same hardware feature, nor does it ever do teardown on last FDIR flow.
>
> Yet another issue is how the "filter restore" code itself is implemented,
> namely that currently it simply does not work. When doing filter restore,
> the driver will walk every FDIR filter stored in the TAILQ, and attempt to
> program it. However, inside the program function, there is a deduplication
> check (to see if flow being installed is already present in the flow hash
> table), which fails because the flows we are programming come from the same
> list that is being checked for deduplication, which makes the entire filter
> restore a no-op.
>
> The FDIR filter programming code itself also has a number of readability
> problems as well as being otherwise hard to use - SW bookkeeping,
> validation, and flow programming is interspersed within the code, and it
> is difficult to reason about what happens when the code is called from
> this or that context.
>
> So, this refactor does the following:
>
> - Reorganize FDIR internals to track packet templates and rte_flow FDIR
>    flows separately
> - Refactor FDIR init/teardown to always happen on first/last rule, so that
>    whichever API happens to call FDIR first, the state is consistent
nit: it seems you've moved it to the next patch
> - Rework the FDIR code to disentangle FDIR flow rule programming, Flex PIT
>    checks, SW bookkeeping, etc. from each other
> - Remove both the rte_flow FDIR TAILQ and the hash map (filter array)
>    structure, because they are redundant (information about the flow is
>    already stored in the rte_flow flow list, and hash_map structure only
>    stored pointers to data we also have in that same list)
> - Fix FDIR filter restore to replay all configuration correctly, as well as
>    re-init the FDIR queue enablement tracking
> - Rework the internal FDIR global state data structure to make a little
>    more sense by grouping things that belong together into structures
>
> Additionally, there was a delay mechanism at flow director rule program
> time, as when programming a rule we might not know if it's actually
> possible to install the rule, because space for the rules may come either
> from our own pool, or it may come from a pool that is shared with other
> VSI's. However, it only makes sense to wait on rule create (i.e. when it is
> programmed into the hardware for the first time), but not when we are
> replaying or removing these rules. So, adjust the waiting mechanism to only
> wait on FDIR rule creation.
>
> Signed-off-by: Anatoly Burakov<anatoly.burakov@intel.com>
> ---
<snip>
> @@ -3860,16 +3851,15 @@ i40e_flow_destroy(struct rte_eth_dev *dev,
>   		ret = i40e_flow_destroy_tunnel_filter(pf,
>   			      (struct i40e_tunnel_filter *)flow->rule);
>   		break;
> -	case RTE_ETH_FILTER_FDIR:
> -		ret = i40e_flow_add_del_fdir_filter(dev,
> -				&((struct i40e_fdir_filter *)flow->rule)->fdir,
> -				0);
> +	case RTE_ETH_FILTER_FDIR: {
> +		struct i40e_fdir_filter *node = flow->rule;
>   
> -		/* If the last flow is destroyed, disable fdir. */
> -		if (!ret && TAILQ_EMPTY(&pf->fdir.fdir_list)) {
> -			i40e_fdir_rx_proc_enable(dev, 0);
> -		}
> +		ret = i40e_fdir_filter_program(dev, node, 0, false);
previous implementation set wait_status depending on 
fdir_info->fdir_invalprio. Do we really need to wait here?
> +		if (ret)
> +			break;
> +		ret = i40e_fdir_filter_unregister(dev, node);
>   		break;
> +	}
>   	case RTE_ETH_FILTER_HASH:
>   		ret = i40e_hash_filter_destroy(pf, flow->rule);
<snip>

-- 
Regards,
Vladimir