netdev - Re: [PATCH net-next 2/2] mlxsw: spectrum_buffers: Calculate the size of the main pool

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: <20191030033154.GA43266@ubuntu-m2-xlarge-x86>
Date:   Tue, 29 Oct 2019 20:31:54 -0700
From:   Nathan Chancellor <natechancellor@...il.com>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        jakub.kicinski@...ronome.com, jiri@...lanox.com,
        petrm@...lanox.com, mlxsw@...lanox.com,
        Ido Schimmel <idosch@...lanox.com>
Subject: Re: [PATCH net-next 2/2] mlxsw: spectrum_buffers: Calculate the size
 of the main pool

On Wed, Oct 23, 2019 at 09:05:00AM +0300, Ido Schimmel wrote:
> From: Petr Machata <petrm@...lanox.com>
> 
> Instead of hard-coding the size of the largest pool, calculate it from the
> reported guaranteed shared buffer size and sizes of other pools (currently
> only the CPU port pool).
> 
> Signed-off-by: Petr Machata <petrm@...lanox.com>
> Signed-off-by: Ido Schimmel <idosch@...lanox.com>
> ---
>  .../mellanox/mlxsw/spectrum_buffers.c         | 46 ++++++++++++++-----
>  1 file changed, 34 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
> index 637151682cf2..5fd9a72c8471 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
> @@ -35,6 +35,7 @@ struct mlxsw_sp_sb_cm {
>  };
>  
>  #define MLXSW_SP_SB_INFI -1U
> +#define MLXSW_SP_SB_REST -2U
>  
>  struct mlxsw_sp_sb_pm {
>  	u32 min_buff;
> @@ -421,19 +422,16 @@ static void mlxsw_sp_sb_ports_fini(struct mlxsw_sp *mlxsw_sp)
>  		.freeze_size = _freeze_size,				\
>  	}
>  
> -#define MLXSW_SP1_SB_PR_INGRESS_SIZE	13768608
> -#define MLXSW_SP1_SB_PR_EGRESS_SIZE	13768608
>  #define MLXSW_SP1_SB_PR_CPU_SIZE	(256 * 1000)
>  
>  /* Order according to mlxsw_sp1_sb_pool_dess */
>  static const struct mlxsw_sp_sb_pr mlxsw_sp1_sb_prs[] = {
> -	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_DYNAMIC,
> -		       MLXSW_SP1_SB_PR_INGRESS_SIZE),
> +	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_DYNAMIC, MLXSW_SP_SB_REST),
>  	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_DYNAMIC, 0),
>  	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_DYNAMIC, 0),
>  	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_DYNAMIC, 0),
> -	MLXSW_SP_SB_PR_EXT(MLXSW_REG_SBPR_MODE_DYNAMIC,
> -			   MLXSW_SP1_SB_PR_EGRESS_SIZE, true, false),
> +	MLXSW_SP_SB_PR_EXT(MLXSW_REG_SBPR_MODE_DYNAMIC, MLXSW_SP_SB_REST,
> +			   true, false),
>  	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_DYNAMIC, 0),
>  	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_DYNAMIC, 0),
>  	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_DYNAMIC, 0),
> @@ -445,19 +443,16 @@ static const struct mlxsw_sp_sb_pr mlxsw_sp1_sb_prs[] = {
>  			   MLXSW_SP1_SB_PR_CPU_SIZE, true, false),
>  };
>  
> -#define MLXSW_SP2_SB_PR_INGRESS_SIZE	34084800
> -#define MLXSW_SP2_SB_PR_EGRESS_SIZE	34084800
>  #define MLXSW_SP2_SB_PR_CPU_SIZE	(256 * 1000)
>  
>  /* Order according to mlxsw_sp2_sb_pool_dess */
>  static const struct mlxsw_sp_sb_pr mlxsw_sp2_sb_prs[] = {
> -	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_DYNAMIC,
> -		       MLXSW_SP2_SB_PR_INGRESS_SIZE),
> +	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_DYNAMIC, MLXSW_SP_SB_REST),
>  	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_STATIC, 0),
>  	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_STATIC, 0),
>  	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_STATIC, 0),
> -	MLXSW_SP_SB_PR_EXT(MLXSW_REG_SBPR_MODE_DYNAMIC,
> -			   MLXSW_SP2_SB_PR_EGRESS_SIZE, true, false),
> +	MLXSW_SP_SB_PR_EXT(MLXSW_REG_SBPR_MODE_DYNAMIC, MLXSW_SP_SB_REST,
> +			   true, false),
>  	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_STATIC, 0),
>  	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_STATIC, 0),
>  	MLXSW_SP_SB_PR(MLXSW_REG_SBPR_MODE_STATIC, 0),
> @@ -471,11 +466,33 @@ static const struct mlxsw_sp_sb_pr mlxsw_sp2_sb_prs[] = {
>  
>  static int mlxsw_sp_sb_prs_init(struct mlxsw_sp *mlxsw_sp,
>  				const struct mlxsw_sp_sb_pr *prs,
> +				const struct mlxsw_sp_sb_pool_des *pool_dess,
>  				size_t prs_len)
>  {
> +	/* Round down, unlike mlxsw_sp_bytes_cells(). */
> +	u32 sb_cells = mlxsw_sp->sb->sb_size / mlxsw_sp->sb->cell_size;

This causes a link time error on arm32. It can be simply reproduced with
the following configs + multi_v7_defconfig:

CONFIG_MLXSW_CORE=y
CONFIG_MLXSW_PCI=y
CONFIG_NET_SWITCHDEV=y
CONFIG_VLAN_8021Q=y
CONFIG_MLXSW_SPECTRUM=y

arm-linux-gnueabi-ld: drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.o: in function `mlxsw_sp_buffers_init':
spectrum_buffers.c:(.text+0x1c88): undefined reference to `__aeabi_uldivmod'

It can be solved by something like this but I am not sure if that is
proper or not since div_u64 returns a u64, which would implicitly get
converted to u32. I can submit it as a formal patch if needed but I
figured I would reach out first in case you want to go in a different
direction.

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
index 33a978af80d6..968f0902e4fe 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
@@ -470,7 +470,7 @@ static int mlxsw_sp_sb_prs_init(struct mlxsw_sp *mlxsw_sp,
 				size_t prs_len)
 {
 	/* Round down, unlike mlxsw_sp_bytes_cells(). */
-	u32 sb_cells = mlxsw_sp->sb->sb_size / mlxsw_sp->sb->cell_size;
+	u32 sb_cells = div_u64(mlxsw_sp->sb->sb_size, mlxsw_sp->sb->cell_size);
 	u32 rest_cells[2] = {sb_cells, sb_cells};
 	int i;
 	int err;