[PATCH RESEND net-next v4 0/4] net/mlx5e: Add GBP VxLAN HW offload support

[PATCH RESEND net-next v4 0/4] net/mlx5e: Add GBP VxLAN HW offload support

[Gavin Li]

Patch-1: Remove unused argument from functions.
Patch-2: Expose helper function vxlan_build_gbp_hdr.
Patch-3: Add helper function for encap_info_equal for tunnels with options.
Patch-4: Add HW offloading support for TC flows with VxLAN GBP encap/decap
        in mlx ethernet driver.

Gavin Li (4):
  vxlan: Remove unused argument from vxlan_build_gbp_hdr( ) and
    vxlan_build_gpe_hdr( )
---
changelog:
v2->v3
- Addressed comments from Paolo Abeni
- Add new patch
---
  vxlan: Expose helper vxlan_build_gbp_hdr
---
changelog:
v1->v2
- Addressed comments from Alexander Lobakin
- Use const to annotate read-only the pointer parameter
---
  net/mlx5e: Add helper for encap_info_equal for tunnels with options
---
changelog:
v3->v4
- Addressed comments from Alexander Lobakin
- Fix vertical alignment issue
v1->v2
- Addressed comments from Alexander Lobakin
- Replace confusing pointer arithmetic with function call
- Use boolean operator NOT to check if the function return value is not zero
---
  net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
---
changelog:
v3->v4
- Addressed comments from Simon Horman
- Using cast in place instead of changing API
v2->v3
- Addressed comments from Alexander Lobakin
- Remove the WA by casting away
v1->v2
- Addressed comments from Alexander Lobakin
- Add a separate pair of braces around bitops
- Remove the WA by casting away
- Fit all log messages into one line
- Use NL_SET_ERR_MSG_FMT_MOD to print the invalid value on error
---

 .../ethernet/mellanox/mlx5/core/en/tc_tun.h   |  3 +
 .../mellanox/mlx5/core/en/tc_tun_encap.c      | 32 ++++++++
 .../mellanox/mlx5/core/en/tc_tun_geneve.c     | 24 +-----
 .../mellanox/mlx5/core/en/tc_tun_vxlan.c      | 76 ++++++++++++++++++-
 drivers/net/vxlan/vxlan_core.c                | 27 +------
 include/linux/mlx5/device.h                   |  6 ++
 include/linux/mlx5/mlx5_ifc.h                 | 13 +++-
 include/net/vxlan.h                           | 19 +++++
 8 files changed, 149 insertions(+), 51 deletions(-)

-- 
2.31.1

[PATCH RESEND net-next v4 1/4] vxlan: Remove unused argument from vxlan_build_gbp_hdr( ) and vxlan_build_gpe_hdr( )

[Gavin Li]

Remove unused argument (i.e. u32 vxflags) in vxlan_build_gbp_hdr( ) and
vxlan_build_gpe_hdr( ) function arguments.

Signed-off-by: Gavin Li <gavinl@nvidia.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
---
 drivers/net/vxlan/vxlan_core.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index b1b179effe2a..86967277ab97 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -2140,8 +2140,7 @@ 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)
+static void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, struct vxlan_metadata *md)
 {
 	struct vxlanhdr_gbp *gbp;
 
@@ -2160,8 +2159,7 @@ static void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, u32 vxflags,
 	gbp->policy_id = htons(md->gbp & VXLAN_GBP_ID_MASK);
 }
 
-static int vxlan_build_gpe_hdr(struct vxlanhdr *vxh, u32 vxflags,
-			       __be16 protocol)
+static int vxlan_build_gpe_hdr(struct vxlanhdr *vxh, __be16 protocol)
 {
 	struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)vxh;
 
@@ -2224,9 +2222,9 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
 	}
 
 	if (vxflags & VXLAN_F_GBP)
-		vxlan_build_gbp_hdr(vxh, vxflags, md);
+		vxlan_build_gbp_hdr(vxh, md);
 	if (vxflags & VXLAN_F_GPE) {
-		err = vxlan_build_gpe_hdr(vxh, vxflags, skb->protocol);
+		err = vxlan_build_gpe_hdr(vxh, skb->protocol);
 		if (err < 0)
 			return err;
 		inner_protocol = skb->protocol;
-- 
2.31.1

[PATCH RESEND net-next v4 2/4] vxlan: Expose helper vxlan_build_gbp_hdr

[Gavin Li]

vxlan_build_gbp_hdr will be used by other modules to build gbp option in
vxlan header according to gbp flags.

Signed-off-by: Gavin Li <gavinl@nvidia.com>
Reviewed-by: Gavi Teitz <gavi@nvidia.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Maor Dickman <maord@nvidia.com>
Acked-by: Saeed Mahameed <saeedm@nvidia.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
---
 drivers/net/vxlan/vxlan_core.c | 19 -------------------
 include/net/vxlan.h            | 19 +++++++++++++++++++
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index 86967277ab97..13faab36b3e1 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -2140,25 +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, 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, __be16 protocol)
 {
 	struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)vxh;
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index bca5b01af247..b6d419fa7ab1 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -566,4 +566,23 @@ static inline bool vxlan_fdb_nh_path_select(struct nexthop *nh,
 	return true;
 }
 
+static inline void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, const 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

