* [PATCH net-next v1 0/3] net/mlx5e: Add GBP VxLAN HW offload support
@ 2023-02-14 13:41 Gavin Li
2023-02-14 13:41 ` [PATCH net-next v1 1/3] vxlan: Expose helper vxlan_build_gbp_hdr Gavin Li
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Gavin Li @ 2023-02-14 13:41 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, roopa, nikolay,
eng.alaamohamedsoliman.am, bigeasy
Cc: netdev, linux-kernel
This patch series adds HW offloading support for TC flows with VxLAN GBP encap/decap.
Patch-1: Expose helper function vxlan_build_gbp_hdr.
Patch-2: Add helper function for encap_info_equal for tunnels with options.
Patch-3: Add HW offloading support for TC flows with VxLAN GBP encap/decap
in mlx ethernet driver.
Gavin Li (3):
vxlan: Expose helper vxlan_build_gbp_hdr
net/mlx5e: Add helper for encap_info_equal for tunnels with options
net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
.../ethernet/mellanox/mlx5/core/en/tc_tun.h | 3 +
.../mellanox/mlx5/core/en/tc_tun_encap.c | 29 +++++++
.../mellanox/mlx5/core/en/tc_tun_geneve.c | 24 +-----
.../mellanox/mlx5/core/en/tc_tun_vxlan.c | 85 ++++++++++++++++++-
drivers/net/vxlan/vxlan_core.c | 20 -----
include/linux/mlx5/device.h | 6 ++
include/linux/mlx5/mlx5_ifc.h | 13 ++-
include/net/vxlan.h | 20 +++++
8 files changed, 153 insertions(+), 47 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH net-next v1 1/3] vxlan: Expose helper vxlan_build_gbp_hdr
2023-02-14 13:41 [PATCH net-next v1 0/3] net/mlx5e: Add GBP VxLAN HW offload support Gavin Li
@ 2023-02-14 13:41 ` Gavin Li
2023-02-14 14:56 ` Alexander Lobakin
2023-02-14 13:41 ` [PATCH net-next v1 2/3] net/mlx5e: Add helper for encap_info_equal for tunnels with options Gavin Li
2023-02-14 13:41 ` [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload Gavin Li
2 siblings, 1 reply; 20+ messages in thread
From: Gavin Li @ 2023-02-14 13:41 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, roopa, nikolay,
eng.alaamohamedsoliman.am, bigeasy
Cc: netdev, linux-kernel, Roi Dayan, Maor Dickman, Saeed Mahameed
vxlan_build_gbp_hdr will be used by other modules to build gbp option in
vxlan header according to gbp flags.
Change-Id: I10d8dd31d6048e1fcd08cd76ee3bcd3029053552
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>
---
drivers/net/vxlan/vxlan_core.c | 20 --------------------
include/net/vxlan.h | 20 ++++++++++++++++++++
2 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index b1b179effe2a..bd44467a5a39 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -2140,26 +2140,6 @@ static bool route_shortcircuit(struct net_device *dev, struct sk_buff *skb)
return false;
}
-static void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, u32 vxflags,
- struct vxlan_metadata *md)
-{
- struct vxlanhdr_gbp *gbp;
-
- if (!md->gbp)
- return;
-
- gbp = (struct vxlanhdr_gbp *)vxh;
- vxh->vx_flags |= VXLAN_HF_GBP;
-
- if (md->gbp & VXLAN_GBP_DONT_LEARN)
- gbp->dont_learn = 1;
-
- if (md->gbp & VXLAN_GBP_POLICY_APPLIED)
- gbp->policy_applied = 1;
-
- gbp->policy_id = htons(md->gbp & VXLAN_GBP_ID_MASK);
-}
-
static int vxlan_build_gpe_hdr(struct vxlanhdr *vxh, u32 vxflags,
__be16 protocol)
{
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index bca5b01af247..08bc762a7e94 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -566,4 +566,24 @@ static inline bool vxlan_fdb_nh_path_select(struct nexthop *nh,
return true;
}
+static inline void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, u32 vxflags,
+ struct vxlan_metadata *md)
+{
+ struct vxlanhdr_gbp *gbp;
+
+ if (!md->gbp)
+ return;
+
+ gbp = (struct vxlanhdr_gbp *)vxh;
+ vxh->vx_flags |= VXLAN_HF_GBP;
+
+ if (md->gbp & VXLAN_GBP_DONT_LEARN)
+ gbp->dont_learn = 1;
+
+ if (md->gbp & VXLAN_GBP_POLICY_APPLIED)
+ gbp->policy_applied = 1;
+
+ gbp->policy_id = htons(md->gbp & VXLAN_GBP_ID_MASK);
+}
+
#endif
--
2.31.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH net-next v1 2/3] net/mlx5e: Add helper for encap_info_equal for tunnels with options
2023-02-14 13:41 [PATCH net-next v1 0/3] net/mlx5e: Add GBP VxLAN HW offload support Gavin Li
2023-02-14 13:41 ` [PATCH net-next v1 1/3] vxlan: Expose helper vxlan_build_gbp_hdr Gavin Li
@ 2023-02-14 13:41 ` Gavin Li
2023-02-14 15:01 ` Alexander Lobakin
2023-02-14 13:41 ` [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload Gavin Li
2 siblings, 1 reply; 20+ messages in thread
From: Gavin Li @ 2023-02-14 13:41 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, roopa, nikolay,
eng.alaamohamedsoliman.am, bigeasy
Cc: netdev, linux-kernel, Roi Dayan, Maor Dickman, Saeed Mahameed
For tunnels with options, eg, geneve and vxlan with gbp, they share the
same way to compare the headers and options. Extract the code as a common
function for them
Change-Id: I3ea697293c8d5d66c0c20080dbde88f60bcbd62f
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>
---
.../ethernet/mellanox/mlx5/core/en/tc_tun.h | 3 ++
.../mellanox/mlx5/core/en/tc_tun_encap.c | 29 +++++++++++++++++++
.../mellanox/mlx5/core/en/tc_tun_geneve.c | 24 +--------------
3 files changed, 33 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.h b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.h
index b38f693bbb52..92065568bb19 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.h
@@ -115,6 +115,9 @@ int mlx5e_tc_tun_parse_udp_ports(struct mlx5e_priv *priv,
bool mlx5e_tc_tun_encap_info_equal_generic(struct mlx5e_encap_key *a,
struct mlx5e_encap_key *b);
+bool mlx5e_tc_tun_encap_info_equal_options(struct mlx5e_encap_key *a,
+ struct mlx5e_encap_key *b,
+ __be16 tun_flags);
#endif /* CONFIG_MLX5_ESWITCH */
#endif //__MLX5_EN_TC_TUNNEL_H__
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
index 780224fd67a1..4df9d27a63ad 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
@@ -571,6 +571,35 @@ bool mlx5e_tc_tun_encap_info_equal_generic(struct mlx5e_encap_key *a,
a->tc_tunnel->tunnel_type == b->tc_tunnel->tunnel_type;
}
+bool mlx5e_tc_tun_encap_info_equal_options(struct mlx5e_encap_key *a,
+ struct mlx5e_encap_key *b,
+ __be16 tun_flags)
+{
+ struct ip_tunnel_info *a_info;
+ struct ip_tunnel_info *b_info;
+ bool a_has_opts, b_has_opts;
+
+ if (!mlx5e_tc_tun_encap_info_equal_generic(a, b))
+ return false;
+
+ a_has_opts = !!(a->ip_tun_key->tun_flags & tun_flags);
+ b_has_opts = !!(b->ip_tun_key->tun_flags & tun_flags);
+
+ /* keys are equal when both don't have any options attached */
+ if (!a_has_opts && !b_has_opts)
+ return true;
+
+ if (a_has_opts != b_has_opts)
+ return false;
+
+ /* options stored in memory next to ip_tunnel_info struct */
+ a_info = container_of(a->ip_tun_key, struct ip_tunnel_info, key);
+ b_info = container_of(b->ip_tun_key, struct ip_tunnel_info, key);
+
+ return a_info->options_len == b_info->options_len &&
+ memcmp(a_info + 1, b_info + 1, a_info->options_len) == 0;
+}
+
static int cmp_decap_info(struct mlx5e_decap_key *a,
struct mlx5e_decap_key *b)
{
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_geneve.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_geneve.c
index 054d80c4e65c..2bcd10b6d653 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_geneve.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_geneve.c
@@ -337,29 +337,7 @@ static int mlx5e_tc_tun_parse_geneve(struct mlx5e_priv *priv,
static bool mlx5e_tc_tun_encap_info_equal_geneve(struct mlx5e_encap_key *a,
struct mlx5e_encap_key *b)
{
- struct ip_tunnel_info *a_info;
- struct ip_tunnel_info *b_info;
- bool a_has_opts, b_has_opts;
-
- if (!mlx5e_tc_tun_encap_info_equal_generic(a, b))
- return false;
-
- a_has_opts = !!(a->ip_tun_key->tun_flags & TUNNEL_GENEVE_OPT);
- b_has_opts = !!(b->ip_tun_key->tun_flags & TUNNEL_GENEVE_OPT);
-
- /* keys are equal when both don't have any options attached */
- if (!a_has_opts && !b_has_opts)
- return true;
-
- if (a_has_opts != b_has_opts)
- return false;
-
- /* geneve options stored in memory next to ip_tunnel_info struct */
- a_info = container_of(a->ip_tun_key, struct ip_tunnel_info, key);
- b_info = container_of(b->ip_tun_key, struct ip_tunnel_info, key);
-
- return a_info->options_len == b_info->options_len &&
- memcmp(a_info + 1, b_info + 1, a_info->options_len) == 0;
+ return mlx5e_tc_tun_encap_info_equal_options(a, b, TUNNEL_GENEVE_OPT);
}
struct mlx5e_tc_tunnel geneve_tunnel = {
--
2.31.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
2023-02-14 13:41 [PATCH net-next v1 0/3] net/mlx5e: Add GBP VxLAN HW offload support Gavin Li
2023-02-14 13:41 ` [PATCH net-next v1 1/3] vxlan: Expose helper vxlan_build_gbp_hdr Gavin Li
2023-02-14 13:41 ` [PATCH net-next v1 2/3] net/mlx5e: Add helper for encap_info_equal for tunnels with options Gavin Li
@ 2023-02-14 13:41 ` Gavin Li
2023-02-14 15:26 ` Alexander Lobakin
2 siblings, 1 reply; 20+ messages in thread
From: Gavin Li @ 2023-02-14 13:41 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, roopa, nikolay,
eng.alaamohamedsoliman.am, bigeasy
Cc: netdev, linux-kernel, Roi Dayan, Maor Dickman, Saeed Mahameed
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 &&
+ 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);
+ }
+
+ 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");
+ 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");
+ 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;
}
@@ -122,6 +189,14 @@ static int mlx5e_tc_tun_parse_vxlan(struct mlx5e_priv *priv,
if (!enc_keyid.mask->keyid)
return 0;
+ if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_OPTS)) {
+ int err;
+
+ err = mlx5e_tc_tun_parse_vxlan_gbp_option(priv, spec, f);
+ if (err)
+ return err;
+ }
+
/* match on VNI is required */
if (!MLX5_CAP_ESW_FLOWTABLE_FDB(priv->mdev,
@@ -143,6 +218,12 @@ static int mlx5e_tc_tun_parse_vxlan(struct mlx5e_priv *priv,
return 0;
}
+static bool mlx5e_tc_tun_encap_info_equal_vxlan(struct mlx5e_encap_key *a,
+ struct mlx5e_encap_key *b)
+{
+ return mlx5e_tc_tun_encap_info_equal_options(a, b, TUNNEL_VXLAN_OPT);
+}
+
static int mlx5e_tc_tun_get_remote_ifindex(struct net_device *mirred_dev)
{
const struct vxlan_dev *vxlan = netdev_priv(mirred_dev);
@@ -160,6 +241,6 @@ struct mlx5e_tc_tunnel vxlan_tunnel = {
.generate_ip_tun_hdr = mlx5e_gen_ip_tunnel_header_vxlan,
.parse_udp_ports = mlx5e_tc_tun_parse_udp_ports_vxlan,
.parse_tunnel = mlx5e_tc_tun_parse_vxlan,
- .encap_info_equal = mlx5e_tc_tun_encap_info_equal_generic,
+ .encap_info_equal = mlx5e_tc_tun_encap_info_equal_vxlan,
.get_remote_ifindex = mlx5e_tc_tun_get_remote_ifindex,
};
diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
index 71b06ebad402..af4dd536a52c 100644
--- a/include/linux/mlx5/device.h
+++ b/include/linux/mlx5/device.h
@@ -1357,6 +1357,12 @@ enum mlx5_qcam_feature_groups {
#define MLX5_CAP_ESW_INGRESS_ACL_MAX(mdev, cap) \
MLX5_CAP_ESW_FLOWTABLE_MAX(mdev, flow_table_properties_esw_acl_ingress.cap)
+#define MLX5_CAP_ESW_FT_FIELD_SUPPORT_2(mdev, cap) \
+ MLX5_CAP_ESW_FLOWTABLE(mdev, ft_field_support_2_esw_fdb.cap)
+
+#define MLX5_CAP_ESW_FT_FIELD_SUPPORT_2_MAX(mdev, cap) \
+ MLX5_CAP_ESW_FLOWTABLE_MAX(mdev, ft_field_support_2_esw_fdb.cap)
+
#define MLX5_CAP_ESW(mdev, cap) \
MLX5_GET(e_switch_cap, \
mdev->caps.hca[MLX5_CAP_ESWITCH]->cur, cap)
diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index 1e530a8a2cf5..caef6aa20454 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -399,10 +399,13 @@ struct mlx5_ifc_flow_table_fields_supported_bits {
u8 metadata_reg_c_0[0x1];
};
+/* Table 2170 - Flow Table Fields Supported 2 Format */
struct mlx5_ifc_flow_table_fields_supported_2_bits {
u8 reserved_at_0[0xe];
u8 bth_opcode[0x1];
- u8 reserved_at_f[0x11];
+ u8 reserved_at_f[0x1];
+ u8 tunnel_header_0_1[0x1];
+ u8 reserved_at_11[0xf];
u8 reserved_at_20[0x60];
};
@@ -890,7 +893,13 @@ struct mlx5_ifc_flow_table_eswitch_cap_bits {
struct mlx5_ifc_flow_table_prop_layout_bits flow_table_properties_esw_acl_egress;
- u8 reserved_at_800[0x1000];
+ u8 reserved_at_800[0xC00];
+
+ struct mlx5_ifc_flow_table_fields_supported_2_bits ft_field_support_2_esw_fdb;
+
+ struct mlx5_ifc_flow_table_fields_supported_2_bits ft_field_bitmask_support_2_esw_fdb;
+
+ u8 reserved_at_1500[0x300];
u8 sw_steering_fdb_action_drop_icm_address_rx[0x40];
--
2.31.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 1/3] vxlan: Expose helper vxlan_build_gbp_hdr
2023-02-14 13:41 ` [PATCH net-next v1 1/3] vxlan: Expose helper vxlan_build_gbp_hdr Gavin Li
@ 2023-02-14 14:56 ` Alexander Lobakin
2023-02-15 8:26 ` Gavin Li
0 siblings, 1 reply; 20+ messages in thread
From: Alexander Lobakin @ 2023-02-14 14:56 UTC (permalink / raw)
To: Gavin Li
Cc: davem, edumazet, kuba, pabeni, roopa, nikolay,
eng.alaamohamedsoliman.am, bigeasy, netdev, linux-kernel,
Roi Dayan, Maor Dickman, Saeed Mahameed
From: Gavin Li <gavinl@nvidia.com>
Date: Tue, 14 Feb 2023 15:41:35 +0200
> vxlan_build_gbp_hdr will be used by other modules to build gbp option in
> vxlan header according to gbp flags.
(not sure if it's okay to write abbreviations with non-capital)
>
> Change-Id: I10d8dd31d6048e1fcd08cd76ee3bcd3029053552
> 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>
Besides the nit above:
Reviewed-by: Alexander Lobakin <alexandr.lobakin@intel.com>
> ---
> drivers/net/vxlan/vxlan_core.c | 20 --------------------
> include/net/vxlan.h | 20 ++++++++++++++++++++
> 2 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index b1b179effe2a..bd44467a5a39 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
> @@ -2140,26 +2140,6 @@ static bool route_shortcircuit(struct net_device *dev, struct sk_buff *skb)
> return false;
> }
>
> -static void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, u32 vxflags,
> - struct vxlan_metadata *md)
> -{
> - struct vxlanhdr_gbp *gbp;
> -
> - if (!md->gbp)
> - return;
> -
> - gbp = (struct vxlanhdr_gbp *)vxh;
> - vxh->vx_flags |= VXLAN_HF_GBP;
> -
> - if (md->gbp & VXLAN_GBP_DONT_LEARN)
> - gbp->dont_learn = 1;
> -
> - if (md->gbp & VXLAN_GBP_POLICY_APPLIED)
> - gbp->policy_applied = 1;
> -
> - gbp->policy_id = htons(md->gbp & VXLAN_GBP_ID_MASK);
> -}
> -
> static int vxlan_build_gpe_hdr(struct vxlanhdr *vxh, u32 vxflags,
> __be16 protocol)
> {
> diff --git a/include/net/vxlan.h b/include/net/vxlan.h
> index bca5b01af247..08bc762a7e94 100644
> --- a/include/net/vxlan.h
> +++ b/include/net/vxlan.h
> @@ -566,4 +566,24 @@ static inline bool vxlan_fdb_nh_path_select(struct nexthop *nh,
> return true;
> }
>
> +static inline void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, u32 vxflags,
> + struct vxlan_metadata *md)
> +{
> + struct vxlanhdr_gbp *gbp;
> +
> + if (!md->gbp)
> + return;
> +
> + gbp = (struct vxlanhdr_gbp *)vxh;
> + vxh->vx_flags |= VXLAN_HF_GBP;
> +
> + if (md->gbp & VXLAN_GBP_DONT_LEARN)
> + gbp->dont_learn = 1;
> +
> + if (md->gbp & VXLAN_GBP_POLICY_APPLIED)
> + gbp->policy_applied = 1;
> +
> + gbp->policy_id = htons(md->gbp & VXLAN_GBP_ID_MASK);
> +}
> +
> #endif
Thanks,
Olek
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 2/3] net/mlx5e: Add helper for encap_info_equal for tunnels with options
2023-02-14 13:41 ` [PATCH net-next v1 2/3] net/mlx5e: Add helper for encap_info_equal for tunnels with options Gavin Li
@ 2023-02-14 15:01 ` Alexander Lobakin
2023-02-15 2:54 ` Gavin Li
0 siblings, 1 reply; 20+ messages in thread
From: Alexander Lobakin @ 2023-02-14 15:01 UTC (permalink / raw)
To: Gavin Li
Cc: davem, edumazet, kuba, pabeni, roopa, eng.alaamohamedsoliman.am,
bigeasy, netdev, linux-kernel, Roi Dayan, Maor Dickman,
Saeed Mahameed
From: Gavin Li <gavinl@nvidia.com>
Date: Tue, 14 Feb 2023 15:41:36 +0200
(dropping non-existent nikolay@nvidia.com)
> For tunnels with options, eg, geneve and vxlan with gbp, they share the
> same way to compare the headers and options. Extract the code as a common
> function for them
>
> Change-Id: I3ea697293c8d5d66c0c20080dbde88f60bcbd62f
> 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>
[...]
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
> index 780224fd67a1..4df9d27a63ad 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
> @@ -571,6 +571,35 @@ bool mlx5e_tc_tun_encap_info_equal_generic(struct mlx5e_encap_key *a,
> a->tc_tunnel->tunnel_type == b->tc_tunnel->tunnel_type;
> }
>
> +bool mlx5e_tc_tun_encap_info_equal_options(struct mlx5e_encap_key *a,
> + struct mlx5e_encap_key *b,
> + __be16 tun_flags)
> +{
> + struct ip_tunnel_info *a_info;
> + struct ip_tunnel_info *b_info;
> + bool a_has_opts, b_has_opts;
> +
> + if (!mlx5e_tc_tun_encap_info_equal_generic(a, b))
> + return false;
> +
> + a_has_opts = !!(a->ip_tun_key->tun_flags & tun_flags);
> + b_has_opts = !!(b->ip_tun_key->tun_flags & tun_flags);
> +
> + /* keys are equal when both don't have any options attached */
> + if (!a_has_opts && !b_has_opts)
> + return true;
> +
> + if (a_has_opts != b_has_opts)
> + return false;
> +
> + /* options stored in memory next to ip_tunnel_info struct */
> + a_info = container_of(a->ip_tun_key, struct ip_tunnel_info, key);
> + b_info = container_of(b->ip_tun_key, struct ip_tunnel_info, key);
> +
> + return a_info->options_len == b_info->options_len &&
> + memcmp(a_info + 1, b_info + 1, a_info->options_len) == 0;
1. memcmp() is not aligned to the first expr (off-by-one to the right).
2. `!expr` is preferred over `expr == 0`.
> +}
> +
> static int cmp_decap_info(struct mlx5e_decap_key *a,
> struct mlx5e_decap_key *b)
> {
[...]
Thanks,
Olek
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
2023-02-14 13:41 ` [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload Gavin Li
@ 2023-02-14 15:26 ` Alexander Lobakin
2023-02-15 2:50 ` Gavin Li
2023-02-15 3:36 ` Gavin Li
0 siblings, 2 replies; 20+ messages in thread
From: Alexander Lobakin @ 2023-02-14 15:26 UTC (permalink / raw)
To: Gavin Li
Cc: davem, edumazet, kuba, pabeni, roopa, eng.alaamohamedsoliman.am,
bigeasy, netdev, linux-kernel, Roi Dayan, Maor Dickman,
Saeed Mahameed
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
2023-02-14 15:26 ` Alexander Lobakin
@ 2023-02-15 2:50 ` Gavin Li
2023-02-15 16:53 ` Alexander Lobakin
2023-02-15 3:36 ` Gavin Li
1 sibling, 1 reply; 20+ messages in thread
From: Gavin Li @ 2023-02-15 2:50 UTC (permalink / raw)
To: Alexander Lobakin
Cc: davem, edumazet, kuba, pabeni, roopa, eng.alaamohamedsoliman.am,
bigeasy, netdev, linux-kernel, Roi Dayan, Maor Dickman,
Saeed Mahameed
On 2/14/2023 11:26 PM, Alexander Lobakin wrote:
> External email: Use caution opening links or attachments
>
>
> 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.
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 2/3] net/mlx5e: Add helper for encap_info_equal for tunnels with options
2023-02-14 15:01 ` Alexander Lobakin
@ 2023-02-15 2:54 ` Gavin Li
2023-02-15 16:56 ` Alexander Lobakin
0 siblings, 1 reply; 20+ messages in thread
From: Gavin Li @ 2023-02-15 2:54 UTC (permalink / raw)
To: Alexander Lobakin
Cc: davem, edumazet, kuba, pabeni, roopa, eng.alaamohamedsoliman.am,
bigeasy, netdev, linux-kernel, Roi Dayan, Maor Dickman,
Saeed Mahameed
On 2/14/2023 11:01 PM, Alexander Lobakin wrote:
> External email: Use caution opening links or attachments
>
>
> From: Gavin Li <gavinl@nvidia.com>
> Date: Tue, 14 Feb 2023 15:41:36 +0200
>
> (dropping non-existent nikolay@nvidia.com)
>
>> For tunnels with options, eg, geneve and vxlan with gbp, they share the
>> same way to compare the headers and options. Extract the code as a common
>> function for them
>>
>> Change-Id: I3ea697293c8d5d66c0c20080dbde88f60bcbd62f
>> 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>
> [...]
>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
>> index 780224fd67a1..4df9d27a63ad 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
>> @@ -571,6 +571,35 @@ bool mlx5e_tc_tun_encap_info_equal_generic(struct mlx5e_encap_key *a,
>> a->tc_tunnel->tunnel_type == b->tc_tunnel->tunnel_type;
>> }
>>
>> +bool mlx5e_tc_tun_encap_info_equal_options(struct mlx5e_encap_key *a,
>> + struct mlx5e_encap_key *b,
>> + __be16 tun_flags)
>> +{
>> + struct ip_tunnel_info *a_info;
>> + struct ip_tunnel_info *b_info;
>> + bool a_has_opts, b_has_opts;
>> +
>> + if (!mlx5e_tc_tun_encap_info_equal_generic(a, b))
>> + return false;
>> +
>> + a_has_opts = !!(a->ip_tun_key->tun_flags & tun_flags);
>> + b_has_opts = !!(b->ip_tun_key->tun_flags & tun_flags);
>> +
>> + /* keys are equal when both don't have any options attached */
>> + if (!a_has_opts && !b_has_opts)
>> + return true;
>> +
>> + if (a_has_opts != b_has_opts)
>> + return false;
>> +
>> + /* options stored in memory next to ip_tunnel_info struct */
>> + a_info = container_of(a->ip_tun_key, struct ip_tunnel_info, key);
>> + b_info = container_of(b->ip_tun_key, struct ip_tunnel_info, key);
>> +
>> + return a_info->options_len == b_info->options_len &&
>> + memcmp(a_info + 1, b_info + 1, a_info->options_len) == 0;
> 1. memcmp() is not aligned to the first expr (off-by-one to the right).
Options start from "info + 1", see ip_tunnel_info_opts and will use it
here to replace the "info+1".
> 2. `!expr` is preferred over `expr == 0`.
ACK
>
>> +}
>> +
>> static int cmp_decap_info(struct mlx5e_decap_key *a,
>> struct mlx5e_decap_key *b)
>> {
> [...]
>
> Thanks,
> Olek
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
2023-02-14 15:26 ` Alexander Lobakin
2023-02-15 2:50 ` Gavin Li
@ 2023-02-15 3:36 ` Gavin Li
2023-02-15 8:30 ` Gavin Li
2023-02-15 16:57 ` Alexander Lobakin
1 sibling, 2 replies; 20+ messages in thread
From: Gavin Li @ 2023-02-15 3:36 UTC (permalink / raw)
To: Alexander Lobakin
Cc: davem, edumazet, kuba, pabeni, roopa, eng.alaamohamedsoliman.am,
bigeasy, netdev, linux-kernel, Roi Dayan, Maor Dickman,
Saeed Mahameed
On 2/14/2023 11:26 PM, Alexander Lobakin wrote:
> External email: Use caution opening links or attachments
>
>
> 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?
ACK. Sorry for the confusion---I misunderstood the comment.
>
>> + }
>> +
>> + 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 1/3] vxlan: Expose helper vxlan_build_gbp_hdr
2023-02-14 14:56 ` Alexander Lobakin
@ 2023-02-15 8:26 ` Gavin Li
0 siblings, 0 replies; 20+ messages in thread
From: Gavin Li @ 2023-02-15 8:26 UTC (permalink / raw)
To: Alexander Lobakin
Cc: davem, edumazet, kuba, pabeni, Roopa Prabhu,
eng.alaamohamedsoliman.am, bigeasy, netdev, linux-kernel,
Roi Dayan, Maor Dickman, Saeed Mahameed
On 2/14/2023 10:56 PM, Alexander Lobakin wrote:
> External email: Use caution opening links or attachments
>
>
> From: Gavin Li <gavinl@nvidia.com>
> Date: Tue, 14 Feb 2023 15:41:35 +0200
>
>> vxlan_build_gbp_hdr will be used by other modules to build gbp option in
>> vxlan header according to gbp flags.
> (not sure if it's okay to write abbreviations with non-capital)
Don't sure either but most of the functions in vxlan (if not all) are
declared in this way.
>
>> Change-Id: I10d8dd31d6048e1fcd08cd76ee3bcd3029053552
>> 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>
> Besides the nit above:
>
> Reviewed-by: Alexander Lobakin <alexandr.lobakin@intel.com>
>
>> ---
>> drivers/net/vxlan/vxlan_core.c | 20 --------------------
>> include/net/vxlan.h | 20 ++++++++++++++++++++
>> 2 files changed, 20 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
>> index b1b179effe2a..bd44467a5a39 100644
>> --- a/drivers/net/vxlan/vxlan_core.c
>> +++ b/drivers/net/vxlan/vxlan_core.c
>> @@ -2140,26 +2140,6 @@ static bool route_shortcircuit(struct net_device *dev, struct sk_buff *skb)
>> return false;
>> }
>>
>> -static void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, u32 vxflags,
>> - struct vxlan_metadata *md)
>> -{
>> - struct vxlanhdr_gbp *gbp;
>> -
>> - if (!md->gbp)
>> - return;
>> -
>> - gbp = (struct vxlanhdr_gbp *)vxh;
>> - vxh->vx_flags |= VXLAN_HF_GBP;
>> -
>> - if (md->gbp & VXLAN_GBP_DONT_LEARN)
>> - gbp->dont_learn = 1;
>> -
>> - if (md->gbp & VXLAN_GBP_POLICY_APPLIED)
>> - gbp->policy_applied = 1;
>> -
>> - gbp->policy_id = htons(md->gbp & VXLAN_GBP_ID_MASK);
>> -}
>> -
>> static int vxlan_build_gpe_hdr(struct vxlanhdr *vxh, u32 vxflags,
>> __be16 protocol)
>> {
>> diff --git a/include/net/vxlan.h b/include/net/vxlan.h
>> index bca5b01af247..08bc762a7e94 100644
>> --- a/include/net/vxlan.h
>> +++ b/include/net/vxlan.h
>> @@ -566,4 +566,24 @@ static inline bool vxlan_fdb_nh_path_select(struct nexthop *nh,
>> return true;
>> }
>>
>> +static inline void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, u32 vxflags,
>> + struct vxlan_metadata *md)
>> +{
>> + struct vxlanhdr_gbp *gbp;
>> +
>> + if (!md->gbp)
>> + return;
>> +
>> + gbp = (struct vxlanhdr_gbp *)vxh;
>> + vxh->vx_flags |= VXLAN_HF_GBP;
>> +
>> + if (md->gbp & VXLAN_GBP_DONT_LEARN)
>> + gbp->dont_learn = 1;
>> +
>> + if (md->gbp & VXLAN_GBP_POLICY_APPLIED)
>> + gbp->policy_applied = 1;
>> +
>> + gbp->policy_id = htons(md->gbp & VXLAN_GBP_ID_MASK);
>> +}
>> +
>> #endif
> Thanks,
> Olek
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
2023-02-15 3:36 ` Gavin Li
@ 2023-02-15 8:30 ` Gavin Li
2023-02-15 17:01 ` Alexander Lobakin
2023-02-15 16:57 ` Alexander Lobakin
1 sibling, 1 reply; 20+ messages in thread
From: Gavin Li @ 2023-02-15 8:30 UTC (permalink / raw)
To: Alexander Lobakin
Cc: davem, edumazet, kuba, pabeni, roopa, eng.alaamohamedsoliman.am,
bigeasy, netdev, linux-kernel, Roi Dayan, Maor Dickman,
Saeed Mahameed
On 2/15/2023 11:36 AM, Gavin Li wrote:
> External email: Use caution opening links or attachments
>
>
> On 2/14/2023 11:26 PM, Alexander Lobakin wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 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.
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?
> ACK. Sorry for the confusion---I misunderstood the comment.
This ip_tunnel_info_opts is tricky to use const to annotate the arg
because it will have to cast from const to non-const again upon returning.
>>
>>> + }
>>> +
>>> + 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
2023-02-15 2:50 ` Gavin Li
@ 2023-02-15 16:53 ` Alexander Lobakin
0 siblings, 0 replies; 20+ messages in thread
From: Alexander Lobakin @ 2023-02-15 16:53 UTC (permalink / raw)
To: Gavin Li
Cc: davem, edumazet, kuba, pabeni, roopa, eng.alaamohamedsoliman.am,
bigeasy, netdev, linux-kernel, Roi Dayan, Maor Dickman,
Saeed Mahameed
From: Gavin Li <gavinl@nvidia.com>
Date: Wed, 15 Feb 2023 10:50:04 +0800
>
> On 2/14/2023 11:26 PM, Alexander Lobakin wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 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.
[...]
>>> @@ -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.
That's what I'm asking - why not make ip_tunnel_info_opts() and
vxlan_build_gbp_hdr() take const tun_info?
>
>>
>>> + }
>>> +
>>> + return 0;
>>> +}
[...]
Thanks,
Olek
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 2/3] net/mlx5e: Add helper for encap_info_equal for tunnels with options
2023-02-15 2:54 ` Gavin Li
@ 2023-02-15 16:56 ` Alexander Lobakin
2023-02-20 11:31 ` Gavin Li
0 siblings, 1 reply; 20+ messages in thread
From: Alexander Lobakin @ 2023-02-15 16:56 UTC (permalink / raw)
To: Gavin Li
Cc: davem, edumazet, kuba, pabeni, roopa, eng.alaamohamedsoliman.am,
bigeasy, netdev, linux-kernel, Roi Dayan, Maor Dickman,
Saeed Mahameed
From: Gavin Li <gavinl@nvidia.com>
Date: Wed, 15 Feb 2023 10:54:12 +0800
>
> On 2/14/2023 11:01 PM, Alexander Lobakin wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> From: Gavin Li <gavinl@nvidia.com>
>> Date: Tue, 14 Feb 2023 15:41:36 +0200
[...]
>>> + if (a_has_opts != b_has_opts)
>>> + return false;
>>> +
>>> + /* options stored in memory next to ip_tunnel_info struct */
>>> + a_info = container_of(a->ip_tun_key, struct ip_tunnel_info, key);
>>> + b_info = container_of(b->ip_tun_key, struct ip_tunnel_info, key);
>>> +
>>> + return a_info->options_len == b_info->options_len &&
>>> + memcmp(a_info + 1, b_info + 1, a_info->options_len) == 0;
>> 1. memcmp() is not aligned to the first expr (off-by-one to the right).
> Options start from "info + 1", see ip_tunnel_info_opts and will use it
> here to replace the "info+1".
Nah, I mean the following. Your code:
return a_info->options_len == b_info->options_len &&
memcmp(a_info + 1, b_info + 1, ...
should be:
return a_info->options_len == b_info->options_len &&
memcmp(a_info + 1, b_info + 1, ...
7 spaces instead of a tab to have it aligned to the prev line.
>> 2. `!expr` is preferred over `expr == 0`.
> ACK
>>
>>> +}
>>> +
>>> static int cmp_decap_info(struct mlx5e_decap_key *a,
>>> struct mlx5e_decap_key *b)
>>> {
>> [...]
>>
>> Thanks,
>> Olek
Thanks,
Olek
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
2023-02-15 3:36 ` Gavin Li
2023-02-15 8:30 ` Gavin Li
@ 2023-02-15 16:57 ` Alexander Lobakin
1 sibling, 0 replies; 20+ messages in thread
From: Alexander Lobakin @ 2023-02-15 16:57 UTC (permalink / raw)
To: Gavin Li
Cc: davem, edumazet, kuba, pabeni, roopa, eng.alaamohamedsoliman.am,
bigeasy, netdev, linux-kernel, Roi Dayan, Maor Dickman,
Saeed Mahameed
From: Gavin Li <gavinl@nvidia.com>
Date: Wed, 15 Feb 2023 11:36:24 +0800
>
> On 2/14/2023 11:26 PM, Alexander Lobakin wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> From: Gavin Li <gavinl@nvidia.com>
>> Date: Tue, 14 Feb 2023 15:41:37 +0200
[...]
>>> @@ -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?
> ACK. Sorry for the confusion---I misunderstood the comment.
Ah, no worries :D Sorry that I sent the prev mail and only then opened
this one.
>>
>>> + }
>>> +
>>> + return 0;
>>> +}
Thanks,
Olek
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
2023-02-15 8:30 ` Gavin Li
@ 2023-02-15 17:01 ` Alexander Lobakin
2023-02-16 8:40 ` Gavin Li
0 siblings, 1 reply; 20+ messages in thread
From: Alexander Lobakin @ 2023-02-15 17:01 UTC (permalink / raw)
To: Gavin Li
Cc: davem, edumazet, kuba, pabeni, roopa, eng.alaamohamedsoliman.am,
bigeasy, netdev, linux-kernel, Roi Dayan, Maor Dickman,
Saeed Mahameed
From: Gavin Li <gavinl@nvidia.com>
Date: Wed, 15 Feb 2023 16:30:04 +0800
>
> On 2/15/2023 11:36 AM, Gavin Li wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 2/14/2023 11:26 PM, Alexander Lobakin wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> From: Gavin Li <gavinl@nvidia.com>
>>> Date: Tue, 14 Feb 2023 15:41:37 +0200
[...]
>>>> @@ -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?
>> ACK. Sorry for the confusion---I misunderstood the comment.
> This ip_tunnel_info_opts is tricky to use const to annotate the arg
> because it will have to cast from const to non-const again upon returning.
It's okay to cast away for the `void *` returned.
Alternatively, use can convert it to a macro and use
__builtin_choose_expr() or _Generic to return const or non-const
depending on whether the argument is constant. That's what was recently
done for container_of() IIRC.
>>>
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
[...]
Thanks,
Olek
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
2023-02-15 17:01 ` Alexander Lobakin
@ 2023-02-16 8:40 ` Gavin Li
2023-02-16 15:19 ` Alexander Lobakin
0 siblings, 1 reply; 20+ messages in thread
From: Gavin Li @ 2023-02-16 8:40 UTC (permalink / raw)
To: Alexander Lobakin
Cc: davem, edumazet, kuba, pabeni, roopa, eng.alaamohamedsoliman.am,
bigeasy, netdev, linux-kernel, Roi Dayan, Maor Dickman,
Saeed Mahameed
On 2/16/2023 1:01 AM, Alexander Lobakin wrote:
> External email: Use caution opening links or attachments
>
>
> From: Gavin Li <gavinl@nvidia.com>
> Date: Wed, 15 Feb 2023 16:30:04 +0800
>
>> On 2/15/2023 11:36 AM, Gavin Li wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 2/14/2023 11:26 PM, Alexander Lobakin wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> From: Gavin Li <gavinl@nvidia.com>
>>>> Date: Tue, 14 Feb 2023 15:41:37 +0200
> [...]
>
>>>>> @@ -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?
>>> ACK. Sorry for the confusion---I misunderstood the comment.
>> This ip_tunnel_info_opts is tricky to use const to annotate the arg
>> because it will have to cast from const to non-const again upon returning.
> It's okay to cast away for the `void *` returned.
> Alternatively, use can convert it to a macro and use
> __builtin_choose_expr() or _Generic to return const or non-const
> depending on whether the argument is constant. That's what was recently
> done for container_of() IIRC.
I've fixed vxlan_build_gbp_hdr in V2. For ip_tunnel_info_opts, it's
confusing to me.
It would be as below after constifying the parameter.
static inline void *ip_tunnel_info_opts(const struct ip_tunnel_info *info)
{
return (void *)(info + 1);
}
Is there any value gained by this change?
>
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
> [...]
>
> Thanks,
> Olek
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
2023-02-16 8:40 ` Gavin Li
@ 2023-02-16 15:19 ` Alexander Lobakin
2023-02-17 2:43 ` Gavin Li
0 siblings, 1 reply; 20+ messages in thread
From: Alexander Lobakin @ 2023-02-16 15:19 UTC (permalink / raw)
To: Gavin Li
Cc: davem, edumazet, kuba, pabeni, roopa, eng.alaamohamedsoliman.am,
bigeasy, netdev, linux-kernel, Roi Dayan, Maor Dickman,
Saeed Mahameed
From: Gavin Li <gavinl@nvidia.com>
Date: Thu, 16 Feb 2023 16:40:33 +0800
>
> On 2/16/2023 1:01 AM, Alexander Lobakin wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> From: Gavin Li <gavinl@nvidia.com>
>> Date: Wed, 15 Feb 2023 16:30:04 +0800
>>
>>> On 2/15/2023 11:36 AM, Gavin Li wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> On 2/14/2023 11:26 PM, Alexander Lobakin wrote:
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> From: Gavin Li <gavinl@nvidia.com>
>>>>> Date: Tue, 14 Feb 2023 15:41:37 +0200
>> [...]
>>
>>>>>> @@ -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?
>>>> ACK. Sorry for the confusion---I misunderstood the comment.
>>> This ip_tunnel_info_opts is tricky to use const to annotate the arg
>>> because it will have to cast from const to non-const again upon
>>> returning.
>> It's okay to cast away for the `void *` returned.
>> Alternatively, use can convert it to a macro and use
>> __builtin_choose_expr() or _Generic to return const or non-const
>> depending on whether the argument is constant. That's what was recently
>> done for container_of() IIRC.
>
> I've fixed vxlan_build_gbp_hdr in V2. For ip_tunnel_info_opts, it's
> confusing to me.
>
> It would be as below after constifying the parameter.
>
> static inline void *ip_tunnel_info_opts(const struct ip_tunnel_info *info)
> {
> return (void *)(info + 1);
> }
> Is there any value gained by this change?
You wouldn't need to W/A it each time in each driver, just do it once in
the inline itself.
I did it once in __skb_header_pointer()[0] to be able to pass data
pointer as const to optimize code a bit and point out explicitly that
the function doesn't modify the packet anyhow, don't see any reason to
not do the same here.
Or, as I said, you can use macros + __builtin_choose_expr() or _Generic.
container_of_const() uses the latter[1]. A __builtin_choose_expr()
variant could rely on the __same_type() macro to check whether the
pointer passed from the driver const or not.
>
>>
>>>>>> + }
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>> [...]
>>
>> Thanks,
>> Olek
[0]
https://elixir.bootlin.com/linux/v6.2-rc8/source/include/linux/skbuff.h#L3992
[1]
https://elixir.bootlin.com/linux/v6.2-rc8/source/include/linux/container_of.h#L33
Thanks,
Olek
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
2023-02-16 15:19 ` Alexander Lobakin
@ 2023-02-17 2:43 ` Gavin Li
0 siblings, 0 replies; 20+ messages in thread
From: Gavin Li @ 2023-02-17 2:43 UTC (permalink / raw)
To: Alexander Lobakin
Cc: davem, edumazet, kuba, pabeni, roopa, eng.alaamohamedsoliman.am,
bigeasy, netdev, linux-kernel, Roi Dayan, Maor Dickman,
Saeed Mahameed
On 2/16/2023 11:19 PM, Alexander Lobakin wrote:
> External email: Use caution opening links or attachments
>
>
> From: Gavin Li <gavinl@nvidia.com>
> Date: Thu, 16 Feb 2023 16:40:33 +0800
>
>> On 2/16/2023 1:01 AM, Alexander Lobakin wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> From: Gavin Li <gavinl@nvidia.com>
>>> Date: Wed, 15 Feb 2023 16:30:04 +0800
>>>
>>>> On 2/15/2023 11:36 AM, Gavin Li wrote:
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> On 2/14/2023 11:26 PM, Alexander Lobakin wrote:
>>>>>> External email: Use caution opening links or attachments
>>>>>>
>>>>>>
>>>>>> From: Gavin Li <gavinl@nvidia.com>
>>>>>> Date: Tue, 14 Feb 2023 15:41:37 +0200
>>> [...]
>>>
>>>>>>> @@ -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?
>>>>> ACK. Sorry for the confusion---I misunderstood the comment.
>>>> This ip_tunnel_info_opts is tricky to use const to annotate the arg
>>>> because it will have to cast from const to non-const again upon
>>>> returning.
>>> It's okay to cast away for the `void *` returned.
>>> Alternatively, use can convert it to a macro and use
>>> __builtin_choose_expr() or _Generic to return const or non-const
>>> depending on whether the argument is constant. That's what was recently
>>> done for container_of() IIRC.
>> I've fixed vxlan_build_gbp_hdr in V2. For ip_tunnel_info_opts, it's
>> confusing to me.
>>
>> It would be as below after constifying the parameter.
>>
>> static inline void *ip_tunnel_info_opts(const struct ip_tunnel_info *info)
>> {
>> return (void *)(info + 1);
>> }
>> Is there any value gained by this change?
> You wouldn't need to W/A it each time in each driver, just do it once in
> the inline itself.
> I did it once in __skb_header_pointer()[0] to be able to pass data
> pointer as const to optimize code a bit and point out explicitly that
> the function doesn't modify the packet anyhow, don't see any reason to
> not do the same here.
> Or, as I said, you can use macros + __builtin_choose_expr() or _Generic.
> container_of_const() uses the latter[1]. A __builtin_choose_expr()
> variant could rely on the __same_type() macro to check whether the
> pointer passed from the driver const or not.
ACK
>
>>>>>>> + }
>>>>>>> +
>>>>>>> + return 0;
>>>>>>> +}
>>> [...]
>>>
>>> Thanks,
>>> Olek
> [0]
> https://elixir.bootlin.com/linux/v6.2-rc8/source/include/linux/skbuff.h#L3992
> [1]
> https://elixir.bootlin.com/linux/v6.2-rc8/source/include/linux/container_of.h#L33
>
> Thanks,
> Olek
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next v1 2/3] net/mlx5e: Add helper for encap_info_equal for tunnels with options
2023-02-15 16:56 ` Alexander Lobakin
@ 2023-02-20 11:31 ` Gavin Li
0 siblings, 0 replies; 20+ messages in thread
From: Gavin Li @ 2023-02-20 11:31 UTC (permalink / raw)
To: Alexander Lobakin
Cc: davem, edumazet, kuba, pabeni, roopa, eng.alaamohamedsoliman.am,
bigeasy, netdev, linux-kernel, Roi Dayan, Maor Dickman,
Saeed Mahameed
On 2/16/2023 12:56 AM, Alexander Lobakin wrote:
> External email: Use caution opening links or attachments
>
>
> From: Gavin Li <gavinl@nvidia.com>
> Date: Wed, 15 Feb 2023 10:54:12 +0800
>
>> On 2/14/2023 11:01 PM, Alexander Lobakin wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> From: Gavin Li <gavinl@nvidia.com>
>>> Date: Tue, 14 Feb 2023 15:41:36 +0200
> [...]
>
>>>> + if (a_has_opts != b_has_opts)
>>>> + return false;
>>>> +
>>>> + /* options stored in memory next to ip_tunnel_info struct */
>>>> + a_info = container_of(a->ip_tun_key, struct ip_tunnel_info, key);
>>>> + b_info = container_of(b->ip_tun_key, struct ip_tunnel_info, key);
>>>> +
>>>> + return a_info->options_len == b_info->options_len &&
>>>> + memcmp(a_info + 1, b_info + 1, a_info->options_len) == 0;
>>> 1. memcmp() is not aligned to the first expr (off-by-one to the right).
>> Options start from "info + 1", see ip_tunnel_info_opts and will use it
>> here to replace the "info+1".
> Nah, I mean the following. Your code:
>
> return a_info->options_len == b_info->options_len &&
> memcmp(a_info + 1, b_info + 1, ...
>
> should be:
>
> return a_info->options_len == b_info->options_len &&
> memcmp(a_info + 1, b_info + 1, ...
>
> 7 spaces instead of a tab to have it aligned to the prev line.
ACK
>
>>> 2. `!expr` is preferred over `expr == 0`.
>> ACK
>>>> +}
>>>> +
>>>> static int cmp_decap_info(struct mlx5e_decap_key *a,
>>>> struct mlx5e_decap_key *b)
>>>> {
>>> [...]
>>>
>>> Thanks,
>>> Olek
> Thanks,
> Olek
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2023-02-20 11:32 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-14 13:41 [PATCH net-next v1 0/3] net/mlx5e: Add GBP VxLAN HW offload support Gavin Li
2023-02-14 13:41 ` [PATCH net-next v1 1/3] vxlan: Expose helper vxlan_build_gbp_hdr Gavin Li
2023-02-14 14:56 ` Alexander Lobakin
2023-02-15 8:26 ` Gavin Li
2023-02-14 13:41 ` [PATCH net-next v1 2/3] net/mlx5e: Add helper for encap_info_equal for tunnels with options Gavin Li
2023-02-14 15:01 ` Alexander Lobakin
2023-02-15 2:54 ` Gavin Li
2023-02-15 16:56 ` Alexander Lobakin
2023-02-20 11:31 ` Gavin Li
2023-02-14 13:41 ` [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload Gavin Li
2023-02-14 15:26 ` Alexander Lobakin
2023-02-15 2:50 ` Gavin Li
2023-02-15 16:53 ` Alexander Lobakin
2023-02-15 3:36 ` Gavin Li
2023-02-15 8:30 ` Gavin Li
2023-02-15 17:01 ` Alexander Lobakin
2023-02-16 8:40 ` Gavin Li
2023-02-16 15:19 ` Alexander Lobakin
2023-02-17 2:43 ` Gavin Li
2023-02-15 16:57 ` Alexander Lobakin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).