[PATCH net-next v2 0/5] net/mlx5e: Make little improvement for mlx5e

[PATCH net-next v2 0/5] net/mlx5e: Make little improvement for mlx5e

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

This serial patches are not bugfixes, and just little improvement for mlx5e.

v1->v2
Patch 1: remove the duplicated error messages and stick to extack only
Patch 2: use the 'unknown' instead of empty string
Patch 5: is new patch

Tonghao Zhang (5):
  net/mlx5e: Return -EOPNOTSUPP when modify header action zero
  net/mlx5e: Make the log friendly when decapsulation offload not
    supported
  net/mlx5e: Remove 'parse_attr' argument in parse_tc_fdb_actions()
  net/mlx5e: Deletes unnecessary setting of esw_attr->parse_attr
  net/mlx5e: Return -EOPNOTSUPP when attempting to offload an
    unsupported action

 .../net/ethernet/mellanox/mlx5/core/en/tc_tun.c    |  8 +++++---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c    | 23 +++++++++++++++-------
 2 files changed, 21 insertions(+), 10 deletions(-)

-- 
1.8.3.1

[PATCH net-next v2 1/5] net/mlx5e: Return -EOPNOTSUPP when modify header action zero

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

When max modify header action is zero, we return -EOPNOTSUPP
directly. In this way, we can ignore wrong message info (e.g.
"mlx5: parsed 0 pedit actions, can't do more").

This happens when offloading pedit actions on mlx VFs.

For example:
$ tc filter add dev mlx5_vf parent ffff: protocol ip prio 1 \
	flower skip_sw dst_mac 00:10:56:fb:64:e8 \
	dst_ip 1.1.1.100 src_ip 1.1.1.200 \
	action pedit ex munge eth src set 00:10:56:b4:5d:20

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index b38986e..708f819 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2002,7 +2002,8 @@ static int offload_pedit_fields(struct pedit_headers_action *hdrs,
 static int alloc_mod_hdr_actions(struct mlx5e_priv *priv,
 				 struct pedit_headers_action *hdrs,
 				 int namespace,
-				 struct mlx5e_tc_flow_parse_attr *parse_attr)
+				 struct mlx5e_tc_flow_parse_attr *parse_attr,
+				 struct netlink_ext_ack *extack)
 {
 	int nkeys, action_size, max_actions;
 
@@ -2015,6 +2016,12 @@ static int alloc_mod_hdr_actions(struct mlx5e_priv *priv,
 	else /* namespace is MLX5_FLOW_NAMESPACE_KERNEL - NIC offloading */
 		max_actions = MLX5_CAP_FLOWTABLE_NIC_RX(priv->mdev, max_modify_header_actions);
 
+	if (!max_actions) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "don't support pedit actions, can't offload");
+		return -EOPNOTSUPP;
+	}
+
 	/* can get up to crazingly 16 HW actions in 32 bits pedit SW key */
 	max_actions = min(max_actions, nkeys * 16);
 
@@ -2072,7 +2079,8 @@ static int alloc_tc_pedit_action(struct mlx5e_priv *priv, int namespace,
 	u8 cmd;
 
 	if (!parse_attr->mod_hdr_actions) {
-		err = alloc_mod_hdr_actions(priv, hdrs, namespace, parse_attr);
+		err = alloc_mod_hdr_actions(priv, hdrs,
+					    namespace, parse_attr, extack);
 		if (err)
 			goto out_err;
 	}
-- 
1.8.3.1

[PATCH net-next v2 2/5] net/mlx5e: Make the log friendly when decapsulation offload not supported

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

If we try to offload decapsulation actions to VFs hw, we get the log [1].
It's not friendly, because the kind of net device is null, and we don't
know what '0' means.

[1] "mlx5_core 0000:05:01.2 vf_0: decapsulation offload is not supported for  net device (0)"

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
index bdcc5e7..6cbfbfa 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
@@ -84,7 +84,7 @@ static const char *mlx5e_netdev_kind(struct net_device *dev)
 	if (dev->rtnl_link_ops)
 		return dev->rtnl_link_ops->kind;
 	else
-		return "";
+		return "unknown";
 }
 
 static int mlx5e_route_lookup_ipv6(struct mlx5e_priv *priv,
@@ -620,8 +620,10 @@ int mlx5e_tc_tun_parse(struct net_device *filter_dev,
 						headers_c, headers_v);
 	} else {
 		netdev_warn(priv->netdev,
-			    "decapsulation offload is not supported for %s net device (%d)\n",
-			    mlx5e_netdev_kind(filter_dev), tunnel_type);
+			    "decapsulation offload is not supported for %s (kind: \"%s\")\n",
+			    netdev_name(filter_dev),
+			    mlx5e_netdev_kind(filter_dev));
+
 		return -EOPNOTSUPP;
 	}
 	return err;