[PATCH RESEND net-next v4 3/4] net/mlx5e: Add helper for encap_info_equal for tunnels with options

[Gavin Li]

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.

Signed-off-by: Gavin Li <gavinl@nvidia.com>
Reviewed-by: Gavi Teitz <gavi@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      | 32 +++++++++++++++++++
 .../mellanox/mlx5/core/en/tc_tun_geneve.c     | 24 +-------------
 3 files changed, 36 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..a108e73c9f66 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
@@ -3,6 +3,7 @@
 
 #include <net/fib_notifier.h>
 #include <net/nexthop.h>
+#include <net/ip_tunnels.h>
 #include "tc_tun_encap.h"
 #include "en_tc.h"
 #include "tc_tun.h"
@@ -571,6 +572,37 @@ 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(ip_tunnel_info_opts(a_info),
+		       ip_tunnel_info_opts(b_info),
+		       a_info->options_len);
+}
+
 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

[PATCH RESEND net-next v4 4/4] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload

[Gavin Li]

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

Signed-off-by: Gavin Li <gavinl@nvidia.com>
Reviewed-by: Gavi Teitz <gavi@nvidia.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Maor Dickman <maord@nvidia.com>
Acked-by: Saeed Mahameed <saeedm@nvidia.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
---
 .../mellanox/mlx5/core/en/tc_tun_vxlan.c      | 76 ++++++++++++++++++-
 include/linux/mlx5/device.h                   |  6 ++
 include/linux/mlx5/mlx5_ifc.h                 | 13 +++-
 3 files changed, 91 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..8ee5d9e67e0a 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);
+	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,61 @@ 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, 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_FMT_MOD(extack, "Wrong VxLAN GBP mask(0x%08X)\n", *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 +180,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 +209,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 +232,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 5ad5126615a1..163cc0638c37 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -404,10 +404,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];
 };
@@ -895,7 +898,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

Re: [PATCH RESEND net-next v4 0/4] net/mlx5e: Add GBP VxLAN HW offload support

[Alexander Lobakin]

From: Gavin Li <gavinl@nvidia.com>
Date: Mon, 6 Mar 2023 05:02:58 +0200

> Patch-1: Remove unused argument from functions.
> Patch-2: Expose helper function vxlan_build_gbp_hdr.
> Patch-3: Add helper function for encap_info_equal for tunnels with options.
> Patch-4: Add HW offloading support for TC flows with VxLAN GBP encap/decap
>         in mlx ethernet driver.
> 
> Gavin Li (4):
>   vxlan: Remove unused argument from vxlan_build_gbp_hdr( ) and
>     vxlan_build_gpe_hdr( )
> ---
> changelog:
> v2->v3
> - Addressed comments from Paolo Abeni
> - Add new patch
> ---
>   vxlan: Expose helper vxlan_build_gbp_hdr
> ---
> changelog:
> v1->v2
> - Addressed comments from Alexander Lobakin
> - Use const to annotate read-only the pointer parameter
> ---
>   net/mlx5e: Add helper for encap_info_equal for tunnels with options
> ---
> changelog:
> v3->v4
> - Addressed comments from Alexander Lobakin
> - Fix vertical alignment issue
> v1->v2
> - Addressed comments from Alexander Lobakin
> - Replace confusing pointer arithmetic with function call
> - Use boolean operator NOT to check if the function return value is not zero
> ---
>   net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
> ---
> changelog:
> v3->v4
> - Addressed comments from Simon Horman
> - Using cast in place instead of changing API

I don't remember me acking this. The last thing I said is that in order
to avoid cast-aways you need to use _Generic(). 2 times. IIRC you said
"Ack" and that was the last message in that thread.
Now this. Without me in CCs, so I noticed it accidentally.
???

> v2->v3
> - Addressed comments from Alexander Lobakin
> - Remove the WA by casting away
> v1->v2
> - Addressed comments from Alexander Lobakin
> - Add a separate pair of braces around bitops
> - Remove the WA by casting away
> - Fit all log messages into one line
> - Use NL_SET_ERR_MSG_FMT_MOD to print the invalid value on error
> ---
> 
>  .../ethernet/mellanox/mlx5/core/en/tc_tun.h   |  3 +
>  .../mellanox/mlx5/core/en/tc_tun_encap.c      | 32 ++++++++
>  .../mellanox/mlx5/core/en/tc_tun_geneve.c     | 24 +-----
>  .../mellanox/mlx5/core/en/tc_tun_vxlan.c      | 76 ++++++++++++++++++-
>  drivers/net/vxlan/vxlan_core.c                | 27 +------
>  include/linux/mlx5/device.h                   |  6 ++
>  include/linux/mlx5/mlx5_ifc.h                 | 13 +++-
>  include/net/vxlan.h                           | 19 +++++
>  8 files changed, 149 insertions(+), 51 deletions(-)
> 

Thanks,
Olek

Re: [PATCH RESEND net-next v4 4/4] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload

[Simon Horman]

On Mon, Mar 06, 2023 at 05:03:02AM +0200, Gavin Li wrote:
> 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
> 
> Signed-off-by: Gavin Li <gavinl@nvidia.com>
> Reviewed-by: Gavi Teitz <gavi@nvidia.com>
> Reviewed-by: Roi Dayan <roid@nvidia.com>
> Reviewed-by: Maor Dickman <maord@nvidia.com>
> Acked-by: Saeed Mahameed <saeedm@nvidia.com>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>

