Message-ID: <711630a2-b810-f8b0-2dcf-1eb7056ecf1d@intel.com>
Date:   Tue, 14 Feb 2023 16:26:54 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP
 encap/decap flows offload
Content-Language: en-US
To:     Gavin Li <gavinl@nvidia.com>
CC:     <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>, <roopa@nvidia.com>,
        <eng.alaamohamedsoliman.am@gmail.com>, <bigeasy@linutronix.de>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        Roi Dayan <roid@nvidia.com>, Maor Dickman <maord@nvidia.com>,
        Saeed Mahameed <saeedm@nvidia.com>
References: <20230214134137.225999-1-gavinl@nvidia.com>
 <20230214134137.225999-4-gavinl@nvidia.com>
From:   Alexander Lobakin <alexandr.lobakin@intel.com>
In-Reply-To: <20230214134137.225999-4-gavinl@nvidia.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TklvN0FWSVcreXFKc0N6Y0lNdytkZlU0amkwOU1yL0xUKzBib29rK0tucUVX?=
 =?utf-8?B?VFQ1QVQySjJmQ3FKSVV3cDZhYjM4c2V1K3ZIWktKTUhHY3Q2WkFQUkZiRWh6?=
 =?utf-8?B?TW9scm5jZldqWXA1cDFla0ovSlVqMEkxWFV5RjE2UjBRN1M0elQ4VE41c1hx?=
 =?utf-8?B?aHU0cG1OSlhmbnpRQmlFaGNwTHhodUV5ZGdwVTFOb3pqVlpKVVpUZE01SWxG?=
 =?utf-8?B?OTk1T1M3bGo5V2Q0c2l1SEIrWnRrTm1mS1VNTmsvTGpvKzhHNWt5VnNPakNE?=
 =?utf-8?B?bmJZSFFlTE1wMEhzQW9ENWtkN2dnNG92MGhuUzdkQWtLRURjTWJjN1ZOemNw?=
 =?utf-8?B?THJRdnlSTE5UY3JRdTZ1T0ZkanQxNFRVKzJFcW13RmdTZnQzSXMvUVAvRm56?=
 =?utf-8?B?N0ErRFpFYnNwRUY5Y0dRYnpaYmZqT0lFNXRUWkNaQ0JMTUxmSkxVb1dORmgr?=
 =?utf-8?B?eE9qZ2FjS1psV0tXT1QzWldndC8vTE5IT050b0hRaXdiZyszR2t5WTl5UFVN?=
 =?utf-8?B?eFlmNzdOWS85T1BBZEdPeDU2VEdQdkRvM1R4eWF1eTJFdDhRYlhib2pZeUtk?=
 =?utf-8?B?Rit6cWhWYmJvNnI4MlY0Qkp1WDVYUkZXNWswbzh6U1B4d0tSUjhMb1V5RXZj?=
 =?utf-8?B?dWMrRFRwMThLa3F0b3JEY2QyWXptanBHMk9tOElXeGdjY3JKV3B3RlBkZFJq?=
 =?utf-8?B?VnF5clhyeFQxZ1FjUjZabFBKdDNSLzdMYzdON0xoNzFObHFjTEFFRlVYVlZY?=
 =?utf-8?B?TXhBeXFCZVhOTHkrWnJKVzhTYTZETGtRcld4cGZtWnM3d2s2TFFLLzdxYUJn?=
 =?utf-8?B?WlJrUEN4T0orVzFNN3FIVVk0a3pRcmJuRzFNTllBc2dzZW81TWtBeVc2emZO?=
 =?utf-8?B?dTlndGR5YnV1TDI1V1AwWUNFWFZsYmxrYUZzV0pzY3VVeWFFUzBSQitJakJD?=
 =?utf-8?B?MXQ0d0hCYU54WG44dFlMN3FqL0FpNkd0YVJUQWFnZjRsS1FsY05Cd3JPWEdG?=
 =?utf-8?B?cVpZZmM2SGhHbDBuYmRid2VpZTZmZUpTWjc2TnVEQW1vNGdXeHNUZGlpTXhO?=
 =?utf-8?B?VnJPdHZlTEp0N1pnYTVxQThZN1M1UjVRMTlFdm16UHY2dHoyNTdnNkt6UTh4?=
 =?utf-8?B?amF4Uk0rQ3N1bTFPcGl6SGNyZnZEclMrNDgrQ3dHV1Y2OFRHNHA3OVFyNm81?=
 =?utf-8?B?THp1aDFtVGNRYUc4MkNtV3NZd0dTQ3VRQmlvU2hCbmNoeENMYnMyQVlKeURi?=
 =?utf-8?B?aGVVTlR3ZmhLZzhTY3QyZXQ3UlVvQ1I5dnlLdFZyV0tTMXNVZWl4dktNcmtP?=
 =?utf-8?B?VTZVMnlhRVEyMEl6V3pJbnMyVFlDTEtVWGNWSjA0Z0VYYThOUXB4cEdmbXhD?=
 =?utf-8?B?anppekZnaS9sNlRibkZvQi9vTXZjLzIybzVwY0gxRFFNVlBzeFlTRzc1dGpn?=
 =?utf-8?B?Vm96VEhDWGk3Y2ZudWtRODRlaENMd3hKaG1pUkNiejhOdzNTYXZlZTFQWlJp?=
 =?utf-8?B?UVlYM3o2SVIzN2ZmbmJFL2NiZ3ZsN3YrUVMycU5iSFNuL3E0ZHlpVmFUT1dK?=
 =?utf-8?B?NDFUZlZuZUl4ZllER2ZzYTc1alUxYWtJemNMVnFEeVVwWU9LYXVvVll0VXpN?=
 =?utf-8?B?WDdpVkpCZnNvZGl4ckVEeVlUSlA4clhpSnhubHN6YklMdmNCRm9KUnUxYlUr?=
 =?utf-8?B?bXg4aU8vVEUzcURUSXZFVC8zckgvYkpnR1J1ZWpZb0FSWG5lT2ttM1VQd242?=
 =?utf-8?B?a3pERlJhUDNLU3pGOHpvcGRmUmkzZkhMK3R6WlVCR3BsbnMzQTRndWgwazlt?=
 =?utf-8?B?d3dDZnFZSklkemNZejlDNDdWdk0vdDBPUFlvdnhUY1QrU3YxY0pjTW9kMTUy?=
 =?utf-8?B?ZDBVUmYwUkFnSlhwMU5icWJ2R3kvUlVuek90eUwyRkZDRVYwRHk2TkNmOHo4?=
 =?utf-8?B?M2NMNnhpaEYySy9QR2phcjhoZkZJU0NhOE1KMzh6YWZwMlJrYnZWVXVPRk9E?=
 =?utf-8?B?dHI5d1BUOWxQcUMvZitXSE9GSVRaazJqSlJ2R2F5UWErQVZ5dFhmWGhEdU0r?=
 =?utf-8?B?NnoyRjNHMHRqUWFpYVpIN3FkdjNtSE0wOEZMd2NtYjE0L0xZTVlhN3doYkc4?=
 =?utf-8?B?MlNINjhJaW55TytNR1dsM3BsQ2xMb0ZkQktnRFhpVEFFWUhBRmFOakszd1dp?=
 =?utf-8?B?ZkE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 41453ed0-c2a2-4127-4763-08db0ea0122c
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Feb 2023 15:28:06.7067
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: OGN7tcKWdKQpPYaXyb5w1oa64tBcN5vf+/9/rIoAMUpLab8VP+6cDEU6NLgc7Ku5EsQuz5urC2TtsAEsIKBO0z3jv8GHaV0RqJ0itPKx58g=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB8018
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Gavin Li <gavinl@nvidia.com>
Date: Tue, 14 Feb 2023 15:41:37 +0200

