Subject: Re: Inaccuracies with SCH_CLK_CPU

From: Patrick McHardy <[email-masked]>
Date: Sat, 07 Aug 2004 20:44:17 +0200

Stephen Hemminger wrote:

>I noticed that the netem delay values are off if using 
>CONFIG_NET_SCH_CKL_CPU. For example, if the latency is set to 500ms
> a ping reports the time as much less (343ms).
>
>Requested	JIFFIES	GETTIMEODAY	CPU
>0		.1	.1		.1
>1		2	2		1
>5		5	6		4
>10		10	11		7
>100		98	101		69
>500		489	500		343
>1000		976	1000		685
>
>The 1ms value is affected by the clock granularity.  Larger jiffie values
>are inaccurate due to the optimization of using shift to do divide (ie 1024 > 1000).
>
>With CLK_CPU /proc/net/psched is:
>000005d9 00000400 000f4240 000003e8
>  
>

sch_netem's interface is in us, but it doesn't convert the values to 
psched_us.
psched_us differ from us with CLK_CPU or CLK_JIFFIES for most values of HZ.
This patch fixes the problem, but the best solution is to change the 
interface
and pass the time values as psched_us from userspace, as done with 
rate-tables
and I think CBQ. I should have done this for HFSC too, but unlike for 
sch_netem,
I think it's too late to change the interface.

Regards
Patrick

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/08/07 20:32:26+02:00 [email-masked] 
#   [PKT_SCHED]: convert us values to psched_us in sch_netem
#   
#   Signed-off-by: Patrick McHardy <[email-masked]>
# 
# net/sched/sch_netem.c
#   2004/08/07 20:32:06+02:00 [email-masked] +23 -4
#   [PKT_SCHED]: convert us values to psched_us in sch_netem
# 
diff -Nru a/net/sched/sch_netem.c b/net/sched/sch_netem.c
--- a/net/sched/sch_netem.c	2004-08-07 20:40:30 +02:00
+++ b/net/sched/sch_netem.c	2004-08-07 20:40:30 +02:00
@@ -752,6 +752,25 @@
 	return ret;
 }
 
+static inline u_int32_t us2psched_us(u_int32_t us)
+{
+	u64 t;
+	
+	t = ((u64)us * PSCHED_JIFFIE2US(HZ));
+	t += 1000000 - 1;
+	do_div(t, 1000000);
+	return t;
+}
+
+static inline u_int32_t psched_us2us(u_int32_t psched_us)
+{
+	u64 t;
+
+	t = ((u64)psched_us * 1000000);
+	do_div(t, PSCHED_JIFFIE2US(HZ));
+	return t;
+}
+
 static int netem_change(struct Qdisc *sch, struct rtattr *opt)
 {
 	struct netem_sched_data *q = (struct netem_sched_data *)sch->data;
@@ -778,8 +797,8 @@
 		if (child != &noop_qdisc)
 			qdisc_destroy(child);
 	
-		q->latency = qopt->latency;
-		q->jitter = qopt->jitter;
+		q->latency = us2psched_us(qopt->latency);
+		q->jitter = us2psched_us(qopt->jitter);
 		q->limit = qopt->limit;
 		q->gap = qopt->gap;
 		q->loss = qopt->loss;
@@ -821,8 +840,8 @@
 	unsigned char	 *b = skb->tail;
 	struct tc_netem_qopt qopt;
 
-	qopt.latency = q->latency;
-	qopt.jitter = q->jitter;
+	qopt.latency = psched_us2us(q->latency);
+	qopt.jitter = psched_us2us(q->jitter);
 	qopt.limit = sch->dev->tx_queue_len;
 	qopt.loss = q->loss;
 	qopt.gap = q->gap;