Re: [PATCH v2] doc: fix support table for ETH and VLAN flow items

[Ilya Maximets <i.maximets@ovn.org>]

On 10/26/22 17:34, Thomas Monjalon wrote:
> 13/10/2022 12:48, Ilya Maximets:
>> 'has_vlan' attribute is only supported by sfc, mlx5 and cnxk.
>> Other drivers doesn't support it.  Most of them (like i40e) just
>> ignore it silently.  Some drivers (like mlx4) never had a full
>> support of the eth item even before introduction of 'has_vlan'
>> (mlx4 allows to match on the destination MAC only).
>>
>> Same for the 'has_more_vlan' flag of the vlan item.
>>
>> 'has_vlan' is part of 'rte_flow_item_eth', so changing 'eth'
>> field to 'partial support' in documentation for all such drivers.
>> 'has_more_vlan' is part of 'rte_flow_item_vlan', so changing
>> 'vlan' to 'partial support' as well.
>>
>> This doesn't solve the issue, but at least marks the problematic
>> drivers.
>>
>> Some details are available in:
>>   https://bugs.dpdk.org/show_bug.cgi?id=958
>>
>> Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN items")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> 
> Applied, thanks.

Thanks!

> 
> How can we encourage fixing the problematic drivers?

That's a tough question and I don't have a good answer.

One thought that I have is to make all drivers report a mask for
each ITEM.  That mask should represent all the bits in the ITEM
theoretically supported by the driver.  If some driver doesn't
report a mask for an ITEM, it is treated as all-zero mask.

While installing the flow or on flow validation, the generic
layer will check the rte_flow against masks reported by the
driver.  If there are no extra bits in the flow, it is passed
to the driver for further validation and installation.
If there are extra bits, flow is rejected at the high level.

Further validation is still required because only the driver
is able to check dependencies between fields.

Once such a checking is merged, it will break offloading for all
the drivers.  Maintainers will need to create appropriate masks
to unblock offloading.  memset() on the whole ITEM or similar
operations must not be allowed for these masks in the driver code.
Thorough review of these patches is necessary.

This will not help with encouraging to implement the features
per se, but will force maintainers to re-check their drivers.
And should eliminate issues where certain fields are silently
ignored, because until the driver is updated with a new mask
generic validation will reject offloading of flows that contain
matches on new fields.

At least applications will not fear to use certain features
knowing that they will be correctly rejected on validation.

Best regards, Ilya Maximets.