netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] netem: fix 64 bit divide
@ 2017-11-14 17:28 Stephen Hemminger
  2017-11-14 17:57 ` Randy Dunlap
  0 siblings, 1 reply; 3+ messages in thread
From: Stephen Hemminger @ 2017-11-14 17:28 UTC (permalink / raw)
  To: davem, dave.taht, rdunlap; +Cc: netdev, Stephen Hemminger, Stephen Hemminger

Since times are now expressed in 64 bit nanosecond, need to now do
true 64 bit divide. Change the name of the function to bette express
the new units.

Fixes: 99803171ef04 ("netem: add uapi to express delay and jitter in nanoseconds")
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 net/sched/sch_netem.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index b686e755fda9..644323d6081c 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -339,10 +339,8 @@ static s64 tabledist(s64 mu, s64 sigma,
 	return  x / NETEM_DIST_SCALE + (sigma / NETEM_DIST_SCALE) * t + mu;
 }
 
-static u64 packet_len_2_sched_time(unsigned int len,
-				   struct netem_sched_data *q)
+static u64 packet_time_ns(u64 len, const struct netem_sched_data *q)
 {
-	u64 offset;
 	len += q->packet_overhead;
 
 	if (q->cell_size) {
@@ -352,9 +350,8 @@ static u64 packet_len_2_sched_time(unsigned int len,
 			cells++;
 		len = cells * (q->cell_size + q->cell_overhead);
 	}
-	offset = (u64)len * NSEC_PER_SEC;
-	do_div(offset, q->rate);
-	return offset;
+
+	return div64_u64(len * NSEC_PER_SEC, q->rate);
 }
 
 static void tfifo_reset(struct Qdisc *sch)
@@ -556,7 +553,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 				now = last->time_to_send;
 			}
 
-			delay += packet_len_2_sched_time(qdisc_pkt_len(skb), q);
+			delay += packet_time_ns(qdisc_pkt_len(skb), q);
 		}
 
 		cb->time_to_send = now + delay;
-- 
2.11.0

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

* Re: [PATCH] netem: fix 64 bit divide
  2017-11-14 17:28 [PATCH] netem: fix 64 bit divide Stephen Hemminger
@ 2017-11-14 17:57 ` Randy Dunlap
  2017-11-14 19:09   ` Stephen Hemminger
  0 siblings, 1 reply; 3+ messages in thread
From: Randy Dunlap @ 2017-11-14 17:57 UTC (permalink / raw)
  To: Stephen Hemminger, davem, dave.taht; +Cc: netdev, Stephen Hemminger

On 11/14/2017 09:28 AM, Stephen Hemminger wrote:
> Since times are now expressed in 64 bit nanosecond, need to now do
> true 64 bit divide. Change the name of the function to bette express
> the new units.
> 
> Fixes: 99803171ef04 ("netem: add uapi to express delay and jitter in nanoseconds")
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Hi Stephen,

I still get it. Maybe it's this % operator:

		skb->data[prandom_u32() % skb_headlen(skb)] ^=
			1<<(prandom_u32() % 8);

in net_enqueue().

> ---
>  net/sched/sch_netem.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
> index b686e755fda9..644323d6081c 100644
> --- a/net/sched/sch_netem.c
> +++ b/net/sched/sch_netem.c
> @@ -339,10 +339,8 @@ static s64 tabledist(s64 mu, s64 sigma,
>  	return  x / NETEM_DIST_SCALE + (sigma / NETEM_DIST_SCALE) * t + mu;
>  }
>  
> -static u64 packet_len_2_sched_time(unsigned int len,
> -				   struct netem_sched_data *q)
> +static u64 packet_time_ns(u64 len, const struct netem_sched_data *q)
>  {
> -	u64 offset;
>  	len += q->packet_overhead;
>  
>  	if (q->cell_size) {
> @@ -352,9 +350,8 @@ static u64 packet_len_2_sched_time(unsigned int len,
>  			cells++;
>  		len = cells * (q->cell_size + q->cell_overhead);
>  	}
> -	offset = (u64)len * NSEC_PER_SEC;
> -	do_div(offset, q->rate);
> -	return offset;
> +
> +	return div64_u64(len * NSEC_PER_SEC, q->rate);
>  }
>  
>  static void tfifo_reset(struct Qdisc *sch)
> @@ -556,7 +553,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
>  				now = last->time_to_send;
>  			}
>  
> -			delay += packet_len_2_sched_time(qdisc_pkt_len(skb), q);
> +			delay += packet_time_ns(qdisc_pkt_len(skb), q);
>  		}
>  
>  		cb->time_to_send = now + delay;
> 


-- 
~Randy

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

* Re: [PATCH] netem: fix 64 bit divide
  2017-11-14 17:57 ` Randy Dunlap
