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<[email protected]>
---
<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