> Add HW offloading support for TC flows with VxLAN GBP encap/decap.
> 
> Example of encap rule:
> tc filter add dev eth0 protocol ip ingress flower \
>     action tunnel_key set id 42 vxlan_opts 512 \
>     action mirred egress redirect dev vxlan1
> 
> Example of decap rule:
> tc filter add dev vxlan1 protocol ip ingress flower \
>     enc_key_id 42 enc_dst_port 4789 vxlan_opts 1024 \
>     action tunnel_key unset action mirred egress redirect dev eth0
> 
> Change-Id: I48f61d02201bf3f79dcbe5d0f022f7bb27ed630f
> Signed-off-by: Gavin Li <gavinl@nvidia.com>
> Reviewed-by: Roi Dayan <roid@nvidia.com>
> Reviewed-by: Maor Dickman <maord@nvidia.com>
> Acked-by: Saeed Mahameed <saeedm@nvidia.com>
> ---
>  .../mellanox/mlx5/core/en/tc_tun_vxlan.c      | 85 ++++++++++++++++++-
>  include/linux/mlx5/device.h                   |  6 ++
>  include/linux/mlx5/mlx5_ifc.h                 | 13 ++-
>  3 files changed, 100 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_vxlan.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_vxlan.c
> index 1f62c702b625..444512ca9e0d 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_vxlan.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_vxlan.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
>  /* Copyright (c) 2018 Mellanox Technologies. */
>  
> +#include <net/ip_tunnels.h>
>  #include <net/vxlan.h>
>  #include "lib/vxlan.h"
>  #include "en/tc_tun.h"
> @@ -86,9 +87,11 @@ static int mlx5e_gen_ip_tunnel_header_vxlan(char buf[],
>  	const struct ip_tunnel_key *tun_key = &e->tun_info->key;
>  	__be32 tun_id = tunnel_id_to_key32(tun_key->tun_id);
>  	struct udphdr *udp = (struct udphdr *)(buf);
> +	const struct vxlan_metadata *md;
>  	struct vxlanhdr *vxh;
>  
> -	if (tun_key->tun_flags & TUNNEL_VXLAN_OPT)
> +	if (tun_key->tun_flags & TUNNEL_VXLAN_OPT &&

A separate pair of braces is preferred around bitops.

> +	    e->tun_info->options_len != sizeof(*md))
>  		return -EOPNOTSUPP;
>  	vxh = (struct vxlanhdr *)((char *)udp + sizeof(struct udphdr));
>  	*ip_proto = IPPROTO_UDP;
> @@ -96,6 +99,70 @@ static int mlx5e_gen_ip_tunnel_header_vxlan(char buf[],
>  	udp->dest = tun_key->tp_dst;
>  	vxh->vx_flags = VXLAN_HF_VNI;
>  	vxh->vx_vni = vxlan_vni_field(tun_id);
> +	if (tun_key->tun_flags & TUNNEL_VXLAN_OPT) {
> +		md = ip_tunnel_info_opts((struct ip_tunnel_info *)e->tun_info);
> +		vxlan_build_gbp_hdr(vxh, tun_key->tun_flags,
> +				    (struct vxlan_metadata *)md);

Maybe constify both ip_tunnel_info_opts() and vxlan_build_gbp_hdr()
arguments instead of working around by casting away?

> +	}
> +
> +	return 0;
> +}
> +
> +static int mlx5e_tc_tun_parse_vxlan_gbp_option(struct mlx5e_priv *priv,
> +					       struct mlx5_flow_spec *spec,
> +					       struct flow_cls_offload *f)
> +{
> +	struct flow_rule *rule = flow_cls_offload_flow_rule(f);
> +	struct netlink_ext_ack *extack = f->common.extack;
> +	struct flow_match_enc_opts enc_opts;
> +	void *misc5_c, *misc5_v;
> +	u32 *gbp, *gbp_mask;
> +
> +	flow_rule_match_enc_opts(rule, &enc_opts);
> +
> +	if (memchr_inv(&enc_opts.mask->data, 0, sizeof(enc_opts.mask->data)) &&
> +	    !MLX5_CAP_ESW_FT_FIELD_SUPPORT_2(priv->mdev, tunnel_header_0_1)) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Matching on VxLAN GBP is not supported");
> +		netdev_warn(priv->netdev,
> +			    "Matching on VxLAN GBP is not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (enc_opts.key->dst_opt_type != TUNNEL_VXLAN_OPT) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Wrong VxLAN option type: not GBP");

Fits into one line I believe.

> +		netdev_warn(priv->netdev,
> +			    "Wrong VxLAN option type: not GBP\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (enc_opts.key->len != sizeof(*gbp) ||
> +	    enc_opts.mask->len != sizeof(*gbp_mask)) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "VxLAN GBP option/mask len is not 32 bits");
> +		netdev_warn(priv->netdev,
> +			    "VxLAN GBP option/mask len is not 32 bits\n");
> +		return -EINVAL;
> +	}
> +
> +	gbp = (u32 *)&enc_opts.key->data[0];
> +	gbp_mask = (u32 *)&enc_opts.mask->data[0];
> +
> +	if (*gbp_mask & ~VXLAN_GBP_MASK) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Wrong VxLAN GBP mask");

You can use new NL_SET_ERR_MSG_FMT_MOD() here to print @gbp_mask to the
user, as you do it next line.

> +		netdev_warn(priv->netdev,
> +			    "Wrong VxLAN GBP mask(0x%08X)\n", *gbp_mask);
> +		return -EINVAL;
> +	}
> +
> +	misc5_c = MLX5_ADDR_OF(fte_match_param, spec->match_criteria, misc_parameters_5);
> +	misc5_v = MLX5_ADDR_OF(fte_match_param, spec->match_value, misc_parameters_5);
> +	MLX5_SET(fte_match_set_misc5, misc5_c, tunnel_header_0, *gbp_mask);
> +	MLX5_SET(fte_match_set_misc5, misc5_v, tunnel_header_0, *gbp);
> +
> +	spec->match_criteria_enable |= MLX5_MATCH_MISC_PARAMETERS_5;
>  
>  	return 0;
>  }

Thanks,
Olek