@ 2017-11-14 19:09   ` Stephen Hemminger
  0 siblings, 0 replies; 3+ messages in thread
From: Stephen Hemminger @ 2017-11-14 19:09 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: davem, dave.taht, netdev, Stephen Hemminger

On Tue, 14 Nov 2017 09:57:11 -0800
Randy Dunlap <rdunlap@infradead.org> wrote:

> On 11/14/2017 09:28 AM, Stephen Hemminger wrote:
> > Since times are now expressed in 64 bit nanosecond, need to now do
> > true 64 bit divide. Change the name of the function to bette express
> > the new units.
> > 
> > Fixes: 99803171ef04 ("netem: add uapi to express delay and jitter in nanoseconds")
> > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>  
> 
> Hi Stephen,
> 
> I still get it. Maybe it's this % operator:
> 
> 		skb->data[prandom_u32() % skb_headlen(skb)] ^=
> 			1<<(prandom_u32() % 8);
> 
> in net_enqueue().
> 
> > ---
> >  net/sched/sch_netem.c | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> > 
> > diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
> > index b686e755fda9..644323d6081c 100644
> > --- a/net/sched/sch_netem.c
> > +++ b/net/sched/sch_netem.c
> > @@ -339,10 +339,8 @@ static s64 tabledist(s64 mu, s64 sigma,
> >  	return  x / NETEM_DIST_SCALE + (sigma / NETEM_DIST_SCALE) * t + mu;
> >  }
> >  
> > -static u64 packet_len_2_sched_time(unsigned int len,
> > -				   struct netem_sched_data *q)
> > +static u64 packet_time_ns(u64 len, const struct netem_sched_data *q)
> >  {
> > -	u64 offset;
> >  	len += q->packet_overhead;
> >  
> >  	if (q->cell_size) {
> > @@ -352,9 +350,8 @@ static u64 packet_len_2_sched_time(unsigned int len,
> >  			cells++;
> >  		len = cells * (q->cell_size + q->cell_overhead);
> >  	}
> > -	offset = (u64)len * NSEC_PER_SEC;
> > -	do_div(offset, q->rate);
> > -	return offset;
> > +
> > +	return div64_u64(len * NSEC_PER_SEC, q->rate);
> >  }
> >  
> >  static void tfifo_reset(struct Qdisc *sch)
> > @@ -556,7 +553,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
> >  				now = last->time_to_send;
> >  			}
> >  
> > -			delay += packet_len_2_sched_time(qdisc_pkt_len(skb), q);
> > +			delay += packet_time_ns(qdisc_pkt_len(skb), q);
> >  		}
> >  
> >  		cb->time_to_send = now + delay;
> >   
> 
> 

Close, the problem is here:
		return (rnd % (2*sigma)) - sigma + mu;
    139d:	8b 4d dc             	mov    -0x24(%ebp),%ecx
    13a0:	8b 5d e0             	mov    -0x20(%ebp),%ebx
    13a3:	0f a4 cb 01          	shld   $0x1,%ecx,%ebx
    13a7:	01 c9                	add    %ecx,%ecx
    13a9:	53                   	push   %ebx
    13aa:	51                   	push   %ecx
    13ab:	e8 fc ff ff ff       	call   13ac <netem_enqueue+0x9dc>
			13ac: R_386_PC32	__moddi3

Will add second patch for that.

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

end of thread, other threads:[~2017-11-14 19:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-14 17:28 [PATCH] netem: fix 64 bit divide Stephen Hemminger
2017-11-14 17:57 ` Randy Dunlap
2017-11-14 19:09   ` Stephen Hemminger

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).