FWIIW, I did provide a Reviewed-by tag for the other patches in this
series, but I don't believe I did for this patch..

Re: [PATCH RESEND net-next v4 0/4] net/mlx5e: Add GBP VxLAN HW offload support

[Gavin Li]

On 3/6/2023 10:47 PM, Alexander Lobakin wrote:
> External email: Use caution opening links or attachments
>
>
> From: Gavin Li <gavinl@nvidia.com>
> Date: Mon, 6 Mar 2023 05:02:58 +0200
>
>> Patch-1: Remove unused argument from functions.
>> Patch-2: Expose helper function vxlan_build_gbp_hdr.
>> Patch-3: Add helper function for encap_info_equal for tunnels with options.
>> Patch-4: Add HW offloading support for TC flows with VxLAN GBP encap/decap
>>          in mlx ethernet driver.
>>
>> Gavin Li (4):
>>    vxlan: Remove unused argument from vxlan_build_gbp_hdr( ) and
>>      vxlan_build_gpe_hdr( )
>> ---
>> changelog:
>> v2->v3
>> - Addressed comments from Paolo Abeni
>> - Add new patch
>> ---
>>    vxlan: Expose helper vxlan_build_gbp_hdr
>> ---
>> changelog:
>> v1->v2
>> - Addressed comments from Alexander Lobakin
>> - Use const to annotate read-only the pointer parameter
>> ---
>>    net/mlx5e: Add helper for encap_info_equal for tunnels with options
>> ---
>> changelog:
>> v3->v4
>> - Addressed comments from Alexander Lobakin
>> - Fix vertical alignment issue
>> v1->v2
>> - Addressed comments from Alexander Lobakin
>> - Replace confusing pointer arithmetic with function call
>> - Use boolean operator NOT to check if the function return value is not zero
>> ---
>>    net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
>> ---
>> changelog:
>> v3->v4
>> - Addressed comments from Simon Horman
>> - Using cast in place instead of changing API
> I don't remember me acking this. The last thing I said is that in order
> to avoid cast-aways you need to use _Generic(). 2 times. IIRC you said
> "Ack" and that was the last message in that thread.
> Now this. Without me in CCs, so I noticed it accidentally.
> ???

Not asked by you but you said you were OK if I used cast-aways. So I did the

change in V3 and reverted back to using cast-away in V4.

>> v2->v3
>> - Addressed comments from Alexander Lobakin
>> - Remove the WA by casting away
>> v1->v2
>> - Addressed comments from Alexander Lobakin
>> - Add a separate pair of braces around bitops
>> - Remove the WA by casting away
>> - Fit all log messages into one line
>> - Use NL_SET_ERR_MSG_FMT_MOD to print the invalid value on error
>> ---
>>
>>   .../ethernet/mellanox/mlx5/core/en/tc_tun.h   |  3 +
>>   .../mellanox/mlx5/core/en/tc_tun_encap.c      | 32 ++++++++
>>   .../mellanox/mlx5/core/en/tc_tun_geneve.c     | 24 +-----
>>   .../mellanox/mlx5/core/en/tc_tun_vxlan.c      | 76 ++++++++++++++++++-
>>   drivers/net/vxlan/vxlan_core.c                | 27 +------
>>   include/linux/mlx5/device.h                   |  6 ++
>>   include/linux/mlx5/mlx5_ifc.h                 | 13 +++-
>>   include/net/vxlan.h                           | 19 +++++
>>   8 files changed, 149 insertions(+), 51 deletions(-)
>>
> Thanks,
> Olek

Re: [PATCH RESEND net-next v4 4/4] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload

[Gavin Li]

On 3/6/2023 11:06 PM, Simon Horman wrote:
> External email: Use caution opening links or attachments
>
>
> On Mon, Mar 06, 2023 at 05:03:02AM +0200, Gavin Li wrote:
>> 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
>>
>> Signed-off-by: Gavin Li <gavinl@nvidia.com>
>> Reviewed-by: Gavi Teitz <gavi@nvidia.com>
>> Reviewed-by: Roi Dayan <roid@nvidia.com>
>> Reviewed-by: Maor Dickman <maord@nvidia.com>
>> Acked-by: Saeed Mahameed <saeedm@nvidia.com>
>> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> FWIIW, I did provide a Reviewed-by tag for the other patches in this
> series, but I don't believe I did for this patch..
ACK. Will remove in V5

Re: [PATCH RESEND net-next v4 0/4] net/mlx5e: Add GBP VxLAN HW offload support

[Alexander Lobakin]

From: Gavin Li <gavinl@nvidia.com>
Date: Tue, 7 Mar 2023 17:19:35 +0800