-- 
1.8.3.1

[PATCH net-next v2 3/5] net/mlx5e: Remove 'parse_attr' argument in parse_tc_fdb_actions()

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

This patch is a little improvement. Simplify the parse_tc_fdb_actions().

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 708f819..e6583b9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2475,13 +2475,13 @@ static int parse_tc_vlan_action(struct mlx5e_priv *priv,
 
 static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
 				struct flow_action *flow_action,
-				struct mlx5e_tc_flow_parse_attr *parse_attr,
 				struct mlx5e_tc_flow *flow,
 				struct netlink_ext_ack *extack)
 {
 	struct pedit_headers_action hdrs[2] = {};
 	struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
 	struct mlx5_esw_flow_attr *attr = flow->esw_attr;
+	struct mlx5e_tc_flow_parse_attr *parse_attr = attr->parse_attr;
 	struct mlx5e_rep_priv *rpriv = priv->ppriv;
 	const struct ip_tunnel_info *info = NULL;
 	const struct flow_action_entry *act;
@@ -2796,7 +2796,7 @@ static bool is_peer_flow_needed(struct mlx5e_tc_flow *flow)
 	if (err)
 		goto err_free;
 
-	err = parse_tc_fdb_actions(priv, &rule->action, parse_attr, flow, extack);
+	err = parse_tc_fdb_actions(priv, &rule->action, flow, extack);
 	if (err)
 		goto err_free;
 
-- 
1.8.3.1

[PATCH net-next v2 4/5] net/mlx5e: Deletes unnecessary setting of esw_attr->parse_attr

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

This patch deletes unnecessary setting of the esw_attr->parse_attr
to parse_attr in parse_tc_fdb_actions() because it is already done
by the mlx5e_flow_esw_attr_init() function.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index e6583b9..d9fcb14 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2566,7 +2566,6 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
 					out_dev->ifindex;
 				parse_attr->tun_info[attr->out_count] = *info;
 				encap = false;
-				attr->parse_attr = parse_attr;
 				attr->dests[attr->out_count].flags |=
 					MLX5_ESW_DEST_ENCAP;
 				attr->out_count++;
-- 
1.8.3.1

[PATCH net-next v2 5/5] net/mlx5e: Return -EOPNOTSUPP when attempting to offload an unsupported action

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

The encapsulation is not supported for mlx5 VFs. When we try to
offload that action, the -EINVAL is returned, but not -EOPNOTSUPP.
This patch changes the returned value and ignore to confuse user.

For example: (p2p1_0 is VF net device)
tc filter add dev p2p1_0 protocol ip  parent ffff: prio 1 flower skip_sw \
	src_mac e4:11:22:33:44:01	\
	action tunnel_key set		\
	src_ip 1.1.1.100		\
	dst_ip 1.1.1.200		\
	dst_port 4789 id 100		\
	action mirred egress redirect dev vxlan0

"RTNETLINK answers: Invalid argument"

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index d9fcb14..f5029ea 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2302,7 +2302,8 @@ static int parse_tc_nic_actions(struct mlx5e_priv *priv,
 			}
 			break;
 		default:
-			return -EINVAL;
+			NL_SET_ERR_MSG_MOD(extack, "The offload action is not supported");
+			return -EOPNOTSUPP;
 		}
 	}
 
@@ -2624,7 +2625,8 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
 			break;
 			}
 		default:
-			return -EINVAL;
+			NL_SET_ERR_MSG_MOD(extack, "The offload action is not supported");
+			return -EOPNOTSUPP;
 		}
 	}
 
-- 
1.8.3.1

Re: [PATCH net-next v2 3/5] net/mlx5e: Remove 'parse_attr' argument in parse_tc_fdb_actions()

[Roi Dayan]

