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
commit a29d43b78d5ddb20a808d9270c2de358556ba5ee Author: Jesper Dangaard Brouer <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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; };
commit 1e89442468dca983e0e3d24b89093370896f2d6a Author: Jesper Dangaard Brouer <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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);