2020-01-30 23:28:01

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps

When building arm32 allmodconfig:

ERROR: "__aeabi_uldivmod"
[drivers/net/ethernet/mellanox/mlxsw/mlxsw_spectrum.ko] undefined!

rate_bytes_ps has type u64, we need to use a 64-bit division helper to
avoid a build error.

Fixes: a44f58c41bfb ("mlxsw: spectrum_qdisc: Support offloading of TBF Qdisc")
Signed-off-by: Nathan Chancellor <[email protected]>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
index 79a2801d59f6..65e681ef01e8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
@@ -614,7 +614,7 @@ mlxsw_sp_qdisc_tbf_rate_kbps(struct tc_tbf_qopt_offload_replace_params *p)
/* TBF interface is in bytes/s, whereas Spectrum ASIC is configured in
* Kbits/s.
*/
- return p->rate.rate_bytes_ps / 1000 * 8;
+ return div_u64(p->rate.rate_bytes_ps, 1000 * 8);
}

static int
--
2.25.0


2020-01-31 01:46:11

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps

On 1/30/20 3:26 PM, Nathan Chancellor wrote:
> When building arm32 allmodconfig:
>
> ERROR: "__aeabi_uldivmod"
> [drivers/net/ethernet/mellanox/mlxsw/mlxsw_spectrum.ko] undefined!
>
> rate_bytes_ps has type u64, we need to use a 64-bit division helper to
> avoid a build error.
>
> Fixes: a44f58c41bfb ("mlxsw: spectrum_qdisc: Support offloading of TBF Qdisc")
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
> drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
> index 79a2801d59f6..65e681ef01e8 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
> @@ -614,7 +614,7 @@ mlxsw_sp_qdisc_tbf_rate_kbps(struct tc_tbf_qopt_offload_replace_params *p)
> /* TBF interface is in bytes/s, whereas Spectrum ASIC is configured in
> * Kbits/s.
> */
> - return p->rate.rate_bytes_ps / 1000 * 8;
> + return div_u64(p->rate.rate_bytes_ps, 1000 * 8);

not quite right AFAICT.

try either
return div_u64(p->rate.rate_bytes_ps * 8, 1000);
or
return div_u64(p->rate.rate_bytes_ps, 1000) * 8;

> }
>
> static int
>


--
~Randy

2020-01-31 01:49:32

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps

On Thu, Jan 30, 2020 at 05:43:56PM -0800, Randy Dunlap wrote:
> On 1/30/20 3:26 PM, Nathan Chancellor wrote:
> > When building arm32 allmodconfig:
> >
> > ERROR: "__aeabi_uldivmod"
> > [drivers/net/ethernet/mellanox/mlxsw/mlxsw_spectrum.ko] undefined!
> >
> > rate_bytes_ps has type u64, we need to use a 64-bit division helper to
> > avoid a build error.
> >
> > Fixes: a44f58c41bfb ("mlxsw: spectrum_qdisc: Support offloading of TBF Qdisc")
> > Signed-off-by: Nathan Chancellor <[email protected]>
> > ---
> > drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
> > index 79a2801d59f6..65e681ef01e8 100644
> > --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
> > +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
> > @@ -614,7 +614,7 @@ mlxsw_sp_qdisc_tbf_rate_kbps(struct tc_tbf_qopt_offload_replace_params *p)
> > /* TBF interface is in bytes/s, whereas Spectrum ASIC is configured in
> > * Kbits/s.
> > */
> > - return p->rate.rate_bytes_ps / 1000 * 8;
> > + return div_u64(p->rate.rate_bytes_ps, 1000 * 8);
>
> not quite right AFAICT.
>
> try either
> return div_u64(p->rate.rate_bytes_ps * 8, 1000);
> or
> return div_u64(p->rate.rate_bytes_ps, 1000) * 8;
>

Gah, I swear I can math... Thank you for catching this, v2 incoming with
the later because I think it looks better.

Cheers,
Nathan

> > }
> >
> > static int
> >
>
>
> --
> ~Randy
>

2020-01-31 01:52:35

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH v2] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps

When building arm32 allmodconfig:

ERROR: "__aeabi_uldivmod"
[drivers/net/ethernet/mellanox/mlxsw/mlxsw_spectrum.ko] undefined!

rate_bytes_ps has type u64, we need to use a 64-bit division helper to
avoid a build error.

Fixes: a44f58c41bfb ("mlxsw: spectrum_qdisc: Support offloading of TBF Qdisc")
Signed-off-by: Nathan Chancellor <[email protected]>
---

v1 -> v2:

* Fix incorrect math as pointed out by Randy Dunlap

drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
index 79a2801d59f6..02526c53d4f5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
@@ -614,7 +614,7 @@ mlxsw_sp_qdisc_tbf_rate_kbps(struct tc_tbf_qopt_offload_replace_params *p)
/* TBF interface is in bytes/s, whereas Spectrum ASIC is configured in
* Kbits/s.
*/
- return p->rate.rate_bytes_ps / 1000 * 8;
+ return div_u64(p->rate.rate_bytes_ps, 1000) * 8;
}

static int
--
2.25.0

2020-01-31 13:49:00

by Ido Schimmel

[permalink] [raw]
Subject: Re: [PATCH v2] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps

On Thu, Jan 30, 2020 at 06:51:23PM -0700, Nathan Chancellor wrote:
> When building arm32 allmodconfig:
>
> ERROR: "__aeabi_uldivmod"
> [drivers/net/ethernet/mellanox/mlxsw/mlxsw_spectrum.ko] undefined!
>
> rate_bytes_ps has type u64, we need to use a 64-bit division helper to
> avoid a build error.
>
> Fixes: a44f58c41bfb ("mlxsw: spectrum_qdisc: Support offloading of TBF Qdisc")
> Signed-off-by: Nathan Chancellor <[email protected]>

Reviewed-by: Ido Schimmel <[email protected]>
Tested-by: Ido Schimmel <[email protected]>

Thanks, Nathan.

2020-01-31 14:30:48

by Jiri Pirko

[permalink] [raw]
Subject: Re: [PATCH v2] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps

Fri, Jan 31, 2020 at 02:51:23AM CET, [email protected] wrote:
>When building arm32 allmodconfig:
>
>ERROR: "__aeabi_uldivmod"
>[drivers/net/ethernet/mellanox/mlxsw/mlxsw_spectrum.ko] undefined!
>
>rate_bytes_ps has type u64, we need to use a 64-bit division helper to
>avoid a build error.
>
>Fixes: a44f58c41bfb ("mlxsw: spectrum_qdisc: Support offloading of TBF Qdisc")
>Signed-off-by: Nathan Chancellor <[email protected]>

Acked-by: Jiri Pirko <[email protected]>

2020-01-31 16:56:26

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH v2] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps

On Thu, 30 Jan 2020 18:51:23 -0700, Nathan Chancellor wrote:
> When building arm32 allmodconfig:
>
> ERROR: "__aeabi_uldivmod"
> [drivers/net/ethernet/mellanox/mlxsw/mlxsw_spectrum.ko] undefined!
>
> rate_bytes_ps has type u64, we need to use a 64-bit division helper to
> avoid a build error.
>
> Fixes: a44f58c41bfb ("mlxsw: spectrum_qdisc: Support offloading of TBF Qdisc")
> Signed-off-by: Nathan Chancellor <[email protected]>

Applied, thank you.

2020-02-03 11:28:07

by Petr Machata

[permalink] [raw]
Subject: Re: [PATCH] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps


Nathan Chancellor <[email protected]> writes:

> On Thu, Jan 30, 2020 at 05:43:56PM -0800, Randy Dunlap wrote:
>> On 1/30/20 3:26 PM, Nathan Chancellor wrote:
>> > When building arm32 allmodconfig:
>> >
>> > ERROR: "__aeabi_uldivmod"
>> > [drivers/net/ethernet/mellanox/mlxsw/mlxsw_spectrum.ko] undefined!
>> >
>> > rate_bytes_ps has type u64, we need to use a 64-bit division helper to
>> > avoid a build error.
>> >
>> > Fixes: a44f58c41bfb ("mlxsw: spectrum_qdisc: Support offloading of TBF Qdisc")
>> > Signed-off-by: Nathan Chancellor <[email protected]>
>> > ---
>> > drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
>> > index 79a2801d59f6..65e681ef01e8 100644
>> > --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
>> > +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
>> > @@ -614,7 +614,7 @@ mlxsw_sp_qdisc_tbf_rate_kbps(struct tc_tbf_qopt_offload_replace_params *p)
>> > /* TBF interface is in bytes/s, whereas Spectrum ASIC is configured in
>> > * Kbits/s.
>> > */
>> > - return p->rate.rate_bytes_ps / 1000 * 8;
>> > + return div_u64(p->rate.rate_bytes_ps, 1000 * 8);
>>
>> not quite right AFAICT.
>>
>> try either
>> return div_u64(p->rate.rate_bytes_ps * 8, 1000);
>> or
>> return div_u64(p->rate.rate_bytes_ps, 1000) * 8;
>>
>
> Gah, I swear I can math... Thank you for catching this, v2 incoming with
> the later because I think it looks better.

Yes, that's the correct choice. Divide first, that way we can't
overflow.

Thanks for taking care of this.