On 25/02/2019 12:40, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> This patch is a little improvement. Simplify the parse_tc_fdb_actions().
> 
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index 708f819..e6583b9 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -2475,13 +2475,13 @@ static int parse_tc_vlan_action(struct mlx5e_priv *priv,
>  
>  static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
>  				struct flow_action *flow_action,
> -				struct mlx5e_tc_flow_parse_attr *parse_attr,
>  				struct mlx5e_tc_flow *flow,
>  				struct netlink_ext_ack *extack)
>  {
>  	struct pedit_headers_action hdrs[2] = {};
>  	struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
>  	struct mlx5_esw_flow_attr *attr = flow->esw_attr;
> +	struct mlx5e_tc_flow_parse_attr *parse_attr = attr->parse_attr;
>  	struct mlx5e_rep_priv *rpriv = priv->ppriv;
>  	const struct ip_tunnel_info *info = NULL;
>  	const struct flow_action_entry *act;
> @@ -2796,7 +2796,7 @@ static bool is_peer_flow_needed(struct mlx5e_tc_flow *flow)
>  	if (err)
>  		goto err_free;
>  
> -	err = parse_tc_fdb_actions(priv, &rule->action, parse_attr, flow, extack);
> +	err = parse_tc_fdb_actions(priv, &rule->action, flow, extack);
>  	if (err)
>  		goto err_free;
>  
> 

Reviewed-by: Roi Dayan <roid@mellanox.com>

Re: [PATCH net-next v2 4/5] net/mlx5e: Deletes unnecessary setting of esw_attr->parse_attr

[Roi Dayan]

On 25/02/2019 12:40, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> This patch deletes unnecessary setting of the esw_attr->parse_attr
> to parse_attr in parse_tc_fdb_actions() because it is already done
> by the mlx5e_flow_esw_attr_init() function.
> 
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index e6583b9..d9fcb14 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -2566,7 +2566,6 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
>  					out_dev->ifindex;
>  				parse_attr->tun_info[attr->out_count] = *info;
>  				encap = false;
> -				attr->parse_attr = parse_attr;
>  				attr->dests[attr->out_count].flags |=
>  					MLX5_ESW_DEST_ENCAP;
>  				attr->out_count++;
> 

Reviewed-by: Roi Dayan <roid@mellanox.com>

Re: [PATCH net-next v2 5/5] net/mlx5e: Return -EOPNOTSUPP when attempting to offload an unsupported action

[Roi Dayan]

On 25/02/2019 12:40, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> The encapsulation is not supported for mlx5 VFs. When we try to
> offload that action, the -EINVAL is returned, but not -EOPNOTSUPP.
> This patch changes the returned value and ignore to confuse user.
> 
> For example: (p2p1_0 is VF net device)
> tc filter add dev p2p1_0 protocol ip  parent ffff: prio 1 flower skip_sw \
> 	src_mac e4:11:22:33:44:01	\
> 	action tunnel_key set		\
> 	src_ip 1.1.1.100		\
> 	dst_ip 1.1.1.200		\
> 	dst_port 4789 id 100		\
> 	action mirred egress redirect dev vxlan0
> 
> "RTNETLINK answers: Invalid argument"
> 
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index d9fcb14..f5029ea 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -2302,7 +2302,8 @@ static int parse_tc_nic_actions(struct mlx5e_priv *priv,
>  			}
>  			break;
>  		default:
> -			return -EINVAL;
> +			NL_SET_ERR_MSG_MOD(extack, "The offload action is not supported");
> +			return -EOPNOTSUPP;
>  		}
>  	}
>  
> @@ -2624,7 +2625,8 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
>  			break;
>  			}
>  		default:
> -			return -EINVAL;
> +			NL_SET_ERR_MSG_MOD(extack, "The offload action is not supported");
> +			return -EOPNOTSUPP;
>  		}
>  	}
>  
> 

Reviewed-by: Roi Dayan <roid@mellanox.com>

Re: [PATCH net-next v2 2/5] net/mlx5e: Make the log friendly when decapsulation offload not supported

[Roi Dayan]

On 25/02/2019 12:40, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> If we try to offload decapsulation actions to VFs hw, we get the log [1].
> It's not friendly, because the kind of net device is null, and we don't
> know what '0' means.
> 
> [1] "mlx5_core 0000:05:01.2 vf_0: decapsulation offload is not supported for  net device (0)"
> 
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
> index bdcc5e7..6cbfbfa 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
> @@ -84,7 +84,7 @@ static const char *mlx5e_netdev_kind(struct net_device *dev)
>  	if (dev->rtnl_link_ops)
>  		return dev->rtnl_link_ops->kind;
>  	else
> -		return "";
> +		return "unknown";
>  }
>  
>  static int mlx5e_route_lookup_ipv6(struct mlx5e_priv *priv,
> @@ -620,8 +620,10 @@ int mlx5e_tc_tun_parse(struct net_device *filter_dev,
>  						headers_c, headers_v);
>  	} else {
>  		netdev_warn(priv->netdev,
> -			    "decapsulation offload is not supported for %s net device (%d)\n",
> -			    mlx5e_netdev_kind(filter_dev), tunnel_type);
> +			    "decapsulation offload is not supported for %s (kind: \"%s\")\n",
> +			    netdev_name(filter_dev),
> +			    mlx5e_netdev_kind(filter_dev));
> +
>  		return -EOPNOTSUPP;
>  	}
>  	return err;
> 