> 
> On 3/6/2023 10:47 PM, Alexander Lobakin wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> From: Gavin Li <gavinl@nvidia.com>
>> Date: Mon, 6 Mar 2023 05:02:58 +0200
>>
>>> Patch-1: Remove unused argument from functions.
>>> Patch-2: Expose helper function vxlan_build_gbp_hdr.
>>> Patch-3: Add helper function for encap_info_equal for tunnels with
>>> options.
>>> Patch-4: Add HW offloading support for TC flows with VxLAN GBP
>>> encap/decap
>>>          in mlx ethernet driver.
>>>
>>> Gavin Li (4):
>>>    vxlan: Remove unused argument from vxlan_build_gbp_hdr( ) and
>>>      vxlan_build_gpe_hdr( )
>>> ---
>>> changelog:
>>> v2->v3
>>> - Addressed comments from Paolo Abeni
>>> - Add new patch
>>> ---
>>>    vxlan: Expose helper vxlan_build_gbp_hdr
>>> ---
>>> changelog:
>>> v1->v2
>>> - Addressed comments from Alexander Lobakin
>>> - Use const to annotate read-only the pointer parameter
>>> ---
>>>    net/mlx5e: Add helper for encap_info_equal for tunnels with options
>>> ---
>>> changelog:
>>> v3->v4
>>> - Addressed comments from Alexander Lobakin
>>> - Fix vertical alignment issue
>>> v1->v2
>>> - Addressed comments from Alexander Lobakin
>>> - Replace confusing pointer arithmetic with function call
>>> - Use boolean operator NOT to check if the function return value is
>>> not zero
>>> ---
>>>    net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
>>> ---
>>> changelog:
>>> v3->v4
>>> - Addressed comments from Simon Horman
>>> - Using cast in place instead of changing API
>> I don't remember me acking this. The last thing I said is that in order
>> to avoid cast-aways you need to use _Generic(). 2 times. IIRC you said
>> "Ack" and that was the last message in that thread.
>> Now this. Without me in CCs, so I noticed it accidentally.
>> ???
> 
> Not asked by you but you said you were OK if I used cast-aways. So I did
> the
> 
> change in V3 and reverted back to using cast-away in V4.

My last reply was[0]:

"
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.

[...]

