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

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 9/19/2026 6:13 PM, Medvedkin, Vladimir wrote:
> 
> 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?

We do not, and wait is false here. We used to set wait_status here 
depending on fdir_info->fdir_invalprio, but in actuality we can avoid 
waiting here (see commit message).

-- 
Thanks,
Anatoly