Re: [PATCH 2/2]: [NET_SCHED]: Making rate table lookups more flexible.

[Jesper Dangaard Brouer <jdb@comx.dk>]

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


On Sun, 2007-09-02 at 23:16 +0200, Patrick McHardy wrote:
> Jesper Dangaard Brouer wrote:
> > On Sun, 2 Sep 2007, Patrick McHardy wrote:
> >>
> >
> > Lets focus on the general case, where the functionality actually is 
> > needed right away.
> >
> > In the general case:
> >
> > - The rate table needs to be aligned (cell_align=-1).
> >   (currently, we miscalculates up to 7 bytes on every lookup)
> 
> We will always do that, thats a consequence of storing the
> transmission times for multiples of 8b.

The issue is that we use the lower boundary for calculating the transmit
cost. Thus, a 15 bytes packet only have a transmit cost of 8 bytes.

> > - The existing tc overhead calc can be made more accurate.
> >   (by adding overhead before doing the lookup, instead of the
> >    current solution where the rate table is modified with its
> >    limited resolution)
> 
> Please demonstrate this with patches (one for the overhead
> calculation, one for the cell_align thing), then we can
> continue this discussion.

I have attached a patch for the overhead calculation.

I'll look into the "the cell_align thing" tomorrow.

-- 
Med venlig hilsen / Best regards
  Jesper Brouer
  ComX Networks A/S
  Linux Network developer
  Cand. Scient Datalog / MSc.
  Author of http://adsl-optimizer.dk

[-- Attachment #2: pkt_sched.h.patch --]
[-- Type: text/x-patch, Size: 721 bytes --]

commit a29d43b78d5ddb20a808d9270c2de358556ba5ee
Author: Jesper Dangaard Brouer <hawk@comx.dk>
Date:   Mon Sep 3 14:35:19 2007 +0200

    [IPROUTE2]: Update pkt_sched.h
    
    In struct tc_ratespec, replace 'addend', with 'cell_align' and 'overhead'.
    
    Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>

diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 268c515..a127d63 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -78,7 +78,8 @@ struct tc_ratespec
 	unsigned char	cell_log;
 	unsigned char	__reserved;
 	unsigned short	feature;
-	short		addend;
+	char		cell_align;
+	unsigned char	overhead;
 	unsigned short	mpu;
 	__u32		rate;
 };

[-- Attachment #3: overhead_to_kernel.patch --]
[-- Type: text/x-patch, Size: 1754 bytes --]

commit 1e89442468dca983e0e3d24b89093370896f2d6a
Author: Jesper Dangaard Brouer <hawk@comx.dk>
Date:   Mon Sep 3 15:49:41 2007 +0200

    [IPROUTE2]: Overhead calculation is now done in the kernel.
    
    The only current user is HTB. HTB overhead argument is now
    passed on the kernel (in the struct tc_ratespec).
    
    Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>

diff --git a/tc/q_htb.c b/tc/q_htb.c
index 53e3f78..b579ebe 100644
--- a/tc/q_htb.c
+++ b/tc/q_htb.c
@@ -206,9 +206,11 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
 	if (!buffer) buffer = opt.rate.rate / get_hz() + mtu;
 	if (!cbuffer) cbuffer = opt.ceil.rate / get_hz() + mtu;
 
-/* encode overhead and mpu, 8 bits each, into lower 16 bits */
-	mpu = (unsigned)mpu8 | (unsigned)overhead << 8;
-	opt.ceil.mpu = mpu; opt.rate.mpu = mpu;
+	opt.ceil.overhead = overhead;
+	opt.rate.overhead = overhead;
+
+	opt.ceil.mpu = mpu;
+	opt.rate.mpu = mpu;
 
 	if ((cell_log = tc_calc_rtable(opt.rate.rate, rtab, cell_log, mtu, mpu)) < 0) {
 		fprintf(stderr, "htb: failed to calculate rate table.\n");
diff --git a/tc/tc_core.c b/tc/tc_core.c
index 58155fb..1ab0ba0 100644
--- a/tc/tc_core.c
+++ b/tc/tc_core.c
@@ -73,8 +73,6 @@ int tc_calc_rtable(unsigned bps, __u32 *rtab, int cell_log, unsigned mtu,
 		   unsigned mpu)
 {
 	int i;
-	unsigned overhead = (mpu >> 8) & 0xFF;
-	mpu = mpu & 0xFF;
 
 	if (mtu == 0)
 		mtu = 2047;
@@ -86,8 +84,6 @@ int tc_calc_rtable(unsigned bps, __u32 *rtab, int cell_log, unsigned mtu,
 	}
 	for (i=0; i<256; i++) {
 		unsigned sz = (i<<cell_log);
-		if (overhead)
-			sz += overhead;
 		if (sz < mpu)
 			sz = mpu;
 		rtab[i] = tc_calc_xmittime(bps, sz);