Hi Maxime From: Maxime Leroy: > Hi Dekel, > > On Thu, Aug 6, 2020 at 12:40 PM Dekel Peled <dek...@mellanox.com> > 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 and VLAN items struct, to clearly > > define the required characteristic of a packet, and enable precise > > match criteria. > > > > [1] > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail > > s.dpdk.org%2Farchives%2Fdev%2F2020- > May%2F166257.html&data=02%7C01% > > > 7Cmatan%40nvidia.com%7C380bfd614a574a3550e108d853d9f535%7C43083d > 157273 > > > 40c1b7db39efd9ccc17a%7C0%7C0%7C637351542877842049&sdata=Sy0q > isaTIw > > ypjkF0dU2FoOwvlHBY%2FQ5SSNRaVk4jIIQ%3D&reserved=0 > > First, I still don't understand the initial change [1] done on RTE_FLOW API. > Before this change, it was possible to match any packets with or without vlan > encapsulations. > At least, it's not anymore possible the case with the mlx5 pmd since this > change.
We are going with the all explicit approach. User should say exactly what he want\doesn't want\doesn't-care of vlan attributes. Previously, the user had no way to say "I want IPv4 but not with vlan". > > 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 you cannot say I don't want to match ethernet type=0x8100\0x88A8. > Why is it not anymore supported to create a rule matching any ip packets > (with/without vlan header) ? > How is RFC handling this issue ? In the current proposal you can match on all - including the don't-care option you mentioned above. The new bit in item_eth allow you to say all options (yes - v 1 m 1,no v 0 m 1,don't-care v0\1 m 0) for the first vlan Any more vlan item after can say the same on the next vlans by the new bit it item_vlan more_vlans_exist. The default case will be don't care. Matan > > > > > Signed-off-by: Dekel Peled <dek...@mellanox.com> > > --- > > lib/librte_ethdev/rte_flow.h | 16 +++++++++++++--- > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > diff --git a/lib/librte_ethdev/rte_flow.h > > b/lib/librte_ethdev/rte_flow.h index cf0eccb..0e0b8d4 100644 > > --- a/lib/librte_ethdev/rte_flow.h > > +++ b/lib/librte_ethdev/rte_flow.h > > @@ -723,14 +723,18 @@ struct rte_flow_item_raw { > > * If the @p type field contains a TPID value, then only tagged packets > > with > the > > * specified TPID will match the pattern. > > * Otherwise, only untagged packets will match the pattern. > > - * 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 field @p vlan_exist can be used to match specific packet > > + types, instead > > + * of using the @p type field. > > + * This can be used to match any type of tagged packets. > > + * If the @p type and @p vlan_exist fields are not specified, then > > + both tagged > > + * and untagged packets will match the pattern. > > */ > > 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 vlan_exist:1; /**< At least one VLAN exist in header. */ > > + uint32_t reserved:31; /**< Reserved, must be zero. */ > > }; > > For vlan_exists=1, it can already be achieved with the following follow: > testpmd> flow create 0 ingress pattern eth type spec 0x8100 type > mask 0xFFFF / vlan inner_type is 0x800 mask is 0xFFFF / end actions mark id 2 > / queue index 0 / end > > For vlan_exists=0, first you can match ipv4 packets without vlan header like > that: > testpmd> flow create 0 ingress pattern eth type spec 0x800 type mask > 0xffff / ipv4 / end actions mark id 2 / queue index > 0 / end > > For matching ethernet packet without any vlan header, it's not possible > today with RTE_FLOW API. > But instead of adding a new field each structure for next fields, I think we > should introduce an attribute 'NOT' in the rte_flow API. > > For example, we could add this flow in test pmd like that: > testpmd> flow create 0 ingress pattern eth / not vlan / end > actions mark id 2 / queue index 0 / end > > > > > /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */ @@ -752,10 +756,16 > @@ > > struct rte_flow_item_eth { > > * the preceding pattern item. > > * If a @p VLAN item is present in the pattern, then only tagged packets > will > > * match the pattern. > > + * The field @p more_vlans_exist can be used to match specific packet > > + types, > > + * instead of using the @p inner_type field. > > + * This can be used to match any type of tagged packets. > > */ > > struct rte_flow_item_vlan { > > rte_be16_t tci; /**< Tag control information. */ > > rte_be16_t inner_type; /**< Inner EtherType or TPID. */ > > + uint32_t more_vlans_exist:1; > > + /**< At least one more VLAN exist in header, following this VLAN. */ > > + uint32_t reserved:31; /**< Reserved, must be zero. */ > > }; > > This can already be achieved with the following RTE_FLOW flows: > > flow create 0 ingress pattern eth / vlan inner_type spec is 0x0 > > mask is 0x0 / ipv4 / end actions mark id 2 / queue index 0 / end > > By setting inner_type mask to 0, it means that we don't care about > inner_type, so inner_type can be any value. Thus it can be 0x8100 for vlan or > 0x800 for ipv4. At the end, this rule matches any ipv4 packets with at least > one vlan header. > > Having more_vlan_exist in rte_flow_item_vlan is not useful. > > Regards, > > Maxime Leroy > > > > > /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */ > > -- > > 1.8.3.1 > >