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

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

On 8/24/21 6:04 PM, Eli Britstein wrote:
> 
> 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?

This one is hard to use.  And I guess, it's hard to determine
what exactly is not supported as some things can be only
supported in certain combinations or otherwise not supported in
certain combinations.  So, rte_flow_validate() doesn't seem to
be suitable for checking particular features and alternating
the flow construction based on that.  If we had clear yes/no
flags for all features that could be easily probed/verified or
otherwise retrived from the driver, that would be better.

But it seems to be a problem for current rte_flow implementations
in actual drivers.

> 
> 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.