* [PATCH net-next v3 1/4] net/mlx5e: Make the log friendly when decapsulation offload not supported
2019-02-26 12:27 [PATCH net-next v3 0/4] net/mlx5e: Make little improvement for mlx5e xiangxia.m.yue
@ 2019-02-26 12:27 ` xiangxia.m.yue
2019-02-26 12:27 ` [PATCH net-next v3 2/4] net/mlx5e: Remove 'parse_attr' argument in parse_tc_fdb_actions() xiangxia.m.yue
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: xiangxia.m.yue @ 2019-02-26 12:27 UTC (permalink / raw)
To: saeedm, gerlitz.or, roid; +Cc: netdev, Tonghao Zhang
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>
Reviewed-by: Roi Dayan <roid@mellanox.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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net-next v3 2/4] net/mlx5e: Remove 'parse_attr' argument in parse_tc_fdb_actions()
2019-02-26 12:27 [PATCH net-next v3 0/4] net/mlx5e: Make little improvement for mlx5e xiangxia.m.yue
2019-02-26 12:27 ` [PATCH net-next v3 1/4] net/mlx5e: Make the log friendly when decapsulation offload not supported xiangxia.m.yue
@ 2019-02-26 12:27 ` xiangxia.m.yue
2019-02-26 12:27 ` [PATCH net-next v3 3/4] net/mlx5e: Deletes unnecessary setting of esw_attr->parse_attr xiangxia.m.yue
2019-02-26 12:27 ` [PATCH net-next v3 4/4] net/mlx5e: Return -EOPNOTSUPP when attempting to offload an unsupported action xiangxia.m.yue
3 siblings, 0 replies; 7+ messages in thread
From: xiangxia.m.yue @ 2019-02-26 12:27 UTC (permalink / raw)
To: saeedm, gerlitz.or, roid; +Cc: netdev, Tonghao Zhang
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>
Reviewed-by: Roi Dayan <roid@mellanox.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 b38986e..336a111 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2467,13 +2467,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;
@@ -2788,7 +2788,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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net-next v3 3/4] net/mlx5e: Deletes unnecessary setting of esw_attr->parse_attr
2019-02-26 12:27 [PATCH net-next v3 0/4] net/mlx5e: Make little improvement for mlx5e xiangxia.m.yue
2019-02-26 12:27 ` [PATCH net-next v3 1/4] net/mlx5e: Make the log friendly when decapsulation offload not supported xiangxia.m.yue
2019-02-26 12:27 ` [PATCH net-next v3 2/4] net/mlx5e: Remove 'parse_attr' argument in parse_tc_fdb_actions() xiangxia.m.yue
@ 2019-02-26 12:27 ` xiangxia.m.yue
2019-02-26 12:27 ` [PATCH net-next v3 4/4] net/mlx5e: Return -EOPNOTSUPP when attempting to offload an unsupported action xiangxia.m.yue
3 siblings, 0 replies; 7+ messages in thread
From: xiangxia.m.yue @ 2019-02-26 12:27 UTC (permalink / raw)
To: saeedm, gerlitz.or, roid; +Cc: netdev, Tonghao Zhang
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>
Reviewed-by: Roi Dayan <roid@mellanox.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 336a111..56ac50d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2558,7 +2558,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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next v3 4/4] net/mlx5e: Return -EOPNOTSUPP when attempting to offload an unsupported action
2019-02-26 12:27 [PATCH net-next v3 0/4] net/mlx5e: Make little improvement for mlx5e xiangxia.m.yue
` (2 preceding siblings ...)
2019-02-26 12:27 ` [PATCH net-next v3 3/4] net/mlx5e: Deletes unnecessary setting of esw_attr->parse_attr xiangxia.m.yue
@ 2019-02-26 12:27 ` xiangxia.m.yue
2019-02-27 14:01 ` Roi Dayan
3 siblings, 1 reply; 7+ messages in thread
From: xiangxia.m.yue @ 2019-02-26 12:27 UTC (permalink / raw)
To: saeedm, gerlitz.or, roid; +Cc: netdev, Tonghao Zhang
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
* Now 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.
The command is shown as below [1].
* 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(cx4) VFs. The command is shown as below [2].
For example: (p2p1_0 is VF net device)
[1]
$ 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
[2]
$ tc filter add dev p2p1_0 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 | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 56ac50d..3a02b22 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2035,7 +2035,7 @@ static int parse_tc_pedit_action(struct mlx5e_priv *priv,
struct netlink_ext_ack *extack)
{
u8 cmd = (act->id == FLOW_ACTION_MANGLE) ? 0 : 1;
- int err = -EOPNOTSUPP;
+ int max_actions, err = -EOPNOTSUPP;
u32 mask, val, offset;
u8 htype;
@@ -2047,6 +2047,17 @@ static int parse_tc_pedit_action(struct mlx5e_priv *priv,
goto out_err;
}
+ if (namespace == MLX5_FLOW_NAMESPACE_FDB) /* FDB offloading */
+ max_actions = MLX5_CAP_ESW_FLOWTABLE_FDB(priv->mdev, max_modify_header_actions);
+ 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");
+ goto out_err;
+ }
+
mask = act->mangle.mask;
val = act->mangle.val;
offset = act->mangle.offset;
@@ -2294,7 +2305,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;
}
}
@@ -2616,7 +2628,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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net-next v3 4/4] net/mlx5e: Return -EOPNOTSUPP when attempting to offload an unsupported action
2019-02-26 12:27 ` [PATCH net-next v3 4/4] net/mlx5e: Return -EOPNOTSUPP when attempting to offload an unsupported action xiangxia.m.yue
@ 2019-02-27 14:01 ` Roi Dayan
2019-02-28 10:22 ` Tonghao Zhang
0 siblings, 1 reply; 7+ messages in thread
From: Roi Dayan @ 2019-02-27 14:01 UTC (permalink / raw)
To: xiangxia.m.yue@gmail.com, Saeed Mahameed, gerlitz.or@gmail.com
Cc: netdev@vger.kernel.org
On 26/02/2019 14:27, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> * Now 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.
> The command is shown as below [1].
>
> * 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(cx4) VFs. The command is shown as below [2].
>
> For example: (p2p1_0 is VF net device)
> [1]
> $ 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
>
> [2]
> $ tc filter add dev p2p1_0 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 | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index 56ac50d..3a02b22 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -2035,7 +2035,7 @@ static int parse_tc_pedit_action(struct mlx5e_priv *priv,
> struct netlink_ext_ack *extack)
> {
> u8 cmd = (act->id == FLOW_ACTION_MANGLE) ? 0 : 1;
> - int err = -EOPNOTSUPP;
> + int max_actions, err = -EOPNOTSUPP;
> u32 mask, val, offset;
> u8 htype;
>
> @@ -2047,6 +2047,17 @@ static int parse_tc_pedit_action(struct mlx5e_priv *priv,
> goto out_err;
> }
>
> + if (namespace == MLX5_FLOW_NAMESPACE_FDB) /* FDB offloading */
> + max_actions = MLX5_CAP_ESW_FLOWTABLE_FDB(priv->mdev, max_modify_header_actions);
> + else /* namespace is MLX5_FLOW_NAMESPACE_KERNEL - NIC offloading */
> + max_actions = MLX5_CAP_FLOWTABLE_NIC_RX(priv->mdev, max_modify_header_actions);
> +
we have the exact same block with comments in alloc_mod_hdr_actions() which
is called later after we parse the action and now parsing the pedit fields.
your check for max_actions was there in prev v. why move it?
if like this I suggest we create a small static function to get max actions
for a namespace and call it from both places to get max actions.
> + if (!max_actions) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "don't support pedit actions, can't offload");
please rephrase the msg to be consistent
i.e. The pedit offload action is not supported
> + goto out_err;
> + }
> +
> mask = act->mangle.mask;
> val = act->mangle.val;
> offset = act->mangle.offset;
> @@ -2294,7 +2305,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;
> }
> }
>
> @@ -2616,7 +2628,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;
> }
> }
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net-next v3 4/4] net/mlx5e: Return -EOPNOTSUPP when attempting to offload an unsupported action
2019-02-27 14:01 ` Roi Dayan
@ 2019-02-28 10:22 ` Tonghao Zhang
0 siblings, 0 replies; 7+ messages in thread
From: Tonghao Zhang @ 2019-02-28 10:22 UTC (permalink / raw)
To: Roi Dayan; +Cc: Saeed Mahameed, gerlitz.or@gmail.com, netdev@vger.kernel.org
On Wed, Feb 27, 2019 at 10:01 PM Roi Dayan <roid@mellanox.com> wrote:
>
>
>
> On 26/02/2019 14:27, xiangxia.m.yue@gmail.com wrote:
> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >
> > * Now 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.
> > The command is shown as below [1].
> >
> > * 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(cx4) VFs. The command is shown as below [2].
> >
> > For example: (p2p1_0 is VF net device)
> > [1]
> > $ 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
> >
> > [2]
> > $ tc filter add dev p2p1_0 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 | 19 ++++++++++++++++---
> > 1 file changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > index 56ac50d..3a02b22 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > @@ -2035,7 +2035,7 @@ static int parse_tc_pedit_action(struct mlx5e_priv *priv,
> > struct netlink_ext_ack *extack)
> > {
> > u8 cmd = (act->id == FLOW_ACTION_MANGLE) ? 0 : 1;
> > - int err = -EOPNOTSUPP;
> > + int max_actions, err = -EOPNOTSUPP;
> > u32 mask, val, offset;
> > u8 htype;
> >
> > @@ -2047,6 +2047,17 @@ static int parse_tc_pedit_action(struct mlx5e_priv *priv,
> > goto out_err;
> > }
> >
> > + if (namespace == MLX5_FLOW_NAMESPACE_FDB) /* FDB offloading */
> > + max_actions = MLX5_CAP_ESW_FLOWTABLE_FDB(priv->mdev, max_modify_header_actions);
> > + else /* namespace is MLX5_FLOW_NAMESPACE_KERNEL - NIC offloading */
> > + max_actions = MLX5_CAP_FLOWTABLE_NIC_RX(priv->mdev, max_modify_header_actions);
> > +
>
> we have the exact same block with comments in alloc_mod_hdr_actions() which
> is called later after we parse the action and now parsing the pedit fields.
> your check for max_actions was there in prev v. why move it?
I think we should check it as soon as possible.
> if like this I suggest we create a small static function to get max actions
> for a namespace and call it from both places to get max actions.
it's ok.
> > + if (!max_actions) {
> > + NL_SET_ERR_MSG_MOD(extack,
> > + "don't support pedit actions, can't offload");
>
> please rephrase the msg to be consistent
> i.e. The pedit offload action is not supported
>
> > + goto out_err;
> > + }
> > +
> > mask = act->mangle.mask;
> > val = act->mangle.val;
> > offset = act->mangle.offset;
> > @@ -2294,7 +2305,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;
> > }
> > }
> >
> > @@ -2616,7 +2628,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;
> > }
> > }
> >
> >
^ permalink raw reply [flat|nested] 7+ messages in thread