Thanks Ilya and Eli On 2022/10/16 13:26, Eli Britstein wrote: > > >> -----Original Message----- >> From: Ilya Maximets <[email protected]> >> Sent: Friday, October 14, 2022 3:37 PM >> To: fengchengwen <[email protected]>; [email protected]; Ori Kam >> <[email protected]>; NBU-Contact-Thomas Monjalon (EXTERNAL) >> <[email protected]>; Eli Britstein <[email protected]> >> Cc: [email protected]; Ajit Khaparde <[email protected]>; >> Somnath Kotur <[email protected]>; Rahul Lakkireddy >> <[email protected]>; Hemant Agrawal >> <[email protected]>; Sachin Saxena <[email protected]>; >> Simei Su <[email protected]>; Wenjun Wu <[email protected]>; John >> Daley <[email protected]>; Hyong Youb Kim <[email protected]>; Ziyang >> Xuan <[email protected]>; Xiaoyun Wang >> <[email protected]>; Guoyang Zhou >> <[email protected]>; Dongdong Liu <[email protected]>; >> Yisen Zhuang <[email protected]>; Yuying Zhang >> <[email protected]>; Beilei Xing <[email protected]>; Jingjing Wu >> <[email protected]>; Qiming Yang <[email protected]>; Qi Zhang >> <[email protected]>; Junfeng Guo <[email protected]>; Rosen Xu >> <[email protected]>; Matan Azrad <[email protected]>; Slava Ovsiienko >> <[email protected]>; Liron Himi <[email protected]>; Jiawen Wu >> <[email protected]>; Jian Wang <[email protected]>; Dekel >> Peled <[email protected]>; [email protected] >> Subject: Re: [PATCH v2] doc: fix support table for ETH and VLAN flow items >> >> External email: Use caution opening links or attachments >> >> >> On 10/14/22 11:41, fengchengwen wrote: >>> Hi Ilya, >>> >>> I have some questions about has_vlan/has_more_vlan fields: >> >> I think, these questions are more for rte_flow maintainers, but I'll try >> answer. >> Maintainers can correct me if I'm wrong. >> >>> >>> a\ DPDK framework support cvlan-tag(0x8100) and svlan-tag(0x88A8), >>> and also deprecated qinq-tag(eg. 0x9100) >> >> Didn't check, but sounds about right. > It is not related to "DPDK framework". It is up to the HW to determine. >> >>> b\ If has_vlan is used, does it mean that all the VLAN >> tags(0x8100/88A8/9100) must be matched ? >>> I think this is different from using type, which can only match one of >> them. >> >> I think so. has_vlan = 1 means that packet has some vlan regardless of the >> actual type of the vlan header. > Again, it is up to the HW. >> >>> c\ And has_more_vlan has the same function as has_vlan ? >> >> Yes, from my understanding, 'has_more_vlan' is the same as 'has_vlan', but >> for the 'inner_type'. >> >>> d\ What the problems are solved by the new two fields? >> >> One of the problems we solved in OVS by using these fields is that we need a >> way to match on the fact that there is a vlan, but we do not care what this >> vlan >> tag is and at the same time we want to match on the inner type for such >> packets. >> >> Trying to workaround that situation will likely require breaking the 1:1 >> mapping between OVS flows and rte_flow rules, so it is not really possible. >> In >> the end, we had to use 'has_vlan' field to fix an incorrect packet matching >> in >> OVS. Alternative, I guess, would be to just not offload vlan flows, but >> doesn't >> make a lot of sense. >> >> Eli should know better what was the actual problem, I think. > OVS does not support offload of qinq, so "has_more_vlan" is still not in use. > For native (untagged) flows, there is a need to tell the HW "has_vlan is 0", > otherwise the HW flow will hit both tagged/untagged traffic, which is wrong. > For tagged flows, OVS will always match on the VLAN properties, so "has_vlan > is 1" can be deducted/implicit. > Before that field existed, it could be implicit to deduct "lack" of VLAN > header (e.g. "eth / ipv4" for example) as "has_vlan is 0". However, other > applications that would like both tagged/untagged traffic to hit needed to > have 2 separated flows (with a probably slightly lower performance).
Got it, Thanks. > Also, DPDK rte-flow is to have things explicit. >> >>> >>> >>> If the above understanding is correct, and the hardware support identify >> there TPID(cvlan-0x8100, svlan-0x88A8, dqing-0x9100) as VLAN, then: >>> Rule: eth has_vlan is 1 / vlan vid is 100 / ipv4 / end actions xxx >>> Result: all ipv4 packets with at least one VLAN(the TPID can be one of >>> the >> above) and the vid is 100 can be matched. >>> >>> Rule: eth type is 0x8100 / vlan vid is 100 / ipv4 / end actions xxx >>> Result: all ipv4 packets with at lease one VLAN(which TPID must be >> 0x8100) and the vid is 100 can be matched. >>> >>> Rule: eth has_vlan is 1 / vlan vid is 100 has_more_vlan is 1 / vlan >>> vid is 200 >> / ipv4 / end action xxx >>> Result: all ipv4 packets with at least two VLAN(the TPID can be one of >>> the >> above) and outer vid is 100 and the next vid is 200 can be matched. >>> >>> Rule: eth type is 0x88A8 / vlan vid is 100 inner_type is 0x8100 / vlan >>> vid is >> 200 / ipv4 / end action xxx >>> Result: all ipv4 packets with at least two VLAN(the first TPID is >>> 0x88A8 and >> second TPID is 0x8100) and outer vid is 100 and the next vid is 200 can be >> matched. >>> Is the above result correct ? >> >> Seems correct, but I don't have much experience with rte_flow notations. >> >> Ori, could you comment on this? Assuming that A is the number of VLANs by flow creation, and B is the number of VLANs of real flow What I'm concerned about is: Whether the matching is successful only when A is equal to B? In addition, the maximum number of VLANs that can be parsed by hardware is limited, For example, if the hardware supports a maximum of two VLAN tags, a rule with the number of two VLAN tags is created for the RTE_Flow. However, the actual flow has more than two VLAN tags. Can this situation be matched? Hi Ori, Could you check on this? >> >> Best regards, Ilya Maximets. >> >>> >>> Thanks >>> >>> On 2022/10/13 18:48, Ilya Maximets wrote: >>>> '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: [email protected] >>>> >>>> Signed-off-by: Ilya Maximets <[email protected]> >>>> --- >>>> >>>> Version 2: >>>> - Rebased on a current main branch. >>>> - Added more clarifications to the commit message. >>>> >>>> I added the stable in CC, but the patch should be extended while >>>> backporting. For 21.11 the cnxk driver should be also updated, for >>>> 20.11, sfc driver should also be included. >>>> >>>> doc/guides/nics/features/bnxt.ini | 4 ++-- >>>> doc/guides/nics/features/cxgbe.ini | 4 ++-- >>>> doc/guides/nics/features/dpaa2.ini | 4 ++-- >>>> doc/guides/nics/features/e1000.ini | 2 +- >>>> doc/guides/nics/features/enic.ini | 4 ++-- >>>> doc/guides/nics/features/hinic.ini | 2 +- >>>> doc/guides/nics/features/hns3.ini | 4 ++-- >>>> doc/guides/nics/features/i40e.ini | 4 ++-- >>>> doc/guides/nics/features/iavf.ini | 4 ++-- >>>> doc/guides/nics/features/ice.ini | 4 ++-- >>>> doc/guides/nics/features/igc.ini | 2 +- >>>> doc/guides/nics/features/ipn3ke.ini | 4 ++-- >>>> doc/guides/nics/features/ixgbe.ini | 4 ++-- >>>> doc/guides/nics/features/mlx4.ini | 4 ++-- >>>> doc/guides/nics/features/mvpp2.ini | 4 ++-- >>>> doc/guides/nics/features/tap.ini | 4 ++-- >>>> doc/guides/nics/features/txgbe.ini | 4 ++-- >>>> 17 files changed, 31 insertions(+), 31 deletions(-) >
