netdev - Re: [net-next 01/15] net/mlx5: Lag, Let user configure multiport eswitch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y+zGFVZPj2UzY0K2@unreal>
Date:   Wed, 15 Feb 2023 13:46:29 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Alexander Lobakin <alexandr.lobakin@...el.com>
Cc:     Saeed Mahameed <saeed@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Saeed Mahameed <saeedm@...dia.com>, netdev@...r.kernel.org,
        Tariq Toukan <tariqt@...dia.com>, Roi Dayan <roid@...dia.com>,
        Maor Dickman <maord@...dia.com>
Subject: Re: [net-next 01/15] net/mlx5: Lag, Let user configure multiport
 eswitch

On Tue, Feb 14, 2023 at 06:07:54PM +0100, Alexander Lobakin wrote:
> From: Saeed Mahameed <saeed@...nel.org>
> Date: Fri, 10 Feb 2023 14:18:07 -0800
> 
> > From: Roi Dayan <roid@...dia.com>
> > 
> > Instead of activating multiport eswitch dynamically through
> > adding a TC rule and meeting certain conditions, allow the user
> > to activate it through devlink.
> > This will remove the forced requirement of using TC.
> > e.g. Bridge offload.
> > 
> > Example:
> >     $ devlink dev param set pci/0000:00:0b.0 name esw_multiport value 1 \
> >                   cmode runtime
> > 
> > Signed-off-by: Roi Dayan <roid@...dia.com>
> > Reviewed-by: Maor Dickman <maord@...dia.com>
> > Signed-off-by: Saeed Mahameed <saeedm@...dia.com>
> > ---
> >  Documentation/networking/devlink/mlx5.rst     |  4 ++
> >  .../net/ethernet/mellanox/mlx5/core/devlink.c | 56 +++++++++++++++++++
> >  .../net/ethernet/mellanox/mlx5/core/devlink.h |  1 +
> >  .../mellanox/mlx5/core/en/tc/act/mirred.c     |  9 ---
> >  .../net/ethernet/mellanox/mlx5/core/en_tc.c   | 22 +-------
> >  .../net/ethernet/mellanox/mlx5/core/en_tc.h   |  6 --
> >  .../net/ethernet/mellanox/mlx5/core/lag/lag.c |  4 +-
> >  .../net/ethernet/mellanox/mlx5/core/lag/lag.h |  1 +
> >  .../ethernet/mellanox/mlx5/core/lag/mpesw.c   | 46 +++++++--------
> >  .../ethernet/mellanox/mlx5/core/lag/mpesw.h   | 12 +---
> >  10 files changed, 87 insertions(+), 74 deletions(-)
> > 
> > diff --git a/Documentation/networking/devlink/mlx5.rst b/Documentation/networking/devlink/mlx5.rst
> > index 29ad304e6fba..1d2ad2727da1 100644
> > --- a/Documentation/networking/devlink/mlx5.rst
> > +++ b/Documentation/networking/devlink/mlx5.rst
> > @@ -54,6 +54,10 @@ parameters.
> >       - Control the number of large groups (size > 1) in the FDB table.
> >  
> >         * The default value is 15, and the range is between 1 and 1024.
> > +   * - ``esw_multiport``
> > +     - Boolean
> > +     - runtime
> > +     - Set the E-Switch lag mode to multiport.
> >  
> >  The ``mlx5`` driver supports reloading via ``DEVLINK_CMD_RELOAD``
> >  
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
> > index b742e04deec1..49392870f695 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
> > @@ -7,6 +7,7 @@
> >  #include "fw_reset.h"
> >  #include "fs_core.h"
> >  #include "eswitch.h"
> > +#include "lag/lag.h"
> >  #include "esw/qos.h"
> >  #include "sf/dev/dev.h"
> >  #include "sf/sf.h"
> > @@ -437,6 +438,55 @@ static int mlx5_devlink_large_group_num_validate(struct devlink *devlink, u32 id
> >  	return 0;
> >  }
> >  
> > +static int mlx5_devlink_esw_multiport_set(struct devlink *devlink, u32 id,
> > +					  struct devlink_param_gset_ctx *ctx)
> > +{
> > +	struct mlx5_core_dev *dev = devlink_priv(devlink);
> > +	int err = 0;
> > +
> > +	if (!MLX5_ESWITCH_MANAGER(dev))
> > +		return -EOPNOTSUPP;
> > +
> > +	if (ctx->val.vbool)
> > +		err = mlx5_lag_mpesw_enable(dev);
> > +	else
> > +		mlx5_lag_mpesw_disable(dev);
> > +
> > +	return err;
> 
> How about
> 
> 	if (ctx->val.vbool)
> 		return mlx5_lag_mpesw_enable(dev);
> 	else
> 		mlx5_lag_mpesw_disable(dev);
> 
> 	return 0;

If such construction is used, there won't need in "else".

 	if (ctx->val.vbool)
 		return mlx5_lag_mpesw_enable(dev);

 	mlx5_lag_mpesw_disable(dev);
 	return 0;




> 
> ?
> 
> > +}
> > +
> > +static int mlx5_devlink_esw_multiport_get(struct devlink *devlink, u32 id,
> > +					  struct devlink_param_gset_ctx *ctx)
> [...]
> 
> Thanks,
> Olek