[PATCH 0/7] Final ADSL-optimizer patch series

[PATCH 0/7] Final ADSL-optimizer patch series

[Jesper Dangaard Brouer]

This is the final part of the ADSL-optimizer patch, which is the real
"holy grail" of packet scheduling on ADSL/ATM lines. (Referring to
Wondershaper claim of the "holy grail".)

Shaping on ADSL has always been surrounded with mystique.  People
reduce and tweek the upstream bandwidth, but how much and why does it
not work all the time?  With this change, the tweeking and bandwidth
waste is gone, simply specify the bandwidth you bought.

This patch series introduces a parameter called "linklayer", which
currently supports "ethernet" and "atm".  Simply, what happens is,
that the rate table is aligned for ATM cells.

This is a general implementation for all shapers, except HFSC which
does not use rate table lookups.  Earlier (around Sep.2007), Patrick
McHardy wanted to make an even more general patch, that also included
HFSC. Nothing has happened since...

After talking with DaveM (at his house during an icehocky match), I've
come to the conclusion that we have something that works now (and has
been since Oct.2004) and we should use it!  Everybody is allowed to
change and improve upon that. Its should not mean that we keep
something like this back, which will allow packet scheduling to
actually work on ADSL.  One should also realize that different shapers
have different properties.

Patrick's further improvements can use the same userspace parameter
"linklayer" to allow userspace parameter compatibility.

This patch series is ABI (Application Binary Interface) compatible.

See you around,
   Jesper Brouer

--
-------------------------------------------------------------------
MSc. Master of Computer Science
Dept. of Computer Science, University of Copenhagen
Author of http://www.adsl-optimizer.dk
-------------------------------------------------------------------

[PATCH 1/7] In police, fix uninitialized "overhead" variable.

[Jesper Dangaard Brouer]

Bug introduced by myself in an earlier patch series.

Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
---
  tc/m_police.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tc/m_police.c b/tc/m_police.c
index 8fa63ad..e55a8c9 100644
--- a/tc/m_police.c
+++ b/tc/m_police.c
@@ -133,7 +133,7 @@ int act_parse_police(struct action_util *a,int *argc_p, char ***argv_p, int tca_
  	__u32 avrate = 0;
  	int presult = 0;
  	unsigned buffer=0, mtu=0, mpu=0;
-	unsigned short overhead;
+	unsigned short overhead=0;
  	int Rcell_log=-1, Pcell_log = -1;
  	struct rtattr *tail;

-- 
1.5.3

[PATCH 2/7] ATM cell alignment.

[Jesper Dangaard Brouer]

Introducing the function that does the ATM cell alignment, and
modifying tc_calc_rtable() to use this based upon a linklayer
parameter.

Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
---
  tc/tc_core.c |   36 +++++++++++++++++++++++++++++++++++-
  tc/tc_core.h |    6 +++++-
  2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/tc/tc_core.c b/tc/tc_core.c
index 9ae4d8e..a9b8076 100644
--- a/tc/tc_core.c
+++ b/tc/tc_core.c
@@ -65,11 +65,35 @@ unsigned tc_calc_xmitsize(unsigned rate, unsigned ticks)
  	return ((double)rate*tc_core_tick2time(ticks))/TIME_UNITS_PER_SEC;
  }

+#define ATM_CELL_SIZE               53  /* ATM cell size incl. header */
+#define ATM_CELL_PAYLOAD            48  /* ATM payload size */
+/*
+ The align to ATM cells is used for determining the (ATM) SAR
+ alignment overhead at the ATM layer. (SAR = Segmentation And
+ Reassembly).  This is for example needed when scheduling packet on an
+ ADSL connection.  Note that the extra ATM-AAL overhead is _not_
+ included in this calculation. This overhead is added in the kernel
+ before doing the rate table lookup, as this gives better precision
+ (as the table will always be aligned for 48 bytes).
+  --Hawk, d.7/11-2004. <hawk@diku.dk>
+ */
+unsigned tc_align_to_atm(unsigned size)
+{
+	int linksize, cells;
+	cells = size / ATM_CELL_PAYLOAD;
+	if ((size % ATM_CELL_PAYLOAD) > 0) {
+		cells++;
+	}
+	linksize = cells * ATM_CELL_SIZE; /* Use full cell size to add ATM tax */
+	return linksize;
+}
+
  /*
     rtab[pkt_len>>cell_log] = pkt_xmit_time
   */

