Thanks, PSB. > -----Original Message----- > From: Maxime Leroy <maxime.le...@6wind.com> > Sent: Monday, September 7, 2020 7:13 PM > To: Dekel Peled <dek...@mellanox.com> > Cc: Eli Britstein <el...@mellanox.com>; ferruh.yi...@intel.com; > arybche...@solarflare.com; Ori Kam <or...@mellanox.com>; NBU-Contact- > Thomas Monjalon <tho...@monjalon.net>; Asaf Penso > <as...@mellanox.com>; dev@dpdk.org > Subject: Re: [dpdk-dev] [RFC] ethdev: add VLAN attributes to ETH item > > Hi Dekel, > > First, I don't understand the initial change [1] done on RTE_FLOW API.
As [1] commit log specifies, it is meant to clarify the required pattern to use, and reduce ambiguity. > Before this change, it was possible to match any packets with or without vlan > encapsulations. > At least, it's not anymore the case with the mlx5 pmd. > > For example, if I want to match any ssh packets whatever if it's encapsulated > with no vlan or N vlan headers: > testpmd> flow create 0 ingress pattern eth type spec 0 type mask 0 / > ipv4 / udp dst is 22 / end actions mark id 2 / queue index 0 / end > > By setting the ethernet type mask to 0x0, it means that ethernet type should > be ignored. It means if ethernet type is 0x800 (i.e. ipv4) or > 0x8100 (i.e. vlan) or 0x88A8 (qinq), the packet should be matched. > > But if you wanted to match only ethernet packets (and not vlan/qinq one), > you can create the following flows: > testpmd> flow create 0 ingress pattern eth type spec 0x800 type mask > 0xffff / ipv4 / udp dst is 22 / end actions mark id 2 / queue index > 0 / end > > With your new RFC, first I don't understand the needs of the num_of_vlans > field. Actually v2 of this RFC was sent already, please refer to https://mails.dpdk.org/archives/dev/2020-August/177536.html. > > You can create the following follow if you want to match any qinq / > ipv4 packets (i.e. 2 vlan level) for example: > > flow create 0 ingress pattern eth type spec 0x88A8 type mask 0xffff > > / vlan inner_type spec is 0x8100 mask is 0xFFFF / vlan / end actions > > mark id 2 / queue index 0 / end > > Could you explain to me the utility of this new field ? > > The cvlan_exist, and svlan_exist seems useless for me. For me, you can > already do the same thing with type field. Because, by setting the type mask > to 0, you can already give the notion of any ethertype. > > Let's take some example: > > 1. with svlan_exists=0, cvlan_exists=0, it can already be configured like > that: > > flow create 0 ingress pattern eth type spec 0x800 type mask 0xFFFF > > / ipv4 / end actions mark id 2 / queue index 0 / end > > 2. With svlan_exists=0, cvlan_exists=1: > > flow create 0 ingress pattern eth type spec 0x8100 type mask 0xFFFF > > / vlan inner_type is 0x800 mask is 0xFFFF / ipv4 / end actions mark > > id 2 / queue index 0 / end > > 3. With svlan_exists=1, cvlan_exists=0: it doesn't seem to be a real use case. > Anyway, you could have: > > flow create 0 ingress pattern eth type spec 0x88A8 type mask 0xffff > > / vlan inner_type spec is 0x800 mask is 0xFFFF / ipv4 / end actions > > mark id 2 / queue index 0 / end > > 4. With svlan_exists=1, cvlan_exists=1: > > flow create 0 ingress pattern eth type spec 0x88A8 type mask 0xffff / > vlan > inner_type spec is 0x8100 mask is 0xFFFF / vlan spec is > 0x800 mask 0xFFFF / ipv4 / end actions mark id 2 / queue index 0 / end > > Could you please explain to me what you try to achieve with this RFC ? > > I would like to know why ether_type value setted by the user is ignored > when I create the following rule: > testpmd> flow create 0 ingress pattern eth type spec 0 type mask 0 / > ipv4 / udp dst is 22 / end actions mark id 2 / queue index 0 / end with the > mlx5 pmd ? (i.e. why this change [1].) > > [1] > https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmails. > dpdk.org%2Farchives%2Fdev%2F2020- > May%2F166257.html&data=02%7C01%7Cdekelp%40nvidia.com%7C7cb0 > 777ea0e841bdc19808d85348f520%7C43083d15727340c1b7db39efd9ccc17a%7 > C0%7C0%7C637350920106123048&sdata=BWVQ0vSBMW2oDaGe0%2F6 > 6EsEY1z76jFVJLKKtrmTQHj0%3D&reserved=0 > > Best Regards, > > Maxime > > On Wed, Aug 5, 2020 at 9:04 AM Matan Azrad <ma...@mellanox.com> > wrote: > > > > But if the user want to force only one vlan and don't care about others? > > > > > > > -----Original Message----- > > > From: Dekel Peled <dek...@mellanox.com> > > > Sent: Wednesday, August 5, 2020 9:54 AM > > > To: Eli Britstein <el...@mellanox.com>; ferruh.yi...@intel.com; > > > arybche...@solarflare.com; Ori Kam <or...@mellanox.com>; Thomas > > > Monjalon <tho...@monjalon.net> > > > Cc: Asaf Penso <as...@mellanox.com>; Matan Azrad > > > <ma...@mellanox.com>; dev@dpdk.org > > > Subject: RE: [RFC] ethdev: add VLAN attributes to ETH item > > > > > > Thanks, PSB. > > > > > > > -----Original Message----- > > > > From: Eli Britstein <el...@mellanox.com> > > > > Sent: Tuesday, August 4, 2020 6:47 PM > > > > To: Dekel Peled <dek...@mellanox.com>; ferruh.yi...@intel.com; > > > > arybche...@solarflare.com; Ori Kam <or...@mellanox.com>; Thomas > > > > Monjalon <tho...@monjalon.net> > > > > Cc: Asaf Penso <as...@mellanox.com>; Matan Azrad > > > <ma...@mellanox.com>; > > > > dev@dpdk.org > > > > Subject: Re: [RFC] ethdev: add VLAN attributes to ETH item > > > > > > > > > > > > On 8/4/2020 6:36 PM, Dekel Peled wrote: > > > > > In existing code the match on tagged/untagged packets is not explicit. > > > > > Recent documentation update [1] describes the different patterns > > > > > and clarifies the intended use of different patterns. > > > > > > > > > > This patch proposes an update to ETH item struct, to clearly > > > > > define the required characteristic of a packet, and enable precise > match criteria. > > > > > > > > > > [1] > > > > > > https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2 > > > > > Fmails.dpdk.org%2Farchives%2Fdev%2F2020- > May%2F166257.html&da > > > > > > ta=02%7C01%7Cdekelp%40nvidia.com%7C7cb0777ea0e841bdc19808d85348f > > > > > > 520%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637350920106123 > > > > > > 048&sdata=BWVQ0vSBMW2oDaGe0%2F66EsEY1z76jFVJLKKtrmTQHj0% > 3D&a > > > > > mp;reserved=0 > > > > > > > > > > Signed-off-by: Dekel Peled <dek...@mellanox.com> > > > > > --- > > > > > lib/librte_ethdev/rte_flow.h | 9 +++++++++ > > > > > 1 file changed, 9 insertions(+) > > > > > > > > > > diff --git a/lib/librte_ethdev/rte_flow.h > > > > > b/lib/librte_ethdev/rte_flow.h index cf0eccb..345feb5 100644 > > > > > --- a/lib/librte_ethdev/rte_flow.h > > > > > +++ b/lib/librte_ethdev/rte_flow.h > > > > > @@ -726,11 +726,20 @@ struct rte_flow_item_raw { > > > > > * If the @p ETH item is the only item in the pattern, and the > > > > > @p type > > > field > > > > > * is not specified, then both tagged and untagged packets > > > > > will match > > > the > > > > > * pattern. > > > > > + * The fields @p cvlan_exist and @p svlan_exist can be used to > > > > > + match specific > > > > > + * packet types, instead of using the @p type field. This can > > > > > + be used to match > > > > > + * on packets that do/don't contain either cvlan, svlan, or both. > > > > > + * The field @p num_of_vlans can be used to match packets by > > > > > + the exact number > > > > > + * of VLANs in header. > > > > > */ > > > > > struct rte_flow_item_eth { > > > > > struct rte_ether_addr dst; /**< Destination MAC. */ > > > > > struct rte_ether_addr src; /**< Source MAC. */ > > > > > rte_be16_t type; /**< EtherType or TPID. */ > > > > > + uint32_t cvlan_exist:1; /**< C-tag VLAN exist in header. */ > > > > > + uint32_t svlan_exist:1; /**< S-tag VLAN exist in header. */ > > > > > + uint32_t reserved:14; /**< Reserved, must be zero. */ uint32_t > > > > > + num_of_vlans:16; /**< Number of VLANs in header. */ > > > > We can deduct from num_of_vlans the values of > > > > cvlan_exist/svlan_exist, so those are redundant fields. Keeping > > > > them introduce a conflicting match. For example num_of_vlans=0 and > cvlan_exist=1. > > > > > > Such conflict is simple to validate and reject. > > > Even if num_of_vlans is removed, we can still get conflict > > > svlan_exist=1, cvlan_exist=0. > > > The different fields are proposed to allow flexible match on > > > different VLAN attributes. > > > Every PMD can choose to support any or none of them. > > > > > > > > }; > > > > > > > > > > /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */
