| Message ID | 20171114172859.27930-1-sthemmin@microsoft.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | netem: fix 64 bit divide | expand |
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; >
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.
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;
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(-)