[dpdk-dev] [RFC v2] ethdev: add VLAN attributes to ETH and VLAN items

[Maxime Leroy]

Hi Dekel,

On Thu, Aug 6, 2020 at 12:40 PM Dekel Peled <dekelp at 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://mails.dpdk.org/archives/dev/2020-May/166257.html

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.

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.

Why is it not anymore supported to create a rule matching any ip
packets (with/without vlan header) ?
How is RFC handling this issue ?

>
> Signed-off-by: Dekel Peled <dekelp at 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
>