Reviewed-by: Roi Dayan <roid@mellanox.com>

Re: [PATCH net-next v2 1/5] net/mlx5e: Return -EOPNOTSUPP when modify header action zero

[Roi Dayan]

On 25/02/2019 12:40, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> When max modify header action is zero, we return -EOPNOTSUPP
> directly. In this way, we can ignore wrong message info (e.g.
> "mlx5: parsed 0 pedit actions, can't do more").
> 
> This happens when offloading pedit actions on mlx VFs.
> 
> For example:
> $ tc filter add dev mlx5_vf parent ffff: protocol ip prio 1 \
> 	flower skip_sw dst_mac 00:10:56:fb:64:e8 \
> 	dst_ip 1.1.1.100 src_ip 1.1.1.200 \
> 	action pedit ex munge eth src set 00:10:56:b4:5d:20
> 
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index b38986e..708f819 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -2002,7 +2002,8 @@ static int offload_pedit_fields(struct pedit_headers_action *hdrs,
>  static int alloc_mod_hdr_actions(struct mlx5e_priv *priv,
>  				 struct pedit_headers_action *hdrs,
>  				 int namespace,
> -				 struct mlx5e_tc_flow_parse_attr *parse_attr)
> +				 struct mlx5e_tc_flow_parse_attr *parse_attr,
> +				 struct netlink_ext_ack *extack)
>  {
>  	int nkeys, action_size, max_actions;
>  
> @@ -2015,6 +2016,12 @@ static int alloc_mod_hdr_actions(struct mlx5e_priv *priv,
>  	else /* namespace is MLX5_FLOW_NAMESPACE_KERNEL - NIC offloading */
>  		max_actions = MLX5_CAP_FLOWTABLE_NIC_RX(priv->mdev, max_modify_header_actions);
>  
> +	if (!max_actions) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "don't support pedit actions, can't offload");

can we rephrase that to match the msg style you did in patch 5 ?
i.e. The pedit offload action is not supported


> +		return -EOPNOTSUPP;
> +	}
> +
>  	/* can get up to crazingly 16 HW actions in 32 bits pedit SW key */
>  	max_actions = min(max_actions, nkeys * 16);
>  
> @@ -2072,7 +2079,8 @@ static int alloc_tc_pedit_action(struct mlx5e_priv *priv, int namespace,
>  	u8 cmd;
>  
>  	if (!parse_attr->mod_hdr_actions) {
> -		err = alloc_mod_hdr_actions(priv, hdrs, namespace, parse_attr);
> +		err = alloc_mod_hdr_actions(priv, hdrs,
> +					    namespace, parse_attr, extack);
>  		if (err)
>  			goto out_err;
>  	}
> 

Re: [PATCH net-next v2 1/5] net/mlx5e: Return -EOPNOTSUPP when modify header action zero

[Or Gerlitz]

On Mon, Feb 25, 2019 at 1:06 PM <xiangxia.m.yue@gmail.com> wrote:

> When max modify header action is zero, we return -EOPNOTSUPP
> directly. In this way, we can ignore wrong message info (e.g.
> "mlx5: parsed 0 pedit actions, can't do more").
>
> This happens when offloading pedit actions on mlx VFs.

this command should work, we support header re-write (pedit offload)
for tc NIC rules

Is this CX5 VF? if yes, something broke

> For example:
> $ tc filter add dev mlx5_vf parent ffff: protocol ip prio 1 \
>         flower skip_sw dst_mac 00:10:56:fb:64:e8 \
>         dst_ip 1.1.1.100 src_ip 1.1.1.200 \
>         action pedit ex munge eth src set 00:10:56:b4:5d:20

Re: [PATCH net-next v2 5/5] net/mlx5e: Return -EOPNOTSUPP when attempting to offload an unsupported action

[Or Gerlitz]

On Mon, Feb 25, 2019 at 1:07 PM <xiangxia.m.yue@gmail.com> wrote:

> The encapsulation is not supported for mlx5 VFs. When we try to
> offload that action, the -EINVAL is returned, but not -EOPNOTSUPP.
> This patch changes the returned value and ignore to confuse user.

FWIW, note that this changes the behavior towards user-space, I don't see
concrete harm done here but we should realize that

> For example: (p2p1_0 is VF net device)
> tc filter add dev p2p1_0 protocol ip  parent ffff: prio 1 flower skip_sw \
>         src_mac e4:11:22:33:44:01       \
>         action tunnel_key set           \
>         src_ip 1.1.1.100                \
>         dst_ip 1.1.1.200                \
>         dst_port 4789 id 100            \
>         action mirred egress redirect dev vxlan0
>
> "RTNETLINK answers: Invalid argument"

Re: [PATCH net-next v2 1/5] net/mlx5e: Return -EOPNOTSUPP when modify header action zero

[Tonghao Zhang]

On Tue, Feb 26, 2019 at 9:54 PM Roi Dayan <roid@mellanox.com> wrote:
>
>
>
> On 25/02/2019 12:40, xiangxia.m.yue@gmail.com wrote:
> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >
> > When max modify header action is zero, we return -EOPNOTSUPP
> > directly. In this way, we can ignore wrong message info (e.g.
> > "mlx5: parsed 0 pedit actions, can't do more").
> >
> > This happens when offloading pedit actions on mlx VFs.
> >
> > For example:
> > $ tc filter add dev mlx5_vf parent ffff: protocol ip prio 1 \
> >       flower skip_sw dst_mac 00:10:56:fb:64:e8 \
> >       dst_ip 1.1.1.100 src_ip 1.1.1.200 \
> >       action pedit ex munge eth src set 00:10:56:b4:5d:20
> >
> > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > ---
> >  drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > index b38986e..708f819 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > @@ -2002,7 +2002,8 @@ static int offload_pedit_fields(struct pedit_headers_action *hdrs,
> >  static int alloc_mod_hdr_actions(struct mlx5e_priv *priv,
> >                                struct pedit_headers_action *hdrs,
> >                                int namespace,
> > -                              struct mlx5e_tc_flow_parse_attr *parse_attr)
> > +                              struct mlx5e_tc_flow_parse_attr *parse_attr,
> > +                              struct netlink_ext_ack *extack)
> >  {
> >       int nkeys, action_size, max_actions;
> >
> > @@ -2015,6 +2016,12 @@ static int alloc_mod_hdr_actions(struct mlx5e_priv *priv,
> >       else /* namespace is MLX5_FLOW_NAMESPACE_KERNEL - NIC offloading */
> >               max_actions = MLX5_CAP_FLOWTABLE_NIC_RX(priv->mdev, max_modify_header_actions);
> >
> > +     if (!max_actions) {
> > +             NL_SET_ERR_MSG_MOD(extack,
> > +                                "don't support pedit actions, can't offload");
>
> can we rephrase that to match the msg style you did in patch 5 ?
Yes, good idea
> i.e. The pedit offload action is not supported
>
>
> > +             return -EOPNOTSUPP;
> > +     }
> > +
> >       /* can get up to crazingly 16 HW actions in 32 bits pedit SW key */
> >       max_actions = min(max_actions, nkeys * 16);
> >
> > @@ -2072,7 +2079,8 @@ static int alloc_tc_pedit_action(struct mlx5e_priv *priv, int namespace,
> >       u8 cmd;
> >
> >       if (!parse_attr->mod_hdr_actions) {
> > -             err = alloc_mod_hdr_actions(priv, hdrs, namespace, parse_attr);
> > +             err = alloc_mod_hdr_actions(priv, hdrs,
> > +                                         namespace, parse_attr, extack);
> >               if (err)
> >                       goto out_err;
> >       }
> >

Re: [PATCH net-next v2 1/5] net/mlx5e: Return -EOPNOTSUPP when modify header action zero

[Tonghao Zhang]

On Wed, Feb 27, 2019 at 6:37 AM Or Gerlitz <gerlitz.or@gmail.com> wrote:
>
> On Mon, Feb 25, 2019 at 1:06 PM <xiangxia.m.yue@gmail.com> wrote:
>>
>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>>
>> When max modify header action is zero, we return -EOPNOTSUPP
>> directly. In this way, we can ignore wrong message info (e.g.
>> "mlx5: parsed 0 pedit actions, can't do more").
>>
>> This happens when offloading pedit actions on mlx VFs.
>>
>> For example:
>> $ tc filter add dev mlx5_vf parent ffff: protocol ip prio 1 \
>>         flower skip_sw dst_mac 00:10:56:fb:64:e8 \
>>         dst_ip 1.1.1.100 src_ip 1.1.1.200 \
>>         action pedit ex munge eth src set 00:10:56:b4:5d:20
>
>
>
> this command should work, we support header re-write (pedit offload) for tc NIC rules
>
> Is this CX5 VF? if yes, something broke
No, this is cx4
>