-int tc_calc_rtable(struct tc_ratespec *r, __u32 *rtab, int cell_log, unsigned mtu)
+int tc_calc_rtable(struct tc_ratespec *r, __u32 *rtab, int cell_log, unsigned mtu,
+		   unsigned int linklayer)
  {
  	int i;
  	unsigned bps = r->rate;
@@ -87,6 +111,16 @@ int tc_calc_rtable(struct tc_ratespec *r, __u32 *rtab, int cell_log, unsigned mt
  		unsigned sz = ((i+1)<<cell_log);
  		if (sz < mpu)
  			sz = mpu;
+		switch (linklayer) {
+		case LINKLAYER_ATM:
+			sz = tc_align_to_atm(sz);
+			break;
+		case LINKLAYER_ETHERNET:
+			// No size adjustments on Ethernet
+			break;
+		default:
+			break;
+		}
  		rtab[i] = tc_calc_xmittime(bps, sz);
  	}
  	r->cell_align=-1; // Due to the sz calc
diff --git a/tc/tc_core.h b/tc/tc_core.h
index 080a0cd..7b2eaa0 100644
--- a/tc/tc_core.h
+++ b/tc/tc_core.h
@@ -6,6 +6,9 @@

  #define TIME_UNITS_PER_SEC	1000000

+#define LINKLAYER_ETHERNET 1
+#define LINKLAYER_ATM      2
+
  int  tc_core_time2big(unsigned time);
  unsigned tc_core_time2tick(unsigned time);
  unsigned tc_core_tick2time(unsigned tick);
@@ -13,7 +16,8 @@ unsigned tc_core_time2ktime(unsigned time);
  unsigned tc_core_ktime2time(unsigned ktime);
  unsigned tc_calc_xmittime(unsigned rate, unsigned size);
  unsigned tc_calc_xmitsize(unsigned rate, unsigned ticks);
-int tc_calc_rtable(struct tc_ratespec *r, __u32 *rtab, int cell_log, unsigned mtu);
+int tc_calc_rtable(struct tc_ratespec *r, __u32 *rtab, int cell_log, unsigned mtu,
+		   unsigned int linklayer);

  int tc_setup_estimator(unsigned A, unsigned time_const, struct tc_estimator *est);

-- 
1.5.3

Re: [PATCH 2/7] ATM cell alignment.

[Chas Williams (CONTRACTOR)]

In message <Pine.LNX.4.64.0804092259460.21022@ask.diku.dk>,Jesper Dangaard Brou
er writes:
>+#define ATM_CELL_SIZE               53  /* ATM cell size incl. header */
>+#define ATM_CELL_PAYLOAD            48  /* ATM payload size */

these constants are already in linux/atm.h

Re: [PATCH 2/7] ATM cell alignment.

[Jesper Dangaard Brouer]

On Thu, 10 Apr 2008, Chas Williams (CONTRACTOR) wrote:

> In message <Pine.LNX.4.64.0804092259460.21022@ask.diku.dk>,Jesper Dangaard Brou
> er writes:
>> +#define ATM_CELL_SIZE               53  /* ATM cell size incl. header */
>> +#define ATM_CELL_PAYLOAD            48  /* ATM payload size */
>
> these constants are already in linux/atm.h

Yes, I remember.  My original patch also contained some #ifndef's to 
handle that case.  I cannot remember, but there was some reason why I 
needed those, something about the ATM part of TC not always being 
included.

+#ifndef ATM_CELL_SIZE
+#define ATM_CELL_SIZE               53  /* ATM cell size incl. header */
+#endif
+#ifndef ATM_CELL_PAYLOAD
+#define ATM_CELL_PAYLOAD            48  /* ATM payload size */
+#endif

Hilsen
   Jesper Brouer

--
-------------------------------------------------------------------
MSc. Master of Computer Science
Dept. of Computer Science, University of Copenhagen
Author of http://www.adsl-optimizer.dk
-------------------------------------------------------------------

Re: [PATCH 2/7] ATM cell alignment.

[Chas Williams (CONTRACTOR)]

In message <Pine.LNX.4.64.0804110650390.16547@ask.diku.dk>,Jesper Dangaard Brou
er writes:
>Yes, I remember.  My original patch also contained some #ifndef's to 
>handle that case.  I cannot remember, but there was some reason why I 
>needed those, something about the ATM part of TC not always being 
>included.

then perhaps just include linux/atm.h in this file. instead of
creating ifdef swiss cheese.

Re: [PATCH 2/7] ATM cell alignment.

[Patrick McHardy]

Jesper Dangaard Brouer wrote:
> @@ -87,6 +111,16 @@ int tc_calc_rtable(struct tc_ratespec *r, __u32 
> *rtab, int cell_log, unsigned mt
>          unsigned sz = ((i+1)<<cell_log);
>          if (sz < mpu)
>              sz = mpu;
> +        switch (linklayer) {
> +        case LINKLAYER_ATM:
> +            sz = tc_align_to_atm(sz);
> +            break;
> +        case LINKLAYER_ETHERNET:
> +            // No size adjustments on Ethernet
> +            break;

Couldn't this use the ARPHRD values?

Re: [PATCH 2/7] ATM cell alignment.

[Jesper Dangaard Brouer]

On Fri, 11 Apr 2008, Patrick McHardy wrote:

> Jesper Dangaard Brouer wrote:
>> @@ -87,6 +111,16 @@ int tc_calc_rtable(struct tc_ratespec *r, __u32
>>  *rtab, int cell_log, unsigned mt
>>           unsigned sz = ((i+1)<<cell_log);
>>           if (sz < mpu)
>>               sz = mpu;
>>  +        switch (linklayer) {
>>  +        case LINKLAYER_ATM:
>>  +            sz = tc_align_to_atm(sz);
>>  +            break;
>>  +        case LINKLAYER_ETHERNET:
>>  +            // No size adjustments on Ethernet
>>  +            break;
>
> Couldn't this use the ARPHRD values?

It would make it more difficult to expand to new linklayer types.  For 
example I cannot find a define suitable for DOCSIS (cabel modems) (Thats 
next om my list to make DaveM happy ;-))

If we need to use the ARPHRD values (ARPHRD_ATM + ARPHDR_ETHER), I'm 
wondering which "if_arp.h" is the correct to include.

#include <linux/if_arp.h>
  or
#include <net/if_arp.h>

Hilsen
   Jesper Brouer

--
-------------------------------------------------------------------
MSc. Master of Computer Science
Dept. of Computer Science, University of Copenhagen
Author of http://www.adsl-optimizer.dk
-------------------------------------------------------------------

Re: [PATCH 2/7] ATM cell alignment.

[Patrick McHardy]

Jesper Dangaard Brouer wrote:
> On Fri, 11 Apr 2008, Patrick McHardy wrote:
> 
>> Jesper Dangaard Brouer wrote:
>>> @@ -87,6 +111,16 @@ int tc_calc_rtable(struct tc_ratespec *r, __u32
>>>  *rtab, int cell_log, unsigned mt
>>>           unsigned sz = ((i+1)<<cell_log);
>>>           if (sz < mpu)
>>>               sz = mpu;
>>>  +        switch (linklayer) {
>>>  +        case LINKLAYER_ATM:
>>>  +            sz = tc_align_to_atm(sz);
>>>  +            break;
>>>  +        case LINKLAYER_ETHERNET:
>>>  +            // No size adjustments on Ethernet
>>>  +            break;
>>
>> Couldn't this use the ARPHRD values?
> 
> It would make it more difficult to expand to new linklayer types.  For 
> example I cannot find a define suitable for DOCSIS (cabel modems) (Thats 
> next om my list to make DaveM happy ;-))

Fair enough, I guess ARPHRD is not the perfect match for this.

Re: [PATCH 2/7] ATM cell alignment.

[Andy Furniss]

Jesper Dangaard Brouer wrote:

> + The align to ATM cells is used for determining the (ATM) SAR
> + alignment overhead at the ATM layer. (SAR = Segmentation And
> + Reassembly).  This is for example needed when scheduling packet on an
> + ADSL connection.  Note that the extra ATM-AAL overhead is _not_
> + included in this calculation. This overhead is added in the kernel
> + before doing the rate table lookup, as this gives better precision
> + (as the table will always be aligned for 48 bytes).

I see overhead is unsigned short. For me using pppoa/vc mux my overhead 
is IP + 10. I am shaping on eth so skb->len is IP+14 hence I need a 
negative overhead.

Recently built a 2.6.25-rc7 and noticed the cell_align has been added 
and the tables jigged. I am (ab)using this at -5 now. Handy that I only 
need to patch TC rather than kernel and I guess I could use other TCs if 
I needed to shape other not atm ifs.

Russel Stuart's patches handled this case IIRC, has it been lost or have 
I missed something (as usual)?

Andy.

Re: [PATCH 2/7] ATM cell alignment.

[Jesper Dangaard Brouer]

On Sun, 13 Apr 2008, Andy Furniss wrote:

> Jesper Dangaard Brouer wrote:
>
>>  + The align to ATM cells is used for determining the (ATM) SAR
>>  + alignment overhead at the ATM layer. (SAR = Segmentation And
>>  + Reassembly).  This is for example needed when scheduling packet on an
>>  + ADSL connection.  Note that the extra ATM-AAL overhead is _not_
>>  + included in this calculation. This overhead is added in the kernel
>>  + before doing the rate table lookup, as this gives better precision
>>  + (as the table will always be aligned for 48 bytes).
>
> I see overhead is unsigned short. For me using pppoa/vc mux my overhead is IP 
> + 10. I am shaping on eth so skb->len is IP+14 hence I need a negative 
> overhead.

I'm not completely sure I understand how you end up with a negative 
overhead.  But I guess what you are saying, is that you need to remove the 
MAC header from the equation is it has already been added to skb->len (as 
you are doing routed and not bridged AAL5 encap).

That makes a good point for a _seperate_ patch (by you ;-)) where we 
change the overhead to be signed.  Or else you can do a userspace TC patch 
that abuse the cell_align, as you mentioned below, to express a negative 
overhead. (I'm trying to say, lets not mix these things... please!)


> Recently built a 2.6.25-rc7 and noticed the cell_align has been added and the 
> tables jigged.

Yes, the tables has been aligned to 2^n and avoids underestimation. 
Thus, with upto 2^4 (16) the table aligns to 48 bytes (ATM cell payload 
size), standard TC uses 2^3. (If I remember correctly you, did comment on 
the patch so you must have read it ;-)).


> I am (ab)using this at -5 now. Handy that I only need to patch TC rather 
> than kernel and I guess I could use other TCs if I needed to shape other 
> not atm ifs.

Yes, its nice that we have added this kind of flexibility to the kernel...


> Russel Stuart's patches handled this case IIRC, has it been lost or have I 
> missed something (as usual)?

I don't think so, but I'm not sure about that... I think it was way too 
complex, which will make it difficult to maintain in the future.

Cheers,
   Jesper Brouer

ps. I'm currently on a roadtrip down the west-coast of USA, so I only have 
periodic wifi coverage at different campgrounds...

--
-------------------------------------------------------------------
MSc. Master of Computer Science
Dept. of Computer Science, University of Copenhagen
Author of http://www.adsl-optimizer.dk
-------------------------------------------------------------------

Re: [PATCH 2/7] ATM cell alignment.

[Andy Furniss]

Jesper Dangaard Brouer wrote:
> On Sun, 13 Apr 2008, Andy Furniss wrote:

>> I see overhead is unsigned short. For me using pppoa/vc mux my 
>> overhead is IP + 10. I am shaping on eth so skb->len is IP+14 hence I 
>> need a negative overhead.
> 
> I'm not completely sure I understand how you end up with a negative 
> overhead.  But I guess what you are saying, is that you need to remove 
> the MAC header from the equation is it has already been added to 
> skb->len (as you are doing routed and not bridged AAL5 encap).

Yep I think there are only two cases that will hit this one.

> 
> That makes a good point for a _seperate_ patch (by you ;-)) where we 
> change the overhead to be signed.  Or else you can do a userspace TC 
> patch that abuse the cell_align, as you mentioned below, to express a 
> negative overhead. (I'm trying to say, lets not mix these things... 
> please!)

Fair enough, I suppose doing it with overhead could be more dangerous 
for accidental user misconfiguration than just in TC.

> 
> 
>> Recently built a 2.6.25-rc7 and noticed the cell_align has been added 
>> and the tables jigged.
> 
> Yes, the tables has been aligned to 2^n and avoids underestimation. 
> Thus, with upto 2^4 (16) the table aligns to 48 bytes (ATM cell payload 
> size), standard TC uses 2^3. (If I remember correctly you, did comment 
> on the patch so you must have read it ;-)).

Hmm did I - was that the post where I wanted overhead to be bigger for 
ingress shaping - what is it with me and overhead :-)

> 
> ps. I'm currently on a roadtrip down the west-coast of USA, so I only 
> have periodic wifi coverage at different campgrounds...

Have a good holiday.

Andy.

[PATCH 3/7] Parsing linklayer types.

[Jesper Dangaard Brouer]

Adding a general function get_linklayer() for parsing linklayer types.

Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
---
  tc/tc_util.c |   18 ++++++++++++++++++
  tc/tc_util.h |    2 ++
  2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/tc/tc_util.c b/tc/tc_util.c
index cdbae42..f8472c5 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -435,6 +435,24 @@ int action_a2n(char *arg, int *result)
  	return 0;
  }

