Inaccuracies with SCH_CLK_CPU

Inaccuracies with SCH_CLK_CPU

[Stephen Hemminger]

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


Instead of computing own values of psched_us_per_tick
and psched_clock_per_hz, couldn't the values already done
in timers code be used?  

Also doesn't PSCHED_US2JIFFIE need to be doing a do_div()
in order to correctly do u64 divided by u32. Otherwise
it ends up just doing an u32 divide.

Re: Inaccuracies with SCH_CLK_CPU

[Patrick McHardy]

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).
>
I need to look at this some more.

>
>With CLK_CPU /proc/net/psched is:
>000005d9 00000400 000f4240 000003e8
>
>
>Instead of computing own values of psched_us_per_tick
>and psched_clock_per_hz, couldn't the values already done
>in timers code be used?
>
Can we do this in a uniform way on all architectures supported by
NET_SCH_CLK_CPU ? I think it's easier to fix up the PSCHED_* macros.

>
>Also doesn't PSCHED_US2JIFFIE need to be doing a do_div()
>in order to correctly do u64 divided by u32. Otherwise
>it ends up just doing an u32 divide.
>
No, PSCHED_US2JIFFIE is intended to be used with psched_tdiff_t, using
psched_time_t doesn't work with NET_SCH_CLK_GETTIMEOFDAY. psched_tdiff_t
is 32-bit on all 32-bit architectures.

Regards
Patrick

Re: Inaccuracies with SCH_CLK_CPU

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 1052 bytes --]

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


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1662 bytes --]

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/08/07 20:32:26+02:00 kaber@coreworks.de 
#   [PKT_SCHED]: convert us values to psched_us in sch_netem
#   
#   Signed-off-by: Patrick McHardy <kaber@trash.net>
# 
# net/sched/sch_netem.c
#   2004/08/07 20:32:06+02:00 kaber@coreworks.de +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;

Re: Inaccuracies with SCH_CLK_CPU

[jamal]

On Sat, 2004-08-07 at 14:44, Patrick McHardy wrote:

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

Nice catch Patrick.
While you are doing that Stephen, can you also compute the probability
tables in user space? This allows me to take advantage of the schem
later on when i convert gact action which has randomness built in.

> I should have done this for HFSC too, but unlike for 
> sch_netem, I think it's too late to change the interface.

Maybe post 2.6?

cheers,
jamal

Re: Inaccuracies with SCH_CLK_CPU

[Stephen Hemminger]

Thanks, I decided easier and more consistent to handle this in the command line.