> -----Original Message----- > From: Ivan Malov <ivan.ma...@arknetworks.am> > Sent: Tuesday, April 23, 2024 13:44 > To: Jie Hai <haij...@huawei.com> > Cc: Yuying Zhang <yuying.zh...@intel.com>; Aman Singh > <aman.deep.si...@intel.com>; NBU-Contact-Thomas Monjalon (EXTERNAL) > <tho...@monjalon.net>; ferruh.yi...@amd.com >> Ferruh Yigit > <ferruh.yi...@amd.com>; andrew.rybche...@oktetlabs.ru; > dek...@nvidia.com; psathe...@marvell.com; NBU-Contact-Adrien Mazarguil > (EXTERNAL) <adrien.mazarg...@6wind.com>; fengchengwen > <fengcheng...@huawei.com>; Huisong Li <lihuis...@huawei.com>; > Dengdui Huang <huangdeng...@huawei.com>; dev@dpdk.org; Ori Kam > <or...@nvidia.com> > Subject: Re: question about eth and vlan item in flow pattern > > Hi, > > Please see below. > > On Tue, 23 Apr 2024, Jie Hai wrote: > > > Hi, Ivan Malov, > > Sorry for the late reply. > > On 2024/4/10 21:19, Ivan Malov wrote: > >> Hi Jie, > >> > >> Consider the following examples: > >> 1) flow create 0 ingress pattern eth / ipv4 proto is 17 / udp / end \ > >> actions queue index 1 / end > >> 2) flow create 0 ingress pattern eth / ipv4 / udp / end \ > >> actions queue index 1 / end > >> > >> Generally speaking, these two rules might be equivalent, with "proto is 17" > >> in > >> the first one probably being redundant (the very presence of "udp" > >> item might stand for the same match criterion -- the protocol ID > >> being 17). > > I can understand that. > Very well. > > >> > >> And I'd be tempted to treat the "has_vlan" case similarly. Consider: > >> 3) flow create 0 ingress pattern eth has_vlan is 1 / vlan / end \ > >> actions queue index 1 / end > >> 4) flow create 0 ingress pattern eth / vlan / end \ > >> actions queue index 1 / end > >> > >> (rule (3) is similar to "rule-0" from your example) > >> > >> Rules (3) and (4) seem equivalent to me -- the "has_vlan is 1" bit > >> might be redundant in flow (3) -- the presence of item "vlan" sort of > >> means the same. > > > >> Rule (3) and (4) should probably match both single-tagged and > >> double-tagged packets because, in both cases, the pattern does not > >> clarify which protocol follows the outermost VLAN tag. If one needs > >> to exclude double-tag match, they should clarify the rule in one of the > possible ways, in example: > >> > >> - pattern eth / vlan has_more_vlan is 0 / end > >> - pattern eth / vlan tpid is 0x0800 / end > >> - pattern eth / vlan / ipv4 / end > >> > >> (the "tpid" goes to the "hdr.eth_proto" of "struct rte_flow_item_vlan"). > >> > > As mentioned above, `has_vlan is 1` and `vlan` are the same, which > > means `vlan` stands for any-tagged vlan, Then, is the introduction of > > `has_vlan` and `has_more_vlan` unnecessary? > It's not clear why you believe it's unnecessary. Some vendors support boolean > match on "is outer VLAN present" and "is inner VLAN present" > in addition to exact match on TPID/TCI. It may be convenient to expose these > match criteria as sub-fields in ETH and VLAN items. > This should allow for faster parsing as one can drop item VLAN from pattern in > case there's no intention to match on TPID/TCI.
To expand the argument on convenience by providing an example, `has_vlan` allows to construct a following logic: 1) flow create 0 group X ingress pattern eth has_vlan is 1 / end actions jump group Y / end 2) flow create 0 group X ingress pattern eth has_vlan is 0 / end actions jump group Z / end 3) flow create 0 group Y ingress pattern eth / vlan vid is N / end actions /* actions for tagged traffic */ 4) flow create 0 group Z ingress pattern eth / end actions /* actions for non-tagged traffic */ Traffic can be split between different groups - one group for tagged and another for non-tagged packets. `has_vlan` gives more flexibility in the API. > > > > There are drivers seeing `vlan` an one vlan layer and match 'eth/vlan/end' > > with single-tagged packets, e.g.bnxt, hns3, etc. > > I think this meaning makes it easier to distinguish the functions of the > > two. > > > > The `has_more_vlan is 1` is defined and used by some drivers, like mlx5. > > While the `has_more_vlan is 0` is not defined. > What do you mean by saying "not defined"? Not defined where? > Having "has_more_vlan is 0" is perfectly legit, isn't it? > > > Here you give the meaning as `having no vlan after this vlan`. > > Then, What should the follow rule mean? single-tagged or double tagged? > > > > - pattern eth / vlan tpid is 0x8100 has_more_vlan is 0 / end > Item VLAN here is supposed to describe the header _immediately_ following > the ETH one, isn't it? If so, the rule means single-tagged. Doesn't it? I also think this rule means single-tagged, but on the other hand I think it's also ill-defined. tpid field in VLAN item defines the inner packet type, so this pattern should match the following packet: dmac=<any> smac=<any> ether_type=0x8100 // match single-tagged packet pcp=<any> vid=<any> ether_type=0x8100 // from tpid VLAN item Which is an incorrect packet. > Thank you. > > > > >> With regard to the question about VLAN ID match, "pattern eth / vlan > >> vid is 10", as well as "pattern eth has_vlan is 1 / vlan vid is 10", > >> should probably match only on the outermost tag ID, -- that is, not > >> on the inner one and not on both. > > I agree with it. > >> That is because the "vid is 10" criterion relates to the specific > >> header, as per the match item it sits in. If the user wants to match > >> on the inner VLAN ID (say, 11), they should clarify specify it as follows: > >> > >> - pattern eth / vlan vid is 10 / vlan vid is 11 / end > >> - pattern eth / vlan / vlan vid is 11 / end > >> > > > >> Hopefully, the rest of DPDK vendor community can correct me in case I > >> got that wrong. Should you have more questions, please feel free to ask > such. > >> > >> Thank you. > >> > >> On Wed, 10 Apr 2024, Jie Hai wrote: > >> > >>> Hi, all, > >>> > >>> I have some questions about the sub-options for ``VLAN`` and ``ETH`` > item. > >>> > >>> According to the documentation, ``has_vlan`` is sub-option of > >>> ``ETH`` item and it means that the pattern contains at least one vlan. > >>> The ``VLAN`` item is used to match tagged packets and have some > >>> sub-options such as ``vid``, ``tci``, etc. > >>> > >>> If we combine them, what should the effect be? > >>> For instance, > >>> > >>> rule-0: flow create 0 ingress pattern eth has_vlan is 1 / vlan / > >>> end actions queue index 2 / end > >>> rule-1: flow create 0 ingress pattern eth has_vlan is 1 / vlan vid > >>> is 10 / end actions queue index 2 / end > >>> > >>> For rule-0, should it match single-tagged packets only or > >>> multi-tagged only or both? > >>> That is to say, which one will take effect, `has_vlan is 1` or > >>> `vlan` or both? > >>> > >>> For rule-2, which packets should it match, with inner VLAN id 10, or > >>> outer VLAN id 10, or both 10? > >>> > >>> The hns3 driver supports only the exact matching of VLAN numer. > >>> And it is planned to adapt ``has_vlan`` and ``has_more_vlan`` to the > >>> meaning of one VLAN for hns3 driver. Therefore, if the preceding > >>> combinations are supported, we need to confirm the exact meanings. > >>> > >>> So, what are your views on the above question? > >>> > >>> > >>> > >>> > >> . > >