+// Function for parsing linklayer type
+int get_linklayer(unsigned int *val, char *arg)
+{
+	int res;
+	if (matches(arg, "ethernet") == 0)
+		res = LINKLAYER_ETHERNET;
+	else if (matches(arg, "atm") == 0)
+		res = LINKLAYER_ATM;
+	else if (matches(arg, "adsl") == 0)
+		res = LINKLAYER_ATM;
+	else
+		return -1; /* Indicate error */
+
+	*val = res;
+	return 0;
+}
+
+
  void print_tm(FILE * f, const struct tcf_t *tm)
  {
  	int hz = get_user_hz();
diff --git a/tc/tc_util.h b/tc/tc_util.h
index 120d6ce..5b8253a 100644
--- a/tc/tc_util.h
+++ b/tc/tc_util.h
@@ -62,6 +62,8 @@ extern char * sprint_time(__u32 time, char *buf);
  extern char * sprint_ticks(__u32 ticks, char *buf);
  extern char * sprint_percent(__u32 percent, char *buf);

+extern int get_linklayer(unsigned int *val, char *arg);
+
  extern void print_tcstats_attr(FILE *fp, struct rtattr *tb[], char *prefix, struct rtattr **xstats);
  extern void print_tcstats2_attr(FILE *fp, struct rtattr *rta, char *prefix, struct rtattr **xstats);

-- 
1.5.3

[PATCH 4/7] Add linklayer parameter to filter action police.

[Jesper Dangaard Brouer]

Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
---
  tc/m_police.c |   12 +++++++++---
  1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/tc/m_police.c b/tc/m_police.c
index e55a8c9..6e4c7fa 100644
--- a/tc/m_police.c
+++ b/tc/m_police.c
@@ -36,7 +36,7 @@ static void usage(void)
  {
  	fprintf(stderr, "Usage: ... police rate BPS burst BYTES[/BYTES] [ mtu BYTES[/BYTES] ]\n");
  	fprintf(stderr, "                [ peakrate BPS ] [ avrate BPS ] [ overhead BYTES ]\n");
-	fprintf(stderr, "                [ ACTIONTERM ]\n");
+	fprintf(stderr, "                [ linklayer TYPE ] [ ACTIONTERM ]\n");
  	fprintf(stderr, "Old Syntax ACTIONTERM := action <EXCEEDACT>[/NOTEXCEEDACT] \n");
  	fprintf(stderr, "New Syntax ACTIONTERM := conform-exceed <EXCEEDACT>[/NOTEXCEEDACT] \n");
  	fprintf(stderr, "Where: *EXCEEDACT := pipe | ok | reclassify | drop | continue \n");
@@ -134,6 +134,7 @@ int act_parse_police(struct action_util *a,int *argc_p, char ***argv_p, int tca_
  	int presult = 0;
  	unsigned buffer=0, mtu=0, mpu=0;
  	unsigned short overhead=0;
+	unsigned int linklayer = LINKLAYER_ETHERNET; /* Assume ethernet */
  	int Rcell_log=-1, Pcell_log = -1;
  	struct rtattr *tail;

@@ -240,6 +241,11 @@ int act_parse_police(struct action_util *a,int *argc_p, char ***argv_p, int tca_
  			if (get_u16(&overhead, *argv, 10)) {
  				explain1("overhead"); return -1;
  			}
+		} else if (matches(*argv, "linklayer") == 0) {
+			NEXT_ARG();
+			if (get_linklayer(&linklayer, *argv)) {
+				explain1("linklayer"); return -1;
+			}
  		} else if (strcmp(*argv, "help") == 0) {
  			usage();
  		} else {
@@ -270,7 +276,7 @@ int act_parse_police(struct action_util *a,int *argc_p, char ***argv_p, int tca_
  	if (p.rate.rate) {
  		p.rate.mpu = mpu;
  		p.rate.overhead = overhead;
-		if (tc_calc_rtable(&p.rate, rtab, Rcell_log, mtu) < 0) {
+		if (tc_calc_rtable(&p.rate, rtab, Rcell_log, mtu, linklayer) < 0) {
  			fprintf(stderr, "TBF: failed to calculate rate table.\n");
  			return -1;
  		}
@@ -280,7 +286,7 @@ int act_parse_police(struct action_util *a,int *argc_p, char ***argv_p, int tca_
  	if (p.peakrate.rate) {
  		p.peakrate.mpu = mpu;
  		p.peakrate.overhead = overhead;
-		if (tc_calc_rtable(&p.peakrate, ptab, Pcell_log, mtu) < 0) {
+		if (tc_calc_rtable(&p.peakrate, ptab, Pcell_log, mtu, linklayer) < 0) {
  			fprintf(stderr, "POLICE: failed to calculate peak rate table.\n");
  			return -1;
  		}
-- 
1.5.3

[PATCH 5/7] Add linklayer parameter to CBQ.

[Jesper Dangaard Brouer]

Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
---
  tc/q_cbq.c |   18 +++++++++++++++---
  1 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/tc/q_cbq.c b/tc/q_cbq.c
index e53d167..c99dc3b 100644
--- a/tc/q_cbq.c
+++ b/tc/q_cbq.c
@@ -32,7 +32,7 @@ static void explain_class(void)
  	fprintf(stderr, "               [ prio NUMBER ] [ cell BYTES ] [ ewma LOG ]\n");
  	fprintf(stderr, "               [ estimator INTERVAL TIME_CONSTANT ]\n");
  	fprintf(stderr, "               [ split CLASSID ] [ defmap MASK/CHANGE ]\n");
-	fprintf(stderr, "               [ overhead BYTES ]\n");
+	fprintf(stderr, "               [ overhead BYTES ] [ linklayer TYPE ]\n");
  }

  static void explain(void)
@@ -55,6 +55,7 @@ static int cbq_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
  	__u32 rtab[256];
  	unsigned mpu=0, avpkt=0, allot=0;
  	unsigned short overhead=0;
+	unsigned int linklayer = LINKLAYER_ETHERNET; /* Assume ethernet */
  	int cell_log=-1;
  	int ewma_log=-1;
  	struct rtattr *tail;
@@ -120,6 +121,11 @@ static int cbq_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
  			if (get_u16(&overhead, *argv, 10)) {
  				explain1("overhead"); return -1;
  			}
+		} else if (matches(*argv, "linklayer") == 0) {
+			NEXT_ARG();
+			if (get_linklayer(&linklayer, *argv)) {
+				explain1("linklayer"); return -1;
+			}
  		} else if (matches(*argv, "help") == 0) {
  			explain();
  			return -1;
@@ -146,7 +152,7 @@ static int cbq_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl

  	r.mpu = mpu;
  	r.overhead = overhead;
-	if (tc_calc_rtable(&r, rtab, cell_log, allot) < 0) {
+	if (tc_calc_rtable(&r, rtab, cell_log, allot, linklayer) < 0) {
  		fprintf(stderr, "CBQ: failed to calculate rate table.\n");
  		return -1;
  	}
@@ -188,6 +194,7 @@ static int cbq_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
  	unsigned bndw = 0;
  	unsigned minburst=0, maxburst=0;
  	unsigned short overhead=0;
+	unsigned int linklayer = LINKLAYER_ETHERNET; /* Assume ethernet */
  	struct rtattr *tail;

  	memset(&r, 0, sizeof(r));
@@ -331,6 +338,11 @@ static int cbq_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
  			if (get_u16(&overhead, *argv, 10)) {
  				explain1("overhead"); return -1;
  			}
+		} else if (matches(*argv, "linklayer") == 0) {
+			NEXT_ARG();
+			if (get_linklayer(&linklayer, *argv)) {
+				explain1("linklayer"); return -1;
+			}
  		} else if (matches(*argv, "help") == 0) {
  			explain_class();
  			return -1;
@@ -351,7 +363,7 @@ static int cbq_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
  			wrr.allot = (lss.avpkt*3)/2;
  		r.mpu = mpu;
  		r.overhead = overhead;
-		if (tc_calc_rtable(&r, rtab, cell_log, pktsize) < 0) {
+		if (tc_calc_rtable(&r, rtab, cell_log, pktsize, linklayer) < 0) {
  			fprintf(stderr, "CBQ: failed to calculate rate table.\n");
  			return -1;
  		}
-- 
1.5.3

[PATCH 6/7] Add linklayer parameter to HTB.

[Jesper Dangaard Brouer]

Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
---
  tc/q_htb.c |   11 +++++++++--
  1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tc/q_htb.c b/tc/q_htb.c
index e24ad6d..7e85bf7 100644
--- a/tc/q_htb.c
+++ b/tc/q_htb.c
@@ -41,6 +41,7 @@ static void explain(void)
  		" burst    max bytes burst which can be accumulated during idle period {computed}\n"
  		" mpu      minimum packet size used in rate computations\n"
  		" overhead per-packet size overhead used in rate computations\n"
+		" linklay  adapting to a linklayer e.g. atm\n"

  		" ceil     definite upper class rate (no borrows) {rate}\n"
  		" cburst   burst but for ceil {computed}\n"
@@ -110,6 +111,7 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
  	unsigned mtu;
  	unsigned short mpu = 0;
  	unsigned short overhead = 0;
+	unsigned int linklayer  = LINKLAYER_ETHERNET; /* Assume ethernet */
  	struct rtattr *tail;

  	memset(&opt, 0, sizeof(opt)); mtu = 1600; /* eth packet len */
@@ -136,6 +138,11 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
  			if (get_u16(&overhead, *argv, 10)) {
  				explain1("overhead"); return -1;
  			}
+		} else if (matches(*argv, "linklayer") == 0) {
+			NEXT_ARG();
+			if (get_linklayer(&linklayer, *argv)) {
+				explain1("linklayer"); return -1;
+			}
  		} else if (matches(*argv, "quantum") == 0) {
  			NEXT_ARG();
  			if (get_u32(&opt.quantum, *argv, 10)) {
@@ -213,13 +220,13 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
  	opt.ceil.mpu = mpu;
  	opt.rate.mpu = mpu;

-	if (tc_calc_rtable(&opt.rate, rtab, cell_log, mtu) < 0) {
+	if (tc_calc_rtable(&opt.rate, rtab, cell_log, mtu, linklayer) < 0) {
  		fprintf(stderr, "htb: failed to calculate rate table.\n");
  		return -1;
  	}
  	opt.buffer = tc_calc_xmittime(opt.rate.rate, buffer);

-	if (tc_calc_rtable(&opt.ceil, ctab, ccell_log, mtu) < 0) {
+	if (tc_calc_rtable(&opt.ceil, ctab, ccell_log, mtu, linklayer) < 0) {
  		fprintf(stderr, "htb: failed to calculate ceil rate table.\n");
  		return -1;
  	}
-- 
1.5.3

[PATCH 7/7] Add linklayer parameter to TBF.

[Jesper Dangaard Brouer]

Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
---
  tc/q_tbf.c |   12 +++++++++---
  1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/tc/q_tbf.c b/tc/q_tbf.c
index 3bdce5a..dbf9586 100644
--- a/tc/q_tbf.c
+++ b/tc/q_tbf.c
@@ -27,7 +27,7 @@ static void explain(void)
  {
  	fprintf(stderr, "Usage: ... tbf limit BYTES burst BYTES[/BYTES] rate KBPS [ mtu BYTES[/BYTES] ]\n");
  	fprintf(stderr, "               [ peakrate KBPS ] [ latency TIME ] ");
-	fprintf(stderr, "[ overhead BYTES ]\n");
+	fprintf(stderr, "[ overhead BYTES ] [ linklayer TYPE ]\n");
  }

  static void explain1(char *arg)
@@ -47,6 +47,7 @@ static int tbf_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
  	unsigned buffer=0, mtu=0, mpu=0, latency=0;
  	int Rcell_log=-1, Pcell_log = -1;
  	unsigned short overhead=0;
+	unsigned int linklayer = LINKLAYER_ETHERNET; /* Assume ethernet */
  	struct rtattr *tail;

  	memset(&opt, 0, sizeof(opt));
@@ -141,6 +142,11 @@ static int tbf_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
  			if (get_u16(&overhead, *argv, 10)) {
  				explain1("overhead"); return -1;
  			}
+		} else if (matches(*argv, "linklayer") == 0) {
+			NEXT_ARG();
+			if (get_linklayer(&linklayer, *argv)) {
+				explain1("linklayer"); return -1;
+			}
  		} else if (strcmp(*argv, "help") == 0) {
  			explain();
  			return -1;
@@ -183,7 +189,7 @@ static int tbf_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl

  	opt.rate.mpu      = mpu;
  	opt.rate.overhead = overhead;
-	if (tc_calc_rtable(&opt.rate, rtab, Rcell_log, mtu) < 0) {
+	if (tc_calc_rtable(&opt.rate, rtab, Rcell_log, mtu, linklayer) < 0) {
  		fprintf(stderr, "TBF: failed to calculate rate table.\n");
  		return -1;
  	}
@@ -192,7 +198,7 @@ static int tbf_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
  	if (opt.peakrate.rate) {
  		opt.peakrate.mpu      = mpu;
  		opt.peakrate.overhead = overhead;
-		if (tc_calc_rtable(&opt.peakrate, ptab, Pcell_log, mtu) < 0) {
+		if (tc_calc_rtable(&opt.peakrate, ptab, Pcell_log, mtu, linklayer) < 0) {
  			fprintf(stderr, "TBF: failed to calculate peak rate table.\n");
  			return -1;
  		}
-- 
1.5.3

Re: [PATCH 0/7] Final ADSL-optimizer patch series

[David Miller]

From: Jesper Dangaard Brouer <hawk@diku.dk>
Date: Wed, 9 Apr 2008 22:57:25 +0200 (CEST)

> This is the final part of the ADSL-optimizer patch, which is the real
> "holy grail" of packet scheduling on ADSL/ATM lines. (Referring to
> Wondershaper claim of the "holy grail".)

I think, sans the very minor macro and header issues mentioned,
Jesper's changes should be added if they haven't been already.

They look totally fine to me, and this stuff does not preclude
doing things in the kernel in the future at all.

Thanks.

Re: [PATCH 0/7] Final ADSL-optimizer patch series

[Stephen Hemminger]

On Wed, 9 Apr 2008 22:57:25 +0200 (CEST)
Jesper Dangaard Brouer <hawk@diku.dk> wrote:

> 
> This is the final part of the ADSL-optimizer patch, which is the real
> "holy grail" of packet scheduling on ADSL/ATM lines. (Referring to
> Wondershaper claim of the "holy grail".)
> 
> Shaping on ADSL has always been surrounded with mystique.  People
> reduce and tweek the upstream bandwidth, but how much and why does it
> not work all the time?  With this change, the tweeking and bandwidth
> waste is gone, simply specify the bandwidth you bought.
> 
> This patch series introduces a parameter called "linklayer", which
> currently supports "ethernet" and "atm".  Simply, what happens is,
> that the rate table is aligned for ATM cells.
> 
> This is a general implementation for all shapers, except HFSC which
> does not use rate table lookups.  Earlier (around Sep.2007), Patrick
> McHardy wanted to make an even more general patch, that also included
> HFSC. Nothing has happened since...
> 
> After talking with DaveM (at his house during an icehocky match), I've
> come to the conclusion that we have something that works now (and has
> been since Oct.2004) and we should use it!  Everybody is allowed to
> change and improve upon that. Its should not mean that we keep
> something like this back, which will allow packet scheduling to
> actually work on ADSL.  One should also realize that different shapers
> have different properties.
> 
> Patrick's further improvements can use the same userspace parameter
> "linklayer" to allow userspace parameter compatibility.
> 
> This patch series is ABI (Application Binary Interface) compatible.
> 
> See you around,
>    Jesper Brouer

Applied but:
* Used constants from atm.h, pulled in include/linux/atm.h etc.

* rolled 2-7 together in one patch.  All though small patches are preferred
  it is more important that the build doesn't break after each patch so
  if an API changes (in this case calc_rtable) fix all the functions that
  use it in one patch.

* Some mailer damage, so the patches would not apply automatically, had
  to do hand edits.