> -----Original Message----- > From: fengchengwen <fengcheng...@huawei.com> > Sent: Monday, 17 October 2022 14:55 > > Thanks Ilya and Eli > > On 2022/10/16 13:26, Eli Britstein wrote: > > > > > >> -----Original Message----- > >> From: Ilya Maximets <i.maxim...@ovn.org> > >> Sent: Friday, October 14, 2022 3:37 PM > >> To: fengchengwen <fengcheng...@huawei.com>; dev@dpdk.org; Ori > Kam > >> <or...@nvidia.com>; NBU-Contact-Thomas Monjalon (EXTERNAL) > >> <tho...@monjalon.net>; Eli Britstein <el...@nvidia.com> > >> Cc: i.maxim...@ovn.org; Ajit Khaparde <ajit.khapa...@broadcom.com>; > >> Somnath Kotur <somnath.ko...@broadcom.com>; Rahul Lakkireddy > >> <rahul.lakkire...@chelsio.com>; Hemant Agrawal > >> <hemant.agra...@nxp.com>; Sachin Saxena > <sachin.sax...@oss.nxp.com>; > >> Simei Su <simei...@intel.com>; Wenjun Wu <wenjun1...@intel.com>; > John > >> Daley <johnd...@cisco.com>; Hyong Youb Kim <hyon...@cisco.com>; > Ziyang > >> Xuan <xuanziya...@huawei.com>; Xiaoyun Wang > >> <cloud.wangxiao...@huawei.com>; Guoyang Zhou > >> <zhouguoy...@huawei.com>; Dongdong Liu > <liudongdo...@huawei.com>; > >> Yisen Zhuang <yisen.zhu...@huawei.com>; Yuying Zhang > >> <yuying.zh...@intel.com>; Beilei Xing <beilei.x...@intel.com>; Jingjing > Wu > >> <jingjing...@intel.com>; Qiming Yang <qiming.y...@intel.com>; Qi > Zhang > >> <qi.z.zh...@intel.com>; Junfeng Guo <junfeng....@intel.com>; Rosen > Xu > >> <rosen...@intel.com>; Matan Azrad <ma...@nvidia.com>; Slava > Ovsiienko > >> <viachesl...@nvidia.com>; Liron Himi <lir...@marvell.com>; Jiawen Wu > >> <jiawe...@trustnetic.com>; Jian Wang <jianw...@trustnetic.com>; > Dekel > >> Peled <dek...@nvidia.com>; sta...@dpdk.org > >> 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. > >>>
Right, just to be extra clear the vid == 100 is for the outer most. > >>> 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. > >>> Right, > >>> 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. > >>> Right, but you don't need to use the has_vlan since vlan item is given in the pattern. > >>> 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? > Yes, everything looks good. > 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? > A must be smaller than B and the last vlan has_more = 1 The matching is based on HW limitations, it depends on how the HW works. > >> > >> 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: sta...@dpdk.org > >>>> > >>>> Signed-off-by: Ilya Maximets <i.maxim...@ovn.org> > >>>> --- > >>>> > >>>> 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(-) > >
