Re: [dpdk-dev] [PATCH V3 1/2] netdev-offload-dpdk: Use has_vlan match attribute

[Eli Britstein <elibr@nvidia.com>]

On 8/24/2021 6:47 PM, Ilya Maximets wrote:
> External email: Use caution opening links or attachments
>
>
> On 8/24/21 5:25 PM, Eli Britstein wrote:
>> On 8/24/2021 6:08 PM, Finn, Emma wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> -----Original Message-----
>>> From: Eli Britstein <elibr@nvidia.com>
>>> Sent: Monday 16 August 2021 14:55
>>> To: dev@openvswitch.org; Ilya Maximets <i.maximets@ovn.org>
>>> Cc: Finn, Emma <emma.finn@intel.com>; Stokes, Ian <ian.stokes@intel.com>; Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>; Gaetan Rivet <gaetanr@nvidia.com>; Majd Dibbiny <majd@nvidia.com>; Eli Britstein <elibr@nvidia.com>; Salem Sol <salems@nvidia.com>
>>> Subject: [PATCH V3 1/2] netdev-offload-dpdk: Use has_vlan match attribute
>>>
>>> DPDK 20.11 introduced an ability to specify existance/non-existance of VLAN tag by [1].
>>> Use this attribute.
>>>
>>> [1]: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN items")
>>>
>>> Signed-off-by: Eli Britstein <elibr@nvidia.com>
>>> Reviewed-by: Salem Sol <salems@nvidia.com>
>>>
>>> Hi Eli,
>>>
>>> I tested this but currently we don't have support in the i40e pmd for the has_vlan match attribute and with these patches it is breaking offload for VLAN packets on Intel devices.
>> Hi Emma,
>>
>> Thanks for testing.
>>
>> Is adding such support in your plans?
>>
>> How do you suggest to proceed? It is needed in order to fix OVS bug.
>>
>> Thanks,
>>
>> Eli
> The "Table 1.2 rte_flow items availability in networking drivers"
> here: https://doc.dpdk.org/guides/nics/overview.html
> says that both ixgbe and i40e has a full support for 'vlan' and
> 'eth' items.  Is it a bug?  Should it be 'partial' instead?
>
> In general, this sounds like a big limitation of rte_flow API.
> I mean the fact that there is no way to get what is implemented by
> a particular driver and what is not implemented in runtime.
> Someone should, probably, work on adding this kind of API to DPDK.
> Otherwise, we will stuck with inability to use certain actions/matches
> unless all the drivers supports them (which is also hard to check
> taking documentation issues into account).  If I missed it and the
> API actually exists, we should definitely start using it.
>
> CC: dpdk-dev and rte_flow maintainers.
>
> Thoughts?

There is such an API - rte_flow_validate().

However, in OVS, as each flow is independent and can have different 
matches and actions, we just call rte_flow_create(). The PMD (at least 
mlx5) first internally validates it (as if rte_flow_validate() is 
called), and bail out with a failure in case validate fails.

Can you suggest an effective way to utilize it in OVS?

In theory, if the API exists in rte_flow, OVS should not care if all 
PMDs support it or not.

In practice, the "has_vlan" field was introduced only in 20.11, and 
apparently Intel has not adapted i40e PMD, so it breaks their offloads. 
I suspected this so I've added Emma and Ian to review it.

I don't know i40e HW capabilities, but at least from PMD point of view, 
it can be silently ignored until a proper support is added.

>
> Best regards, Ilya Maximets.