netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps
@ 2020-01-30 23:26 Nathan Chancellor
  2020-01-31  1:43 ` Randy Dunlap
  2020-01-31  1:51 ` [PATCH v2] " Nathan Chancellor
  0 siblings, 2 replies; 8+ messages in thread
From: Nathan Chancellor @ 2020-01-30 23:26 UTC (permalink / raw)
  To: Jiri Pirko, Ido Schimmel, David S. Miller
  Cc: Petr Machata, netdev, linux-kernel, Nathan Chancellor

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 <natechancellor@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps
  2020-01-30 23:26 [PATCH] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps Nathan Chancellor
@ 2020-01-31  1:43 ` Randy Dunlap
  2020-01-31  1:48   ` Nathan Chancellor
  2020-01-31  1:51 ` [PATCH v2] " Nathan Chancellor
  1 sibling, 1 reply; 8+ messages in thread
From: Randy Dunlap @ 2020-01-31  1:43 UTC (permalink / raw)
  To: Nathan Chancellor, Jiri Pirko, Ido Schimmel, David S. Miller
  Cc: Petr Machata, netdev, linux-kernel

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 <natechancellor@gmail.com>
> ---
>  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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps
  2020-01-31  1:43 ` Randy Dunlap
@ 2020-01-31  1:48   ` Nathan Chancellor
  2020-02-03 10:03     ` Petr Machata
  0 siblings, 1 reply; 8+ messages in thread
From: Nathan Chancellor @ 2020-01-31  1:48 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Jiri Pirko, Ido Schimmel, David S. Miller, Petr Machata, netdev,
	linux-kernel

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 <natechancellor@gmail.com>
> > ---
> >  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
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps
  2020-01-30 23:26 [PATCH] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps Nathan Chancellor
  2020-01-31  1:43 ` Randy Dunlap
@ 2020-01-31  1:51 ` Nathan Chancellor
  2020-01-31 13:47   ` Ido Schimmel
                     ` (2 more replies)
  1 sibling, 3 replies; 8+ messages in thread
From: Nathan Chancellor @ 2020-01-31  1:51 UTC (permalink / raw)
  To: Jiri Pirko, Ido Schimmel, David S. Miller
  Cc: Petr Machata, netdev, linux-kernel, Randy Dunlap, Nathan Chancellor

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 <natechancellor@gmail.com>
---

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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps
  2020-01-31  1:51 ` [PATCH v2] " Nathan Chancellor
@ 2020-01-31 13:47   ` Ido Schimmel
  2020-01-31 14:27   ` Jiri Pirko
  2020-01-31 16:55   ` Jakub Kicinski
  2 siblings, 0 replies; 8+ messages in thread
From: Ido Schimmel @ 2020-01-31 13:47 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Jiri Pirko, David S. Miller, Petr Machata, netdev, linux-kernel,
	Randy Dunlap

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 <natechancellor@gmail.com>

Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Tested-by: Ido Schimmel <idosch@mellanox.com>

Thanks, Nathan.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps
  2020-01-31  1:51 ` [PATCH v2] " Nathan Chancellor
  2020-01-31 13:47   ` Ido Schimmel
@ 2020-01-31 14:27   ` Jiri Pirko
  2020-01-31 16:55   ` Jakub Kicinski
  2 siblings, 0 replies; 8+ messages in thread
From: Jiri Pirko @ 2020-01-31 14:27 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Jiri Pirko, Ido Schimmel, David S. Miller, Petr Machata, netdev,
	linux-kernel, Randy Dunlap

Fri, Jan 31, 2020 at 02:51:23AM CET, natechancellor@gmail.com 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 <natechancellor@gmail.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps
  2020-01-31  1:51 ` [PATCH v2] " Nathan Chancellor
  2020-01-31 13:47   ` Ido Schimmel
  2020-01-31 14:27   ` Jiri Pirko
@ 2020-01-31 16:55   ` Jakub Kicinski
  2 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2020-01-31 16:55 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Jiri Pirko, Ido Schimmel, David S. Miller, Petr Machata, netdev,
	linux-kernel, Randy Dunlap

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 <natechancellor@gmail.com>

Applied, thank you.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps
  2020-01-31  1:48   ` Nathan Chancellor
@ 2020-02-03 10:03     ` Petr Machata
  0 siblings, 0 replies; 8+ messages in thread
From: Petr Machata @ 2020-02-03 10:03 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Randy Dunlap, Jiri Pirko, Ido Schimmel, David S. Miller, netdev,
	linux-kernel


Nathan Chancellor <natechancellor@gmail.com> 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 <natechancellor@gmail.com>
>> > ---
>> >  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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-02-03 10:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-30 23:26 [PATCH] mlxsw: spectrum_qdisc: Fix 64-bit division error in mlxsw_sp_qdisc_tbf_rate_kbps Nathan Chancellor
2020-01-31  1:43 ` Randy Dunlap
2020-01-31  1:48   ` Nathan Chancellor
2020-02-03 10:03     ` Petr Machata
2020-01-31  1:51 ` [PATCH v2] " Nathan Chancellor
2020-01-31 13:47   ` Ido Schimmel
2020-01-31 14:27   ` Jiri Pirko
2020-01-31 16:55   ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).