[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7537ee48-c92a-a7df-b876-9a14ce2d34c6@nvidia.com>
Date: Wed, 15 Feb 2023 10:50:04 +0800
From: Gavin Li <gavinl@...dia.com>
To: Alexander Lobakin <alexandr.lobakin@...el.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, roopa@...dia.com,
eng.alaamohamedsoliman.am@...il.com, bigeasy@...utronix.de,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
Roi Dayan <roid@...dia.com>, Maor Dickman <maord@...dia.com>,
Saeed Mahameed <saeedm@...dia.com>
Subject: Re: [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP
encap/decap flows offload
On 2/14/2023 11:26 PM, Alexander Lobakin wrote:
> External email: Use caution opening links or attachments
>
>
> From: Gavin Li <gavinl@...dia.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@...dia.com>
>> Reviewed-by: Roi Dayan <roid@...dia.com>
>> Reviewed-by: Maor Dickman <maord@...dia.com>
>> Acked-by: Saeed Mahameed <saeedm@...dia.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.
ACK
>
>> + 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?
The reason to cast it is a WA to use ip_tunnel_info_opts which takes
non-const arg while
e->tun_info here is a const one.
>
>> + }
>> +
>> + 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.
ACK
>
>> + 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.
ACK
>
>> + 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
Powered by blists - more mailing lists