[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
"

Where did I say here I'm fine with W/As in the drivers? I mentioned two
options: cast-away in THE GENERIC INLINE, not the driver, or, more
preferred, following the way of container_of_const().
Then your reply[1]:

"ACK"

What did you ack then if you picked neither of those 2 options?

> 
>>> v2->v3
>>> - Addressed comments from Alexander Lobakin
>>> - Remove the WA by casting away
>>> v1->v2
>>> - Addressed comments from Alexander Lobakin
>>> - Add a separate pair of braces around bitops
>>> - Remove the WA by casting away
>>> - Fit all log messages into one line
>>> - Use NL_SET_ERR_MSG_FMT_MOD to print the invalid value on error
>>> ---
>>>
>>>   .../ethernet/mellanox/mlx5/core/en/tc_tun.h   |  3 +
>>>   .../mellanox/mlx5/core/en/tc_tun_encap.c      | 32 ++++++++
>>>   .../mellanox/mlx5/core/en/tc_tun_geneve.c     | 24 +-----
>>>   .../mellanox/mlx5/core/en/tc_tun_vxlan.c      | 76 ++++++++++++++++++-
>>>   drivers/net/vxlan/vxlan_core.c                | 27 +------
>>>   include/linux/mlx5/device.h                   |  6 ++
>>>   include/linux/mlx5/mlx5_ifc.h                 | 13 +++-
>>>   include/net/vxlan.h                           | 19 +++++
>>>   8 files changed, 149 insertions(+), 51 deletions(-)
>>>
>> Thanks,
>> Olek

[0]
https://lore.kernel.org/netdev/aefe00f0-2a15-9a43-2451-6d01e74cc48a@intel.com
[1]
https://lore.kernel.org/netdev/ca729a48-35a1-ef05-59d3-ef1539003051@nvidia.com

Thanks,
Olek

Re: [PATCH RESEND net-next v4 0/4] net/mlx5e: Add GBP VxLAN HW offload support

[Gavin Li]

On 3/8/2023 12:58 AM, Alexander Lobakin wrote:
> External email: Use caution opening links or attachments
>
>
> From: Gavin Li <gavinl@nvidia.com>
> Date: Tue, 7 Mar 2023 17:19:35 +0800
>
>> On 3/6/2023 10:47 PM, Alexander Lobakin wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> From: Gavin Li <gavinl@nvidia.com>
>>> Date: Mon, 6 Mar 2023 05:02:58 +0200
>>>
>>>> Patch-1: Remove unused argument from functions.
>>>> Patch-2: Expose helper function vxlan_build_gbp_hdr.
>>>> Patch-3: Add helper function for encap_info_equal for tunnels with
>>>> options.
>>>> Patch-4: Add HW offloading support for TC flows with VxLAN GBP
>>>> encap/decap
>>>>           in mlx ethernet driver.
>>>>
>>>> Gavin Li (4):
>>>>     vxlan: Remove unused argument from vxlan_build_gbp_hdr( ) and
>>>>       vxlan_build_gpe_hdr( )
>>>> ---
>>>> changelog:
>>>> v2->v3
>>>> - Addressed comments from Paolo Abeni
>>>> - Add new patch
>>>> ---
>>>>     vxlan: Expose helper vxlan_build_gbp_hdr
>>>> ---
>>>> changelog:
>>>> v1->v2
>>>> - Addressed comments from Alexander Lobakin
>>>> - Use const to annotate read-only the pointer parameter
>>>> ---
>>>>     net/mlx5e: Add helper for encap_info_equal for tunnels with options
>>>> ---
>>>> changelog:
>>>> v3->v4
>>>> - Addressed comments from Alexander Lobakin
>>>> - Fix vertical alignment issue
>>>> v1->v2
>>>> - Addressed comments from Alexander Lobakin
>>>> - Replace confusing pointer arithmetic with function call
>>>> - Use boolean operator NOT to check if the function return value is
>>>> not zero
>>>> ---
>>>>     net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
>>>> ---
>>>> changelog:
>>>> v3->v4
>>>> - Addressed comments from Simon Horman
>>>> - Using cast in place instead of changing API
>>> I don't remember me acking this. The last thing I said is that in order
>>> to avoid cast-aways you need to use _Generic(). 2 times. IIRC you said
>>> "Ack" and that was the last message in that thread.
>>> Now this. Without me in CCs, so I noticed it accidentally.
>>> ???
>> Not asked by you but you said you were OK if I used cast-aways. So I did
>> the
>>
>> change in V3 and reverted back to using cast-away in V4.
> My last reply was[0]:
>
> "
> 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.
>
> [...]
>
> [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
> "
>
> Where did I say here I'm fine with W/As in the drivers? I mentioned two
> options: cast-away in THE GENERIC INLINE, not the driver, or, more
> preferred, following the way of container_of_const().
> Then your reply[1]:
>
> "ACK"
>
> What did you ack then if you picked neither of those 2 options?

I had fixed it with "cast-away in THE GENERIC INLINE" in V3 and got 
comments and concern

from Simon Horman. So, it was reverted.

"But I really do wonder if this patch masks rather than fixes the 
problem."----From Simon.

I thought you were OK to revert the changes based on the reply.

 From my understanding, the function always return a non-const pointer 
regardless the type of the

  input one, which is slightly different from your examples.

Any comments, Simon?

If both or you are OK with option #1, I'll follow.

>>>> v2->v3
>>>> - Addressed comments from Alexander Lobakin
>>>> - Remove the WA by casting away
>>>> v1->v2
>>>> - Addressed comments from Alexander Lobakin
>>>> - Add a separate pair of braces around bitops
>>>> - Remove the WA by casting away
>>>> - Fit all log messages into one line
>>>> - Use NL_SET_ERR_MSG_FMT_MOD to print the invalid value on error
>>>> ---
>>>>
>>>>    .../ethernet/mellanox/mlx5/core/en/tc_tun.h   |  3 +
>>>>    .../mellanox/mlx5/core/en/tc_tun_encap.c      | 32 ++++++++
>>>>    .../mellanox/mlx5/core/en/tc_tun_geneve.c     | 24 +-----
>>>>    .../mellanox/mlx5/core/en/tc_tun_vxlan.c      | 76 ++++++++++++++++++-
>>>>    drivers/net/vxlan/vxlan_core.c                | 27 +------
>>>>    include/linux/mlx5/device.h                   |  6 ++
>>>>    include/linux/mlx5/mlx5_ifc.h                 | 13 +++-
>>>>    include/net/vxlan.h                           | 19 +++++
>>>>    8 files changed, 149 insertions(+), 51 deletions(-)
>>>>
>>> Thanks,
>>> Olek
> [0]
> https://lore.kernel.org/netdev/aefe00f0-2a15-9a43-2451-6d01e74cc48a@intel.com
> [1]
> https://lore.kernel.org/netdev/ca729a48-35a1-ef05-59d3-ef1539003051@nvidia.com
>
> Thanks,
> Olek

Re: [PATCH RESEND net-next v4 0/4] net/mlx5e: Add GBP VxLAN HW offload support

[Alexander Lobakin]

From: Gavin Li <gavinl@nvidia.com>
Date: Wed, 8 Mar 2023 10:22:36 +0800

> 
> On 3/8/2023 12:58 AM, Alexander Lobakin wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> From: Gavin Li <gavinl@nvidia.com>
>> Date: Tue, 7 Mar 2023 17:19:35 +0800
>>
>>> On 3/6/2023 10:47 PM, Alexander Lobakin wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> From: Gavin Li <gavinl@nvidia.com>
>>>> Date: Mon, 6 Mar 2023 05:02:58 +0200
>>>>
>>>>> Patch-1: Remove unused argument from functions.
>>>>> Patch-2: Expose helper function vxlan_build_gbp_hdr.
>>>>> Patch-3: Add helper function for encap_info_equal for tunnels with
>>>>> options.
>>>>> Patch-4: Add HW offloading support for TC flows with VxLAN GBP
>>>>> encap/decap
>>>>>           in mlx ethernet driver.
>>>>>
>>>>> Gavin Li (4):
>>>>>     vxlan: Remove unused argument from vxlan_build_gbp_hdr( ) and
>>>>>       vxlan_build_gpe_hdr( )
>>>>> ---
>>>>> changelog:
>>>>> v2->v3
>>>>> - Addressed comments from Paolo Abeni
>>>>> - Add new patch
>>>>> ---
>>>>>     vxlan: Expose helper vxlan_build_gbp_hdr
>>>>> ---
>>>>> changelog:
>>>>> v1->v2
>>>>> - Addressed comments from Alexander Lobakin
>>>>> - Use const to annotate read-only the pointer parameter
>>>>> ---
>>>>>     net/mlx5e: Add helper for encap_info_equal for tunnels with
>>>>> options
>>>>> ---
>>>>> changelog:
>>>>> v3->v4
>>>>> - Addressed comments from Alexander Lobakin
>>>>> - Fix vertical alignment issue
>>>>> v1->v2
>>>>> - Addressed comments from Alexander Lobakin
>>>>> - Replace confusing pointer arithmetic with function call
>>>>> - Use boolean operator NOT to check if the function return value is
>>>>> not zero
>>>>> ---
>>>>>     net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
>>>>> ---
>>>>> changelog:
>>>>> v3->v4
>>>>> - Addressed comments from Simon Horman
>>>>> - Using cast in place instead of changing API
>>>> I don't remember me acking this. The last thing I said is that in order
>>>> to avoid cast-aways you need to use _Generic(). 2 times. IIRC you said
>>>> "Ack" and that was the last message in that thread.
>>>> Now this. Without me in CCs, so I noticed it accidentally.
>>>> ???
>>> Not asked by you but you said you were OK if I used cast-aways. So I did
>>> the
>>>
>>> change in V3 and reverted back to using cast-away in V4.
>> My last reply was[0]:
>>
>> "
>> 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.
>>
>> [...]
>>
>> [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
>> "
>>
>> Where did I say here I'm fine with W/As in the drivers? I mentioned two
>> options: cast-away in THE GENERIC INLINE, not the driver, or, more
>> preferred, following the way of container_of_const().
>> Then your reply[1]:
>>
>> "ACK"
>>
>> What did you ack then if you picked neither of those 2 options?
> 
> I had fixed it with "cast-away in THE GENERIC INLINE" in V3 and got
> comments and concern
> 
> from Simon Horman. So, it was reverted.
> 
> "But I really do wonder if this patch masks rather than fixes the
> problem."----From Simon.
> 
> I thought you were OK to revert the changes based on the reply.

No I wasn't.
Yes, it masks, because you need to return either const or non-const
depending on the input pointer qualifier. container_of_const(), telling
this 4th time.

> 
> From my understanding, the function always return a non-const pointer
> regardless the type of the
> 
>  input one, which is slightly different from your examples.

See above.

> 
> Any comments, Simon?
> 
> If both or you are OK with option #1, I'll follow.
> 
>>>>> v2->v3
>>>>> - Addressed comments from Alexander Lobakin
>>>>> - Remove the WA by casting away
>>>>> v1->v2
>>>>> - Addressed comments from Alexander Lobakin
>>>>> - Add a separate pair of braces around bitops
>>>>> - Remove the WA by casting away
>>>>> - Fit all log messages into one line
>>>>> - Use NL_SET_ERR_MSG_FMT_MOD to print the invalid value on error
>>>>> ---
>>>>>
>>>>>    .../ethernet/mellanox/mlx5/core/en/tc_tun.h   |  3 +
>>>>>    .../mellanox/mlx5/core/en/tc_tun_encap.c      | 32 ++++++++
>>>>>    .../mellanox/mlx5/core/en/tc_tun_geneve.c     | 24 +-----
>>>>>    .../mellanox/mlx5/core/en/tc_tun_vxlan.c      | 76
>>>>> ++++++++++++++++++-
>>>>>    drivers/net/vxlan/vxlan_core.c                | 27 +------
>>>>>    include/linux/mlx5/device.h                   |  6 ++
>>>>>    include/linux/mlx5/mlx5_ifc.h                 | 13 +++-
>>>>>    include/net/vxlan.h                           | 19 +++++
>>>>>    8 files changed, 149 insertions(+), 51 deletions(-)
>>>>>
>>>> Thanks,
>>>> Olek
>> [0]
>> https://lore.kernel.org/netdev/aefe00f0-2a15-9a43-2451-6d01e74cc48a@intel.com
>> [1]
>> https://lore.kernel.org/netdev/ca729a48-35a1-ef05-59d3-ef1539003051@nvidia.com
>>
>> Thanks,
>> Olek

Thanks,
Olek

Re: [PATCH RESEND net-next v4 0/4] net/mlx5e: Add GBP VxLAN HW offload support

[Simon Horman]

On Wed, Mar 08, 2023 at 02:34:28PM +0100, Alexander Lobakin wrote:
> From: Gavin Li <gavinl@nvidia.com>
> Date: Wed, 8 Mar 2023 10:22:36 +0800
> 
> > 
> > On 3/8/2023 12:58 AM, Alexander Lobakin wrote:
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> From: Gavin Li <gavinl@nvidia.com>
> >> Date: Tue, 7 Mar 2023 17:19:35 +0800
> >>
> >>> On 3/6/2023 10:47 PM, Alexander Lobakin wrote:
> >>>> External email: Use caution opening links or attachments
> >>>>
> >>>>
> >>>> From: Gavin Li <gavinl@nvidia.com>
> >>>> Date: Mon, 6 Mar 2023 05:02:58 +0200
> >>>>
> >>>>> Patch-1: Remove unused argument from functions.
> >>>>> Patch-2: Expose helper function vxlan_build_gbp_hdr.
> >>>>> Patch-3: Add helper function for encap_info_equal for tunnels with
> >>>>> options.
> >>>>> Patch-4: Add HW offloading support for TC flows with VxLAN GBP
> >>>>> encap/decap
> >>>>>           in mlx ethernet driver.
> >>>>>
> >>>>> Gavin Li (4):
> >>>>>     vxlan: Remove unused argument from vxlan_build_gbp_hdr( ) and
> >>>>>       vxlan_build_gpe_hdr( )
> >>>>> ---
> >>>>> changelog:
> >>>>> v2->v3
> >>>>> - Addressed comments from Paolo Abeni
> >>>>> - Add new patch
> >>>>> ---
> >>>>>     vxlan: Expose helper vxlan_build_gbp_hdr
> >>>>> ---
> >>>>> changelog:
> >>>>> v1->v2
> >>>>> - Addressed comments from Alexander Lobakin
> >>>>> - Use const to annotate read-only the pointer parameter
> >>>>> ---
> >>>>>     net/mlx5e: Add helper for encap_info_equal for tunnels with
> >>>>> options
> >>>>> ---
> >>>>> changelog:
> >>>>> v3->v4
> >>>>> - Addressed comments from Alexander Lobakin
> >>>>> - Fix vertical alignment issue
> >>>>> v1->v2
> >>>>> - Addressed comments from Alexander Lobakin
> >>>>> - Replace confusing pointer arithmetic with function call
> >>>>> - Use boolean operator NOT to check if the function return value is
> >>>>> not zero
> >>>>> ---
> >>>>>     net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
> >>>>> ---
> >>>>> changelog:
> >>>>> v3->v4
> >>>>> - Addressed comments from Simon Horman
> >>>>> - Using cast in place instead of changing API
> >>>> I don't remember me acking this. The last thing I said is that in order
> >>>> to avoid cast-aways you need to use _Generic(). 2 times. IIRC you said
> >>>> "Ack" and that was the last message in that thread.
> >>>> Now this. Without me in CCs, so I noticed it accidentally.
> >>>> ???
> >>> Not asked by you but you said you were OK if I used cast-aways. So I did
> >>> the
> >>>
> >>> change in V3 and reverted back to using cast-away in V4.
> >> My last reply was[0]:
> >>
> >> "
> >> 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.
> >>
> >> [...]
> >>
> >> [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
> >> "
> >>
> >> Where did I say here I'm fine with W/As in the drivers? I mentioned two
> >> options: cast-away in THE GENERIC INLINE, not the driver, or, more
> >> preferred, following the way of container_of_const().
> >> Then your reply[1]:
> >>
> >> "ACK"
> >>
> >> What did you ack then if you picked neither of those 2 options?
> > 
> > I had fixed it with "cast-away in THE GENERIC INLINE" in V3 and got
> > comments and concern
> > 
> > from Simon Horman. So, it was reverted.
> > 
> > "But I really do wonder if this patch masks rather than fixes the
> > problem."----From Simon.
> > 
> > I thought you were OK to revert the changes based on the reply.
> 
> No I wasn't.
> Yes, it masks, because you need to return either const or non-const
> depending on the input pointer qualifier. container_of_const(), telling
> this 4th time.
> 
> > 
> > From my understanding, the function always return a non-const pointer
> > regardless the type of the
> > 
> >  input one, which is slightly different from your examples.
> 
> See above.
> 
> > 
> > Any comments, Simon?
> > 
> > If both or you are OK with option #1, I'll follow.

I'd like suggest moving on from the who said what aspect of this conversation.
Clearly there has been some misunderstanding. Let's move on.

Regarding the more technical topic of constness.
Unless I am mistaken function in question looks like this:

static inline void *ip_tunnel_info_opts(const struct ip_tunnel_info *info)
{
     return info + 1;
}

My view is that if the input is const, the output should be const;
conversely, if the output is non-const then the input should be non-const.

It does seem to me that container_of_const has this property.
And from that perspective may be the basis of a good solution.

This is my opinion. I do understand that others may have different opinions.

Re: [PATCH RESEND net-next v4 0/4] net/mlx5e: Add GBP VxLAN HW offload support

[Gavin Li]

On 3/9/2023 4:13 AM, Simon Horman wrote:
> External email: Use caution opening links or attachments
>
>
> On Wed, Mar 08, 2023 at 02:34:28PM +0100, Alexander Lobakin wrote:
>> From: Gavin Li <gavinl@nvidia.com>
>> Date: Wed, 8 Mar 2023 10:22:36 +0800
>>
>>> On 3/8/2023 12:58 AM, Alexander Lobakin wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> From: Gavin Li <gavinl@nvidia.com>
>>>> Date: Tue, 7 Mar 2023 17:19:35 +0800
>>>>
>>>>> On 3/6/2023 10:47 PM, Alexander Lobakin wrote:
>>>>>> External email: Use caution opening links or attachments
>>>>>>
>>>>>>
>>>>>> From: Gavin Li <gavinl@nvidia.com>
>>>>>> Date: Mon, 6 Mar 2023 05:02:58 +0200
>>>>>>
>>>>>>> Patch-1: Remove unused argument from functions.
>>>>>>> Patch-2: Expose helper function vxlan_build_gbp_hdr.
>>>>>>> Patch-3: Add helper function for encap_info_equal for tunnels with
>>>>>>> options.
>>>>>>> Patch-4: Add HW offloading support for TC flows with VxLAN GBP
>>>>>>> encap/decap
>>>>>>>            in mlx ethernet driver.
>>>>>>>
>>>>>>> Gavin Li (4):
>>>>>>>      vxlan: Remove unused argument from vxlan_build_gbp_hdr( ) and
>>>>>>>        vxlan_build_gpe_hdr( )
>>>>>>> ---
>>>>>>> changelog:
>>>>>>> v2->v3
>>>>>>> - Addressed comments from Paolo Abeni
>>>>>>> - Add new patch
>>>>>>> ---
>>>>>>>      vxlan: Expose helper vxlan_build_gbp_hdr
>>>>>>> ---
>>>>>>> changelog:
>>>>>>> v1->v2
>>>>>>> - Addressed comments from Alexander Lobakin
>>>>>>> - Use const to annotate read-only the pointer parameter
>>>>>>> ---
>>>>>>>      net/mlx5e: Add helper for encap_info_equal for tunnels with
>>>>>>> options
>>>>>>> ---
>>>>>>> changelog:
>>>>>>> v3->v4
>>>>>>> - Addressed comments from Alexander Lobakin
>>>>>>> - Fix vertical alignment issue
>>>>>>> v1->v2
>>>>>>> - Addressed comments from Alexander Lobakin
>>>>>>> - Replace confusing pointer arithmetic with function call
>>>>>>> - Use boolean operator NOT to check if the function return value is
>>>>>>> not zero
>>>>>>> ---
>>>>>>>      net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
>>>>>>> ---
>>>>>>> changelog:
>>>>>>> v3->v4
>>>>>>> - Addressed comments from Simon Horman
>>>>>>> - Using cast in place instead of changing API
>>>>>> I don't remember me acking this. The last thing I said is that in order
>>>>>> to avoid cast-aways you need to use _Generic(). 2 times. IIRC you said
>>>>>> "Ack" and that was the last message in that thread.
>>>>>> Now this. Without me in CCs, so I noticed it accidentally.
>>>>>> ???
>>>>> Not asked by you but you said you were OK if I used cast-aways. So I did
>>>>> the
>>>>>
>>>>> change in V3 and reverted back to using cast-away in V4.
>>>> My last reply was[0]:
>>>>
>>>> "
>>>> 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.
>>>>
>>>> [...]
>>>>
>>>> [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
>>>> "
>>>>
>>>> Where did I say here I'm fine with W/As in the drivers? I mentioned two
>>>> options: cast-away in THE GENERIC INLINE, not the driver, or, more
>>>> preferred, following the way of container_of_const().
>>>> Then your reply[1]:
>>>>
>>>> "ACK"
>>>>
>>>> What did you ack then if you picked neither of those 2 options?
>>> I had fixed it with "cast-away in THE GENERIC INLINE" in V3 and got
>>> comments and concern
>>>
>>> from Simon Horman. So, it was reverted.
>>>
>>> "But I really do wonder if this patch masks rather than fixes the
>>> problem."----From Simon.
>>>
>>> I thought you were OK to revert the changes based on the reply.
>> No I wasn't.
>> Yes, it masks, because you need to return either const or non-const
>> depending on the input pointer qualifier. container_of_const(), telling
>> this 4th time.
>>
>>>  From my understanding, the function always return a non-const pointer
>>> regardless the type of the
>>>
>>>   input one, which is slightly different from your examples.
>> See above.
>>
>>> Any comments, Simon?
>>>
>>> If both or you are OK with option #1, I'll follow.
> I'd like suggest moving on from the who said what aspect of this conversation.
> Clearly there has been some misunderstanding. Let's move on.
>
> Regarding the more technical topic of constness.
> Unless I am mistaken function in question looks like this:
>
> static inline void *ip_tunnel_info_opts(const struct ip_tunnel_info *info)
> {
>       return info + 1;
> }
>
> My view is that if the input is const, the output should be const;
> conversely, if the output is non-const then the input should be non-const.
>
> It does seem to me that container_of_const has this property.
> And from that perspective may be the basis of a good solution.
>
> This is my opinion. I do understand that others may have different opinions.
ACK

Re: [PATCH RESEND net-next v4 0/4] net/mlx5e: Add GBP VxLAN HW offload support

[Alexander Lobakin]

From: Simon.horman@corigine.com <simon.horman@corigine.com>
Date: Wed, 8 Mar 2023 21:13:31 +0100

> On Wed, Mar 08, 2023 at 02:34:28PM +0100, Alexander Lobakin wrote:
>> From: Gavin Li <gavinl@nvidia.com>
>> Date: Wed, 8 Mar 2023 10:22:36 +0800

[...]

>>> Any comments, Simon?
>>>
>>> If both or you are OK with option #1, I'll follow.
> 
> I'd like suggest moving on from the who said what aspect of this conversation.
> Clearly there has been some misunderstanding. Let's move on.
> 
> Regarding the more technical topic of constness.
> Unless I am mistaken function in question looks like this:
> 
> static inline void *ip_tunnel_info_opts(const struct ip_tunnel_info *info)
> {
>      return info + 1;
> }
> 
> My view is that if the input is const, the output should be const;
> conversely, if the output is non-const then the input should be non-const.
> 
> It does seem to me that container_of_const has this property.
> And from that perspective may be the basis of a good solution.
> 
> This is my opinion. I do understand that others may have different opinions.

Mine is basically the very same :)

Thanks,
Olek