login
Search
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
-------------------------------------------------------------------
--
On Wed, 9 Apr 2008 22:57:25 +0200 (CEST)
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.--
From: Jesper Dangaard Brouer <hawk@diku.dk>
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.
--
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 nlopt.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...
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 (...
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 nlr.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;
unsign...
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 ...
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--
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 = mp...
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.
--
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 negativeYes, 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 onI 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 Brouerps. 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
-------------------------------------------------------------------
--
Fair enough, I suppose doing it with overhead could be more dangerous
Hmm did I - was that the post where I wanted overhead to be bigger for
Have a good holiday.
Andy.
--
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
-------------------------------------------------------------------
--
Fair enough, I guess ARPHRD is not the perfect match for this.
--
In message <Pine.LNX.4.64.0804092259460.21022@ask.diku.dk>,Jesper Dangaard Brou
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 */
+#endifHilsen
Jesper Brouer--
-------------------------------------------------------------------
MSc. Master of Computer Science
Dept. of Computer Science, University of Copenhagen
Author of http://www.adsl-optimizer.dk
-------------------------------------------------------------------
--
In message <Pine.LNX.4.64.0804110650390.16547@ask.diku.dk>,Jesper Dangaard Brou
then perhaps just include linux/atm.h in this file. instead of
creating ifdef swiss cheese.
--
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--
