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] [day] [month] [year] [list]

Message-ID: <20191030075751.GA16499@splinter>
Date:   Wed, 30 Oct 2019 09:57:51 +0200
From:   Ido Schimmel <idosch@...sch.org>
To:     Nathan Chancellor <natechancellor@...il.com>
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 Tue, Oct 29, 2019 at 08:31:54PM -0700, Nathan Chancellor wrote:
> On Wed, Oct 23, 2019 at 09:05:00AM +0300, Ido Schimmel wrote:
> >  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.

Yes, this looks fine to me. The value can fit in a u32. Tested on my
system without issues.

Thanks for reaching out

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