Re: [dpdk-dev] [PATCH v2] net/mlx5: fix VLAN item match for DV mode

[Raslan Darawsheh <rasland@mellanox.com>]

Hi,

> -----Original Message-----
> From: Dekel Peled <dekelp@mellanox.com>
> Sent: Tuesday, February 11, 2020 1:05 PM
> To: Matan Azrad <matan@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>; Raslan Darawsheh <rasland@mellanox.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: [PATCH v2] net/mlx5: fix VLAN item match for DV mode
> 
> Currently MLX5 PMD can't match on untagged packets specifically.
> Tagged traffic still hits the flows intended for untagged packets.
> If the flow has ETH, it will catch all matching packets, tagged
> and untagged.
> The solution is to use cvlan_tag bit.
> If mask=1 and value=0 it matches on untagged traffic.
> If mask=1 and value=1 it matches on tagged traffic.
> This is the kernel implementation.
> 
> This patch updated MLX5 PMD to set cvlan_tag mask and value according
> to flow rule contents.
> This update is relevant when using DV flow engine (dv_flow_en=1).
> 
> See example at
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdoc.d
> pdk.org%2Fguides%2Fnics%2Fmlx5.html%23limitations&amp;data=02%7C01
> %7Crasland%40mellanox.com%7Ca0e3ceda39d24d8e045808d7aee24dcd%7C
> a652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637170159302953553&a
> mp;sdata=EtKSvNXJ7qiz10fi%2FQYGzEv0fbLH3VvhpYBGZsu9yZ0%3D&amp;re
> served=0.
> 
> Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> 
> v2: Update PMD documentation and release notes, update commit message.
> 
> ---
>  doc/guides/nics/mlx5.rst               |  9 +++------
>  doc/guides/rel_notes/release_20_02.rst |  1 +
>  drivers/net/mlx5/mlx5_flow_dv.c        | 11 +++++++++++
>  3 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
> index 2411fb3..2ea4fa9 100644
> --- a/doc/guides/nics/mlx5.rst
> +++ b/doc/guides/nics/mlx5.rst
> @@ -110,21 +110,18 @@ Limitations
>      process. If the external memory is registered by primary process but has
>      different virtual address in secondary process, unexpected error may
> happen.
> 
> -- Flow pattern without any specific vlan will match for vlan packets as well:
> +- When using Verbs flow engine (``dv_flow_en`` = 0), flow pattern without
> any
> +  specific VLAN will match for VLAN packets as well:
> 
>    When VLAN spec is not specified in the pattern, the matching rule will be
> created with VLAN as a wild card.
>    Meaning, the flow rule::
> 
>          flow create 0 ingress pattern eth / vlan vid is 3 / ipv4 / end ...
> 
> -  Will only match vlan packets with vid=3. and the flow rules::
> +  Will only match vlan packets with vid=3. and the flow rule::
> 
>          flow create 0 ingress pattern eth / ipv4 / end ...
> 
> -  Or::
> -
> -        flow create 0 ingress pattern eth / vlan / ipv4 / end ...
> -
>    Will match any ipv4 packet (VLAN included).
> 
>  - VLAN pop offload command:
> diff --git a/doc/guides/rel_notes/release_20_02.rst
> b/doc/guides/rel_notes/release_20_02.rst
> index 786f9b1..2ffccf2 100644
> --- a/doc/guides/rel_notes/release_20_02.rst
> +++ b/doc/guides/rel_notes/release_20_02.rst
> @@ -112,6 +112,7 @@ New Features
> 
>    * Added support for RSS using L3/L4 source/destination only.
>    * Added support for matching on GTP tunnel header item.
> +  * Removed limitation of matching on tagged/untagged packets (when
> using DV flow engine).
> 
>  * **Add new vDPA PMD based on Mellanox devices**
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index e71b3b5..bc067a2 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -5216,6 +5216,15 @@ struct field_modify_info modify_tcp[] = {
>  		 rte_be_to_cpu_16(eth_m->type));
>  	l24_v = MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_v,
> ethertype);
>  	*(uint16_t *)(l24_v) = eth_m->type & eth_v->type;
> +	if (eth_v->type) {
> +		/* When ethertype is present set mask for tagged VLAN. */
> +		MLX5_SET(fte_match_set_lyr_2_4, headers_m, cvlan_tag,
> 1);
> +		/* Set value for tagged VLAN if ethertype is 802.1Q. */
> +		if (eth_v->type == RTE_BE16(RTE_ETHER_TYPE_VLAN) ||
> +		    eth_v->type == RTE_BE16(RTE_ETHER_TYPE_QINQ))
> +			MLX5_SET(fte_match_set_lyr_2_4, headers_v,
> cvlan_tag,
> +				 1);
> +	}
>  }
> 
>  /**
> @@ -5356,6 +5365,7 @@ struct field_modify_info modify_tcp[] = {
>  		 ipv4_m->hdr.next_proto_id);
>  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol,
>  		 ipv4_v->hdr.next_proto_id & ipv4_m->hdr.next_proto_id);
> +	MLX5_SET(fte_match_set_lyr_2_4, headers_m, cvlan_tag, 1);
>  }
> 
>  /**
> @@ -5460,6 +5470,7 @@ struct field_modify_info modify_tcp[] = {
>  		 ipv6_m->hdr.proto);
>  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol,
>  		 ipv6_v->hdr.proto & ipv6_m->hdr.proto);
> +	MLX5_SET(fte_match_set_lyr_2_4, headers_m, cvlan_tag, 1);
>  }
> 
>  /**
> --
> 1.8.3.1

Patch applied to next-net-mlx,


Kindest regards,
Raslan Darawsheh