diff mbox series

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

Message ID 1551091207-10366-2-git-send-email-xiangxia.m.yue@gmail.com
State Awaiting Upstream
Delegated to: David Miller
Headers show
Series net/mlx5e: Make little improvement for mlx5e | expand

Commit Message

Tonghao Zhang Feb. 25, 2019, 10:40 a.m. UTC 
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(-)

Comments

Roi Dayan Feb. 26, 2019, 1:54 p.m. UTC | #1 
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;
>  	}
>
Or Gerlitz Feb. 26, 2019, 10:41 p.m. UTC | #2 
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
Tonghao Zhang Feb. 27, 2019, 11:46 a.m. UTC | #3 
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;
> >       }
> >
Tonghao Zhang Feb. 27, 2019, 11:47 a.m. UTC | #4 
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
>
diff mbox series

Patch

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;
 	}