* Re: [RFC/PATCH 1/2] in-kernel sockets API
From: Brian F. G. Bidulock @ 2006-06-14 9:28 UTC (permalink / raw)
To: Chase Venters; +Cc: netdev, linux-kernel
In-Reply-To: <200606140258.30302.chase.venters@clientec.com>
Chase,
On Wed, 14 Jun 2006, Chase Venters wrote:
>
> One point I remember coming up in the discussion was that the
> EXPORT_SYMBOL()/EXPORT_SYMBOL_GPL() split was a compromise of sorts.
> Interfaces that were needed to support users would reasonably be placed under
> EXPORT_SYMBOL(). By contrast, EXPORT_SYMBOL_GPL() would indicate
> functionality that would only seem to be used by derived works. It implies
> that any code using it should probably be GPL as well.
The difficulty with EXPORT_SYMBOL_GPL() as I see it that it reached farther
than the GPL. GPL does not impact non-derived works, which can be licensed
under any terms their authors see fit. Whereas, EXPORT_SYMBOL_GPL() requires
a non-derived work to declare a GPL license to even use it. If you subscribe
to the FSF view of derived work (just linking is a derivation) then I suppose
you would support the EXPORT_SYMBOL_GPL(). IANAL, but I don't believe that
TRIPS nor Berne Convention case law supports the FSF view. Linus' statements
in the COPYING file take a different view: that simple use of a technical
interface is not necessarily (in itself) derivation.
Now, I understand the use of EXPORT_SYMBOL() vs. EXPORT_SYMBOL_GPL() to allow
authors to differ on this idea. But, in the case in point, the function
pointers can be accessed by merely including the appropriate header files.
Changing a the wrapper access to them to EXPORT_SYMBOL_GPL() strikes me as
similar to changing kmalloc() from EXPORT_SYMBOL() to EXPORT_SYMBOL_GPL().
Understand that all exported symbols, regardless of licensing or modversions
or whatever, are available in the kernel boot image and can be linked to by
any module at any time. That is, those that would abuse the concept of
derivation will not be impeded by EXPORT_SYMBOL_GPL(). (Rip the symbol from
the kernel image, write a thin GPL'ed module that aliases the symbol and the
exports it again as EXPORT_SYMBOL() without module versioning, copy the lines
of code into the proprietary module, reversing the order of arbitrary lines,
etc.)
In any case, all it serves to do is to punish honest non-derivative works not
published compatible with the GPL.
What I resist is the apparent attempt to change these symbols to _GPL as some
matter of general policy in this case contrary to the author's original
intentions as expressed in the original patch submission, and without the
author of the interface being wrappered jumping up an screaming that his code
was under strict FSF linking-is-derivation GPL (in which case we could have
had a good discussion on whether Linux NET4 is actually a derivative work of
BSD 4.4 Lite which was licensed under the "old" BSD license, incompatible with
the GPL ;)
As a general policy I would say make it EXPORT_SYMBOL() unless the author of
the patch (derivation) or author of the original (derived) code dictates that
it be EXPORT_SYMBOL_GPL().
Ok, I'll shut up now... ...really.
^ permalink raw reply
* [PATCH 0/2] NET: Accurate packet scheduling for ATM/ADSL
From: Jesper Dangaard Brouer @ 2006-06-14 9:40 UTC (permalink / raw)
To: Stephen Hemminger, Jamal Hadi Salim, netdev, lartc
Cc: russell-tcatm, hawk, hawk, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1311 bytes --]
The Linux traffic's control engine inaccurately calculates
transmission times for packets sent over ADSL links. For
some packet sizes the error rises to over 50%. This occurs
because ADSL uses ATM as its link layer transport, and ATM
transmits packets in fixed sized 53 byte cells.
The following patches to iproute2 and the kernel add an
option to calculate traffic transmission times over all
ATM links, including ADSL, with perfect accuracy.
A longer presentation of the patch, its rational, what it
does and how to use it can be found here:
http://www.stuart.id.au/russell/files/tc/tc-atm/
A earlier version of the patch, and a _detailed_ empirical
investigation of its effects can be found here:
http://www.adsl-optimizer.dk/
The patches are both backwards and forwards compatible.
This means unpatched kernels will work with a patched
version of iproute2, and an unpatched iproute2 will work
on patches kernels.
This is a combined effort of Jesper Brouer and Russell Stuart,
to get these patches into the upstream kernel.
Let the discussion start about what we need to change to get this
upstream?
We see this as a feature enhancement, as thus hope that it can be
queued in davem's net-2.6.18.git tree.
---
Regards,
Jesper Brouer & Russell Stuart.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH 1/2] NET: Accurate packet scheduling for ATM/ADSL (kernel)
From: Jesper Dangaard Brouer @ 2006-06-14 9:40 UTC (permalink / raw)
To: Stephen Hemminger, Jamal Hadi Salim, netdev, lartc
Cc: russell-tcatm, hawk, hawk, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 4899 bytes --]
The Linux traffic's control engine inaccurately calculates
transmission times for packets sent over ADSL links. For
some packet sizes the error rises to over 50%. This occurs
because ADSL uses ATM as its link layer transport, and ATM
transmits packets in fixed sized 53 byte cells.
This changes the kernel rate table lookup, to be able to lookup
packet transmission times over all ATM links, including ADSL,
with perfect accuracy. The accuracy is dependent on the rate
table that is calculated in userspace by iproute2 command tc.
A longer presentation of the patch, its rational, what it
does and how to use it can be found here:
http://www.stuart.id.au/russell/files/tc/tc-atm/
A earlier version of the patch, and a _detailed_ empirical
investigation of its effects can be found here:
http://www.adsl-optimizer.dk/
Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
Signed-off-by: Russell Stuart <russell-tcatm@stuart.id.au>
---
diff -Nurp kernel-source-2.6.16.orig/include/linux/pkt_sched.h kernel-source-2.6.16/include/linux/pkt_sched.h
--- kernel-source-2.6.16.orig/include/linux/pkt_sched.h 2006-03-20 15:53:29.000000000 +1000
+++ kernel-source-2.6.16/include/linux/pkt_sched.h 2006-06-13 11:42:12.000000000 +1000
@@ -77,8 +77,9 @@ struct tc_ratespec
{
unsigned char cell_log;
unsigned char __reserved;
- unsigned short feature;
- short addend;
+ unsigned short feature; /* Always 0 in pre-atm patch kernels */
+ char cell_align; /* Always 0 in pre-atm patch kernels */
+ unsigned char __unused;
unsigned short mpu;
__u32 rate;
};
diff -Nurp kernel-source-2.6.16.orig/include/net/sch_generic.h kernel-source-2.6.16/include/net/sch_generic.h
--- kernel-source-2.6.16.orig/include/net/sch_generic.h 2006-03-20 15:53:29.000000000 +1000
+++ kernel-source-2.6.16/include/net/sch_generic.h 2006-06-13 11:42:12.000000000 +1000
@@ -307,4 +307,18 @@ drop:
return NET_XMIT_DROP;
}
+/* Lookup a qdisc_rate_table to determine how long it will take to send a
+ packet given its size.
+ */
+static inline u32 qdisc_l2t(struct qdisc_rate_table* rtab, int pktlen)
+{
+ int slot = pktlen + rtab->rate.cell_align;
+ if (slot < 0)
+ slot = 0;
+ slot >>= rtab->rate.cell_log;
+ if (slot > 255)
+ return rtab->data[255] + 1;
+ return rtab->data[slot];
+}
+
#endif
diff -Nurp kernel-source-2.6.16.orig/net/sched/act_police.c kernel-source-2.6.16/net/sched/act_police.c
--- kernel-source-2.6.16.orig/net/sched/act_police.c 2006-03-20 15:53:29.000000000 +1000
+++ kernel-source-2.6.16/net/sched/act_police.c 2006-06-13 11:42:12.000000000 +1000
@@ -33,8 +33,8 @@
#include <net/sock.h>
#include <net/act_api.h>
-#define L2T(p,L) ((p)->R_tab->data[(L)>>(p)->R_tab->rate.cell_log])
-#define L2T_P(p,L) ((p)->P_tab->data[(L)>>(p)->P_tab->rate.cell_log])
+#define L2T(p,L) qdisc_l2t((p)->R_tab,L)
+#define L2T_P(p,L) qdisc_l2t((p)->P_tab,L)
#define PRIV(a) ((struct tcf_police *) (a)->priv)
/* use generic hash table */
diff -Nurp kernel-source-2.6.16.orig/net/sched/sch_cbq.c kernel-source-2.6.16/net/sched/sch_cbq.c
--- kernel-source-2.6.16.orig/net/sched/sch_cbq.c 2006-03-20 15:53:29.000000000 +1000
+++ kernel-source-2.6.16/net/sched/sch_cbq.c 2006-06-13 11:42:12.000000000 +1000
@@ -193,7 +193,7 @@ struct cbq_sched_data
};
-#define L2T(cl,len) ((cl)->R_tab->data[(len)>>(cl)->R_tab->rate.cell_log])
+#define L2T(cl,len) qdisc_l2t((cl)->R_tab,len)
static __inline__ unsigned cbq_hash(u32 h)
diff -Nurp kernel-source-2.6.16.orig/net/sched/sch_htb.c kernel-source-2.6.16/net/sched/sch_htb.c
--- kernel-source-2.6.16.orig/net/sched/sch_htb.c 2006-03-20 15:53:29.000000000 +1000
+++ kernel-source-2.6.16/net/sched/sch_htb.c 2006-06-13 11:42:12.000000000 +1000
@@ -206,12 +206,10 @@ struct htb_class
static __inline__ long L2T(struct htb_class *cl,struct qdisc_rate_table *rate,
int size)
{
- int slot = size >> rate->rate.cell_log;
- if (slot > 255) {
+ long result = qdisc_l2t(rate, size);
+ if (result > rate->data[255])
cl->xstats.giants++;
- slot = 255;
- }
- return rate->data[slot];
+ return result;
}
struct htb_sched
diff -Nurp kernel-source-2.6.16.orig/net/sched/sch_tbf.c kernel-source-2.6.16/net/sched/sch_tbf.c
--- kernel-source-2.6.16.orig/net/sched/sch_tbf.c 2006-03-20 15:53:29.000000000 +1000
+++ kernel-source-2.6.16/net/sched/sch_tbf.c 2006-06-13 11:42:12.000000000 +1000
@@ -132,8 +132,8 @@ struct tbf_sched_data
struct Qdisc *qdisc; /* Inner qdisc, default - bfifo queue */
};
-#define L2T(q,L) ((q)->R_tab->data[(L)>>(q)->R_tab->rate.cell_log])
-#define L2T_P(q,L) ((q)->P_tab->data[(L)>>(q)->P_tab->rate.cell_log])
+#define L2T(q,L) qdisc_l2t((q)->R_tab,L)
+#define L2T_P(q,L) qdisc_l2t((q)->P_tab,L)
static int tbf_enqueue(struct sk_buff *skb, struct Qdisc* sch)
{
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH 2/2] NET: Accurate packet scheduling for ATM/ADSL (userspace)
From: Jesper Dangaard Brouer @ 2006-06-14 9:40 UTC (permalink / raw)
To: Stephen Hemminger, Jamal Hadi Salim, netdev, lartc
Cc: russell-tcatm, hawk, hawk, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 22649 bytes --]
The Linux traffic's control engine inaccurately calculates
transmission times for packets sent over ADSL links. For
some packet sizes the error rises to over 50%. This occurs
because ADSL uses ATM as its link layer transport, and ATM
transmits packets in fixed sized 53 byte cells.
This changes the userspace tool iproute2/tc by adding an
option to calculate traffic transmission times (rate table)
over all ATM links, including ADSL, with perfect accuracy.
A longer presentation of the patch, its rational, what it
does and how to use it can be found here:
http://www.stuart.id.au/russell/files/tc/tc-atm/
A earlier version of the patch, and a _detailed_ empirical
investigation of its effects can be found here:
http://www.adsl-optimizer.dk/
Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
Signed-off-by: Russell Stuart <russell-tcatm@stuart.id.au>
---
diff -Nurp iproute2.orig/include/linux/pkt_sched.h iproute2/include/linux/pkt_sched.h
--- iproute2.orig/include/linux/pkt_sched.h 2005-12-10 09:27:44.000000000 +1000
+++ iproute2/include/linux/pkt_sched.h 2006-06-13 11:53:27.000000000 +1000
@@ -77,8 +77,9 @@ struct tc_ratespec
{
unsigned char cell_log;
unsigned char __reserved;
- unsigned short feature;
- short addend;
+ unsigned short feature; /* Always 0 in pre-atm patch kernels */
+ char cell_align; /* Always 0 in pre-atm patch kernels */
+ unsigned char __unused;
unsigned short mpu;
__u32 rate;
};
diff -Nurp iproute2.orig/tc/m_police.c iproute2/tc/m_police.c
--- iproute2.orig/tc/m_police.c 2005-01-19 08:11:58.000000000 +1000
+++ iproute2/tc/m_police.c 2006-06-13 11:53:27.000000000 +1000
@@ -35,7 +35,7 @@ struct action_util police_action_util =
static void explain(void)
{
fprintf(stderr, "Usage: ... police rate BPS burst BYTES[/BYTES] [ mtu BYTES[/BYTES] ]\n");
- fprintf(stderr, " [ peakrate BPS ] [ avrate BPS ]\n");
+ fprintf(stderr, " [ peakrate BPS ] [ avrate BPS ] [ overhead OVERHEAD ] [ atm ]\n");
fprintf(stderr, " [ ACTIONTERM ]\n");
fprintf(stderr, "Old Syntax ACTIONTERM := action <EXCEEDACT>[/NOTEXCEEDACT] \n");
fprintf(stderr, "New Syntax ACTIONTERM := conform-exceed <EXCEEDACT>[/NOTEXCEEDACT] \n");
@@ -134,7 +134,10 @@ int act_parse_police(struct action_util
__u32 ptab[256];
__u32 avrate = 0;
int presult = 0;
- unsigned buffer=0, mtu=0, mpu=0;
+ unsigned buffer=0, mtu=0;
+ __u8 mpu=0;
+ __s8 overhead=0;
+ int atm=0;
int Rcell_log=-1, Pcell_log = -1;
struct rtattr *tail;
@@ -184,7 +187,7 @@ int act_parse_police(struct action_util
fprintf(stderr, "Double \"mpu\" spec\n");
return -1;
}
- if (get_size(&mpu, *argv)) {
+ if (get_u8(&mpu, *argv, 10)) {
explain1("mpu");
return -1;
}
@@ -198,6 +201,18 @@ int act_parse_police(struct action_util
explain1("rate");
return -1;
}
+ } else if (strcmp(*argv, "overhead") == 0) {
+ NEXT_ARG();
+ if (p.rate.rate) {
+ fprintf(stderr, "Double \"overhead\" spec\n");
+ return -1;
+ }
+ if (get_s8(&overhead, *argv, 10)) {
+ explain1("overhead");
+ return -1;
+ }
+ } else if (strcmp(*argv, "atm") == 0) {
+ atm = 1;
} else if (strcmp(*argv, "avrate") == 0) {
NEXT_ARG();
if (avrate) {
@@ -264,22 +279,12 @@ int act_parse_police(struct action_util
}
if (p.rate.rate) {
- if ((Rcell_log = tc_calc_rtable(p.rate.rate, rtab, Rcell_log, mtu, mpu)) < 0) {
- fprintf(stderr, "TBF: failed to calculate rate table.\n");
- return -1;
- }
+ tc_calc_ratespec(&p.rate, rtab, p.rate.rate, Rcell_log, mtu, mpu, atm, overhead);
p.burst = tc_calc_xmittime(p.rate.rate, buffer);
- p.rate.cell_log = Rcell_log;
- p.rate.mpu = mpu;
}
p.mtu = mtu;
if (p.peakrate.rate) {
- if ((Pcell_log = tc_calc_rtable(p.peakrate.rate, ptab, Pcell_log, mtu, mpu)) < 0) {
- fprintf(stderr, "POLICE: failed to calculate peak rate table.\n");
- return -1;
- }
- p.peakrate.cell_log = Pcell_log;
- p.peakrate.mpu = mpu;
+ tc_calc_ratespec(&p.peakrate, ptab, p.peakrate.rate, Pcell_log, mtu, mpu, atm, overhead);
}
tail = NLMSG_TAIL(n);
diff -Nurp iproute2.orig/tc/q_cbq.c iproute2/tc/q_cbq.c
--- iproute2.orig/tc/q_cbq.c 2005-07-06 08:37:15.000000000 +1000
+++ iproute2/tc/q_cbq.c 2006-06-13 11:53:27.000000000 +1000
@@ -32,6 +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 ] [ atm ]\n");
}
static void explain(void)
@@ -52,7 +53,10 @@ static int cbq_parse_opt(struct qdisc_ut
struct tc_ratespec r;
struct tc_cbq_lssopt lss;
__u32 rtab[256];
- unsigned mpu=0, avpkt=0, allot=0;
+ unsigned avpkt=0, allot=0;
+ __u8 mpu=0;
+ __s8 overhead=0;
+ int atm=0;
int cell_log=-1;
int ewma_log=-1;
struct rtattr *tail;
@@ -102,7 +106,7 @@ static int cbq_parse_opt(struct qdisc_ut
}
} else if (strcmp(*argv, "mpu") == 0) {
NEXT_ARG();
- if (get_size(&mpu, *argv)) {
+ if (get_u8(&mpu, *argv, 10)) {
explain1("mpu");
return -1;
}
@@ -113,6 +117,14 @@ static int cbq_parse_opt(struct qdisc_ut
explain1("allot");
return -1;
}
+ } else if (strcmp(*argv, "overhead") == 0) {
+ NEXT_ARG();
+ if (get_s8(&overhead, *argv, 10)) {
+ explain1("overhead");
+ return -1;
+ }
+ } else if (strcmp(*argv, "atm") == 0) {
+ atm = 1;
} else if (strcmp(*argv, "help") == 0) {
explain();
return -1;
@@ -137,12 +149,7 @@ static int cbq_parse_opt(struct qdisc_ut
if (allot < (avpkt*3)/2)
allot = (avpkt*3)/2;
- if ((cell_log = tc_calc_rtable(r.rate, rtab, cell_log, allot, mpu)) < 0) {
- fprintf(stderr, "CBQ: failed to calculate rate table.\n");
- return -1;
- }
- r.cell_log = cell_log;
- r.mpu = mpu;
+ tc_calc_ratespec(&r, rtab, r.rate, cell_log, allot, mpu, atm, overhead);
if (ewma_log < 0)
ewma_log = TC_CBQ_DEF_EWMA;
@@ -175,7 +182,9 @@ static int cbq_parse_class_opt(struct qd
struct tc_cbq_fopt fopt;
struct tc_cbq_ovl ovl;
__u32 rtab[256];
- unsigned mpu=0;
+ __u8 mpu=0;
+ __s8 overhead = 0;
+ int atm = 0;
int cell_log=-1;
int ewma_log=-1;
unsigned bndw = 0;
@@ -289,10 +298,18 @@ static int cbq_parse_class_opt(struct qd
lss.change |= TCF_CBQ_LSS_AVPKT;
} else if (strcmp(*argv, "mpu") == 0) {
NEXT_ARG();
- if (get_size(&mpu, *argv)) {
+ if (get_u8(&mpu, *argv, 10)) {
explain1("mpu");
return -1;
}
+ } else if (strcmp(*argv, "overhead") == 0) {
+ NEXT_ARG();
+ if (get_s8(&overhead, *argv, 10)) {
+ explain1("overhead");
+ return -1;
+ }
+ } else if (strcmp(*argv, "atm") == 0) {
+ atm = 1;
} else if (strcmp(*argv, "weight") == 0) {
NEXT_ARG();
if (get_size(&wrr.weight, *argv)) {
@@ -336,12 +353,7 @@ static int cbq_parse_class_opt(struct qd
unsigned pktsize = wrr.allot;
if (wrr.allot < (lss.avpkt*3)/2)
wrr.allot = (lss.avpkt*3)/2;
- if ((cell_log = tc_calc_rtable(r.rate, rtab, cell_log, pktsize, mpu)) < 0) {
- fprintf(stderr, "CBQ: failed to calculate rate table.\n");
- return -1;
- }
- r.cell_log = cell_log;
- r.mpu = mpu;
+ tc_calc_ratespec(&r, rtab, r.rate, cell_log, pktsize, mpu, atm, overhead);
}
if (ewma_log < 0)
ewma_log = TC_CBQ_DEF_EWMA;
@@ -463,8 +475,12 @@ static int cbq_print_opt(struct qdisc_ut
fprintf(f, "rate %s ", buf);
if (show_details) {
fprintf(f, "cell %ub ", 1<<r->cell_log);
- if (r->mpu)
- fprintf(f, "mpu %ub ", r->mpu);
+ if (r->mpu & 0xff)
+ fprintf(f, "mpu %ub ", (__u8)r->mpu);
+ if ((r->mpu >> 8))
+ fprintf(f, "overhead %db ", (__s8)(r->mpu >> 8));
+ if (r->feature & 0x0001)
+ fprintf(f, "atm ");
}
}
if (lss && lss->flags) {
diff -Nurp iproute2.orig/tc/q_htb.c iproute2/tc/q_htb.c
--- iproute2.orig/tc/q_htb.c 2005-01-19 08:11:58.000000000 +1000
+++ iproute2/tc/q_htb.c 2006-06-13 11:53:27.000000000 +1000
@@ -34,14 +34,14 @@ static void explain(void)
" default minor id of class to which unclassified packets are sent {0}\n"
" r2q DRR quantums are computed as rate in Bps/r2q {10}\n"
" debug string of 16 numbers each 0-3 {0}\n\n"
- "... class add ... htb rate R1 [burst B1] [mpu B] [overhead O]\n"
+ "... class add ... htb rate R1 [burst B1] [mpu B] [overhead O] [atm]\n"
" [prio P] [slot S] [pslot PS]\n"
" [ceil R2] [cburst B2] [mtu MTU] [quantum Q]\n"
" rate rate allocated to this class (class can still borrow)\n"
" 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"
-
+ " atm include atm cell tax in rate computations\n"
" ceil definite upper class rate (no borrows) {rate}\n"
" cburst burst but for ceil {computed}\n"
" mtu max packet size we create rate map for {1600}\n"
@@ -107,8 +107,10 @@ static int htb_parse_class_opt(struct qd
__u32 rtab[256],ctab[256];
unsigned buffer=0,cbuffer=0;
int cell_log=-1,ccell_log = -1;
- unsigned mtu, mpu;
- unsigned char mpu8 = 0, overhead = 0;
+ unsigned mtu;
+ __u8 mpu8=0;
+ __s8 overhead=0;
+ int atm=0;
struct rtattr *tail;
memset(&opt, 0, sizeof(opt)); mtu = 1600; /* eth packet len */
@@ -132,9 +134,11 @@ static int htb_parse_class_opt(struct qd
}
} else if (matches(*argv, "overhead") == 0) {
NEXT_ARG();
- if (get_u8(&overhead, *argv, 10)) {
+ if (get_s8(&overhead, *argv, 10)) {
explain1("overhead"); return -1;
}
+ } else if (matches(*argv, "atm") == 0) {
+ atm = 1;
} else if (matches(*argv, "quantum") == 0) {
NEXT_ARG();
if (get_u32(&opt.quantum, *argv, 10)) {
@@ -206,23 +210,11 @@ static int htb_parse_class_opt(struct qd
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;
-
- if ((cell_log = tc_calc_rtable(opt.rate.rate, rtab, cell_log, mtu, mpu)) < 0) {
- fprintf(stderr, "htb: failed to calculate rate table.\n");
- return -1;
- }
+ /* encode overhead and mpu, 8 bits each, into lower 16 bits */
+ tc_calc_ratespec(&opt.rate, rtab, opt.rate.rate, cell_log, mtu, mpu8, atm, overhead);
+ tc_calc_ratespec(&opt.ceil, ctab, opt.ceil.rate, cell_log, mtu, mpu8, atm, overhead);
opt.buffer = tc_calc_xmittime(opt.rate.rate, buffer);
- opt.rate.cell_log = cell_log;
-
- if ((ccell_log = tc_calc_rtable(opt.ceil.rate, ctab, cell_log, mtu, mpu)) < 0) {
- fprintf(stderr, "htb: failed to calculate ceil rate table.\n");
- return -1;
- }
opt.cbuffer = tc_calc_xmittime(opt.ceil.rate, cbuffer);
- opt.ceil.cell_log = ccell_log;
tail = NLMSG_TAIL(n);
addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
@@ -267,12 +259,16 @@ static int htb_print_opt(struct qdisc_ut
sprint_size(buffer, b1),
1<<hopt->rate.cell_log,
sprint_size(hopt->rate.mpu&0xFF, b2),
- sprint_size((hopt->rate.mpu>>8)&0xFF, b3));
+ sprint_size((__s8)(hopt->rate.mpu>>8), b3));
+ if (hopt->rate.feature & 0x0001)
+ fprintf(f, "atm ");
fprintf(f, "cburst %s/%u mpu %s overhead %s ",
sprint_size(cbuffer, b1),
1<<hopt->ceil.cell_log,
sprint_size(hopt->ceil.mpu&0xFF, b2),
- sprint_size((hopt->ceil.mpu>>8)&0xFF, b3));
+ sprint_size((__s8)(hopt->ceil.mpu>>8), b3));
+ if (hopt->ceil.feature & 0x0001)
+ fprintf(f, "atm ");
fprintf(f, "level %d ", (int)hopt->level);
} else {
fprintf(f, "burst %s ", sprint_size(buffer, b1));
diff -Nurp iproute2.orig/tc/q_tbf.c iproute2/tc/q_tbf.c
--- iproute2.orig/tc/q_tbf.c 2005-01-19 08:11:58.000000000 +1000
+++ iproute2/tc/q_tbf.c 2006-06-13 11:53:27.000000000 +1000
@@ -26,7 +26,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 ]\n");
+ fprintf(stderr, " [ peakrate KBPS ] [ latency TIME ] [ overhead OVERHEAD ] [ atm ]\n");
}
static void explain1(char *arg)
@@ -43,7 +43,10 @@ static int tbf_parse_opt(struct qdisc_ut
struct tc_tbf_qopt opt;
__u32 rtab[256];
__u32 ptab[256];
- unsigned buffer=0, mtu=0, mpu=0, latency=0;
+ unsigned buffer=0, mtu=0, latency=0;
+ __u8 mpu=0;
+ __s8 overhead=0;
+ int atm=0;
int Rcell_log=-1, Pcell_log = -1;
struct rtattr *tail;
@@ -103,7 +106,7 @@ static int tbf_parse_opt(struct qdisc_ut
fprintf(stderr, "Double \"mpu\" spec\n");
return -1;
}
- if (get_size(&mpu, *argv)) {
+ if (get_u8(&mpu, *argv, 10)) {
explain1("mpu");
return -1;
}
@@ -119,6 +122,20 @@ static int tbf_parse_opt(struct qdisc_ut
return -1;
}
ok++;
+ } else if (strcmp(*argv, "overhead") == 0) {
+ NEXT_ARG();
+ if (overhead) {
+ fprintf(stderr, "Double \"overhead\" spec\n");
+ return -1;
+ }
+ if (get_s8(&overhead, *argv, 10)) {
+ explain1("overhead");
+ return -1;
+ }
+ ok++;
+ } else if (strcmp(*argv, "atm") == 0) {
+ atm = 1;
+ ok++;
} else if (matches(*argv, "peakrate") == 0) {
NEXT_ARG();
if (opt.peakrate.rate) {
@@ -170,21 +187,11 @@ static int tbf_parse_opt(struct qdisc_ut
opt.limit = lim;
}
- if ((Rcell_log = tc_calc_rtable(opt.rate.rate, rtab, Rcell_log, mtu, mpu)) < 0) {
- fprintf(stderr, "TBF: failed to calculate rate table.\n");
- return -1;
- }
+ tc_calc_ratespec(&opt.rate, rtab, opt.rate.rate, Rcell_log, mtu, mpu, atm, overhead);
opt.buffer = tc_calc_xmittime(opt.rate.rate, buffer);
- opt.rate.cell_log = Rcell_log;
- opt.rate.mpu = mpu;
if (opt.peakrate.rate) {
- if ((Pcell_log = tc_calc_rtable(opt.peakrate.rate, ptab, Pcell_log, mtu, mpu)) < 0) {
- fprintf(stderr, "TBF: failed to calculate peak rate table.\n");
- return -1;
- }
+ tc_calc_ratespec(&opt.peakrate, ptab, opt.peakrate.rate, Pcell_log, mtu, mpu, atm, overhead);
opt.mtu = tc_calc_xmittime(opt.peakrate.rate, mtu);
- opt.peakrate.cell_log = Pcell_log;
- opt.peakrate.mpu = mpu;
}
tail = NLMSG_TAIL(n);
@@ -220,8 +227,12 @@ static int tbf_print_opt(struct qdisc_ut
fprintf(f, "rate %s ", sprint_rate(qopt->rate.rate, b1));
buffer = ((double)qopt->rate.rate*tc_core_tick2usec(qopt->buffer))/1000000;
if (show_details) {
- fprintf(f, "burst %s/%u mpu %s ", sprint_size(buffer, b1),
- 1<<qopt->rate.cell_log, sprint_size(qopt->rate.mpu, b2));
+ fprintf(f, "burst %s/%u mpu %s overhead %d ", sprint_size(buffer, b1),
+ 1<<qopt->rate.cell_log,
+ sprint_size(qopt->rate.mpu & 0xFF, b2),
+ (__s8)(qopt->rate.mpu >> 8));
+ if (qopt->rate.feature & 0x0001)
+ fprintf(f, "atm ");
} else {
fprintf(f, "burst %s ", sprint_size(buffer, b1));
}
@@ -232,8 +243,12 @@ static int tbf_print_opt(struct qdisc_ut
if (qopt->mtu || qopt->peakrate.mpu) {
mtu = ((double)qopt->peakrate.rate*tc_core_tick2usec(qopt->mtu))/1000000;
if (show_details) {
- fprintf(f, "mtu %s/%u mpu %s ", sprint_size(mtu, b1),
- 1<<qopt->peakrate.cell_log, sprint_size(qopt->peakrate.mpu, b2));
+ fprintf(f, "mtu %s/%u mpu %s overhead %d ", sprint_size(mtu, b1),
+ 1<<qopt->peakrate.cell_log,
+ sprint_size(qopt->peakrate.mpu & 0xFF, b2),
+ (__s8)(qopt->peakrate.mpu >> 8));
+ if (qopt->peakrate.feature & 0x0001)
+ fprintf(f, "atm ");
} else {
fprintf(f, "minburst %s ", sprint_size(mtu, b1));
}
diff -Nurp iproute2.orig/tc/tc_core.c iproute2/tc/tc_core.c
--- iproute2.orig/tc/tc_core.c 2004-07-31 06:26:15.000000000 +1000
+++ iproute2/tc/tc_core.c 2006-06-13 11:53:27.000000000 +1000
@@ -23,6 +23,9 @@
#include "tc_core.h"
+#define ATM_CELL_SIZE 53
+#define ATM_CELL_PAYLOAD 48
+
static __u32 t2us=1;
static __u32 us2t=1;
static double tick_in_usec = 1;
@@ -43,33 +46,124 @@ unsigned tc_calc_xmittime(unsigned rate,
}
/*
- rtab[pkt_len>>cell_log] = pkt_xmit_time
+ * Calculate the ATM cell overhead. ATM sends each packet in 48 byte
+ * chunks, the last chunk being padded if necessary. Each chunk carries
+ * an additional 5 byte overhead - the ATM header.
*/
+static int tc_align_to_cells(int size)
+{
+ int cells;
+
+ cells = size / ATM_CELL_PAYLOAD;
+ if (size % ATM_CELL_PAYLOAD != 0)
+ cells++;
+ return cells * ATM_CELL_SIZE;
+}
-int tc_calc_rtable(unsigned bps, __u32 *rtab, int cell_log, unsigned mtu,
- unsigned mpu)
+/*
+ * The number this function calculates is subtle. Ignore it and just believe
+ * it works if you have a choice, otherwise ..
+ *
+ * If there we are calculating the ATM cell overhead the kernel calculations
+ * will be out sometimes if the range of packet sizes spanned by one
+ * rate table element crosses an ATM cell boundary. Consider these three
+ * senarios:
+ * (a) the packet is sent across the ATM link without addition
+ * overheads the kernel doesn't know about, and
+ * (b) a packet that has 1 byte of additional overhead the kernel
+ * doesn't know about. Here
+ * (c) a packet that has 2 bytes of additional overhead the
+ * kernel doesn't know about.
+ * The table below presents what happens. Each row is for a single rate
+ * table element. The "Sizes" column shows what packet sizes the rate table
+ * element will be used for. This packet size includes the "unknown to
+ * kernel" overhead, but does not include overhead incurred by breaking the
+ * packet up into ATM cells. This ATM cell overhead consists of the 5 byte
+ * header per ATM cell, plus the padding in the last cell. The "ATM" column
+ * shows how many bytes are actually sent across the ATM link, ie it does
+ * include the ATM cell overhead.
+ *
+ * RateTable Entry Sizes(a) ATM(a) Sizes(b) ATM(b) Sizes(c) ATM(c)
+ * ratetable[0] 0..7 53 1..8 53 2..9 53
+ * ratetable[1] 8..15 53 9..16 53 2..17 53
+ * ratetable[2] 16..23 53 17..24 53 18..25 53
+ * ratetable[3] 24..31 53 25..32 53 26..33 53
+ * ratetable[4] 32..39 53 33..40 53 34..41 53
+ * ratetable[5] 40..47 53 41..48 53 42..49 53,106
+ * ratetable[6] 48..55 53,106 49..56 106 50..57 106
+ *
+ * For senario (a), the ratetable[6] entry covers two cases: one were a single
+ * ATM cell is needed to transmit the data, and one where two ATM cells are
+ * required. It can't be right for both. Unfortunately the error is large.
+ * The same problem arises in senario (c) for ratetable[5]. The problem
+ * doesn't happen for senario (b), because the boundary between rate table
+ * entries happens to match the boundary between ATM cells.
+ *
+ * What we would like to do is ensure that ratetable boundaries always match
+ * the ATM cells. If we do this the error goes away. The solution is to make
+ * the kernel add a small bias to the packet size. (Small because the bias
+ * will always be smaller than cell_log.) Adding this small bias will in
+ * effect slide the ratetable along a bit, so the boundaries match. The code
+ * below calculates that bias. Provided the MTU is less than 4092, doing
+ * this can always eliminate the error.
+ *
+ * Old kernels won't add this bias, so they will have the error described above
+ * in most cases. In the worst case senario, considering all possible ATM cell
+ * sizes (1..48), for 7 of these sizes the old kernel will calculate the rate
+ * wrongly - ie, be out by 53 bytes.
+ */
+static int tc_calc_cell_align(int atm_cell_tax, char overhead, int cell_log)
+{
+ int cell_size;
+
+ if (!atm_cell_tax)
+ return 0;
+ cell_size = 1 << cell_log;
+ return (overhead + cell_size - 2) % cell_size - cell_size + 1;
+}
+
+
+/*
+ * A constructor for a tc_ratespec.
+ */
+void tc_calc_ratespec(struct tc_ratespec* spec, __u32* rtab, unsigned bps,
+ int cell_log, unsigned mtu, unsigned char mpu, int atm_cell_tax,
+ char overhead)
{
int i;
- unsigned overhead = (mpu >> 8) & 0xFF;
- mpu = mpu & 0xFF;
if (mtu == 0)
mtu = 2047;
+ /* rtab[pkt_len>>cell_log] = pkt_xmit_time */
if (cell_log < 0) {
cell_log = 0;
while ((mtu>>cell_log) > 255)
cell_log++;
}
+
for (i=0; i<256; i++) {
- unsigned sz = (i<<cell_log);
- if (overhead)
- sz += overhead;
+ /*
+ * sz is the length of packet we will use for this ratetable
+ * entry. The time taken to send a packet of this length will
+ * be used for all packet lengths this ratetable entry applies
+ * to. As underestimating how long it will take to transmit a
+ * packet is a worse error than overestimating it, the longest
+ * packet this rate table entry applies to is used.
+ */
+ int sz = ((i+1)<<cell_log) - 1 + overhead;
if (sz < mpu)
sz = mpu;
- rtab[i] = tc_core_usec2tick(1000000*((double)sz/bps));
+ if (atm_cell_tax)
+ sz = tc_align_to_cells(sz);
+ rtab[i] = tc_calc_xmittime(bps, sz);
}
- return cell_log;
+
+ spec->cell_align = tc_calc_cell_align(atm_cell_tax, overhead, cell_log);
+ spec->cell_log = cell_log;
+ spec->feature = 0x8000 | (atm_cell_tax ? 1 : 0);
+ spec->mpu = mpu | (unsigned)(overhead << 8);
+ spec->rate = bps;
}
int tc_core_init()
diff -Nurp iproute2.orig/tc/tc_core.h iproute2/tc/tc_core.h
--- iproute2.orig/tc/tc_core.h 2005-03-15 05:02:41.000000000 +1000
+++ iproute2/tc/tc_core.h 2006-06-13 11:53:27.000000000 +1000
@@ -7,7 +7,9 @@
long tc_core_usec2tick(long usec);
long tc_core_tick2usec(long tick);
unsigned tc_calc_xmittime(unsigned rate, unsigned size);
-int tc_calc_rtable(unsigned bps, __u32 *rtab, int cell_log, unsigned mtu, unsigned mpu);
+void tc_calc_ratespec(struct tc_ratespec* spec, __u32* rtab, unsigned bps,
+ int cell_log, unsigned mtu, unsigned char mpu, int atm_cell_tax,
+ char overhead);
int tc_setup_estimator(unsigned A, unsigned time_const, struct tc_estimator *est);
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Your money, newspaper post
From: Abraham Mcmahon @ 2006-06-14 10:06 UTC (permalink / raw)
To: netdev
Even if you have no erectin problems SOFT CIAzLIS
would help you to make BETTER SE X MORE OFTEN!
and to bring unimagnable plesure to her.
Just disolve half a pil under your tongue
and get ready for action in 15 minutes.
The tests showed that the majority of men
after taking this medic ation were able to have
PERFECT ER ECTI ON during 36 hours!
VISIT US, AND GET OUR SPECIAL 70% DISC OUNT OFER!
http://qjomos.skinscloth.com/?88863051
=====
It went like lightning through the Flock. Those birds are Outcast!
report From the police--again, what could they know there except for my old
or twice?"
Damn those eggheads! Some thinking to lay the road over the dump! And I
at sea, hungry, happy, learning.
"Shut up."
hour. He swallowed, knowing that if his wings unfolded at that speed be'd
saying nothing. Suddenly he clapped his hands, rubbed his palms together,
^ permalink raw reply
* Re: [RFC/PATCH 1/2] in-kernel sockets API
From: Alan Cox @ 2006-06-14 10:43 UTC (permalink / raw)
To: bidulock; +Cc: Chase Venters, netdev, linux-kernel
In-Reply-To: <20060614000710.C7232@openss7.org>
Ar Mer, 2006-06-14 am 00:07 -0600, ysgrifennodd Brian F. G. Bidulock:
> I think that a policy that intentionally makes it hard for proprietary
> modules to be developed defeats the purpose of ultimate opening and merging.
It isn't "policy" its called copyright law.
> The interface currently under discussion is ultimately derived from the BSD
> socket-protocol interface, and IMHO should be EXPORT_SYMBOL instead of
> EXPORT_SYMBOL_GPL, if only because using _GPL serves no purpose here and can
> be defeated with 3 or 4 obvious (and probably existing) lines of code
You don't seem to understand copyright law either. The GPL like all
copyright licenses deals with the right to make copies and to create and
control derivative works. It's not "defeated" by four lines of code.
I
> wrote similar wrappers for STREAMS TPI to Linux NET4 interface instead of
> using pointers directly quite a few years ago. I doubt I was the first.
Is that a confession ;)
> There is nothing really so novel here that it deserves _GPL.
Copyright is not about novelty, you have it confused with the
theoretical (not actual) role of patents. Wrong kind of intellectual
monopoly right.
Alan
^ permalink raw reply
* Re: [RFC/PATCH 1/2] in-kernel sockets API
From: Theodore Tso @ 2006-06-14 10:36 UTC (permalink / raw)
To: Chase Venters
Cc: bidulock, Daniel Phillips, Stephen Hemminger, Sridhar Samudrala,
netdev, linux-kernel
In-Reply-To: <200606131953.42002.chase.venters@clientec.com>
On Tue, Jun 13, 2006 at 07:53:19PM -0500, Chase Venters wrote:
> > It is the lack of an ABI that is most frustrating to these users.
>
> And the presence of an ABI would be _very_ frustrating to core
> developers. Not only would these people suffer, everyone would --
> developer time would be wasted dealing with cruft, and forward
> progress would be slowed.
Note that just because an interface is EXPORT_SYMBOL doesn't mean that
the interface is guaranteed to be stable. So folks who are aruging
that an interface shouldn't be usable by non-GPL applications because
we are therefore guaranteeing a stable API are making an unwarranted
assumption.
- Ted
^ permalink raw reply
* Re: [PATCH 2/2] NET: Accurate packet scheduling for ATM/ADSL (userspace)
From: Alan Cox @ 2006-06-14 10:57 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Stephen Hemminger, Jamal Hadi Salim, netdev, lartc, russell-tcatm,
hawk, linux-kernel
In-Reply-To: <1150278040.26181.37.camel@localhost.localdomain>
Ar Mer, 2006-06-14 am 11:40 +0200, ysgrifennodd Jesper Dangaard Brouer:
> option to calculate traffic transmission times (rate table)
> over all ATM links, including ADSL, with perfect accuracy.
<Pedant>
Only if the lowest level is encoded in a time linear manner. If you are
using NRZ, NRZI etc at the bottom level then you may still be out...
</Pedant>
The other problem I see with this code is it is very tightly tied to ATM
cell sizes, not to solving the generic question of packetisation. I'm
not sure if that matters but for modern processors I'm also sceptical
that the clever computation is actually any faster than just doing the
maths, especially if something cache intensive is also running.
Alan
^ permalink raw reply
* Re: [RFC/PATCH 1/2] in-kernel sockets API
From: Brian F. G. Bidulock @ 2006-06-14 10:54 UTC (permalink / raw)
To: Alan Cox; +Cc: netdev, linux-kernel
In-Reply-To: <1150281823.3490.14.camel@localhost.localdomain>
Alan,
On Wed, 14 Jun 2006, Alan Cox wrote:
> It isn't "policy" its called copyright law.
I know that I said I'd shut up, but I missed in TRIPS where it said
that symbols must be EXPORT_SYMBOL_GPL... Could you point that out?
(Just kidding.)
> You don't seem to understand copyright law either. The GPL like all
> copyright licenses deals with the right to make copies and to create and
> control derivative works. It's not "defeated" by four lines of code.
The 3 or 4 lines of code that I wrote as an original expression before
the patch was submitted.
> Is that a confession ;)
No, just a declaration: the code in question was released under GPL
Version 2.
> Copyright is not about novelty, you have it confused with the
> theoretical (not actual) role of patents. Wrong kind of intellectual
> monopoly right.
Yes, perhaps I should have said "original" instead of "novel". The patch
is not "original" as it was predated by equivalent (machine translatable)
original expressions.
^ permalink raw reply
* Re: Remove Prism II support from Orinoco
From: Jiri Benc @ 2006-06-14 11:53 UTC (permalink / raw)
To: Faidon Liambotis; +Cc: netdev
In-Reply-To: <448E3F80.5050103@cube.gr>
On Tue, 13 Jun 2006 07:30:56 +0300, Faidon Liambotis wrote:
> Unfortunately, that workaround doesn't work so well when you want to
> have the ability to plug real orinoco (hermes) cards to your computer...
> In other words and unless I'm missing something, there isn't currently a
> way to have a Hermes card and a Prism II card both plugged in and working.
Do you know about /sys/bus/pci/drivers/*/bind and unbind?
http://lwn.net/Articles/143397/
--
Jiri Benc
SUSE Labs
^ permalink raw reply
* Re: [PATCH 0/2] NET: Accurate packet scheduling for ATM/ADSL
From: jamal @ 2006-06-14 12:06 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: hawk, russell-tcatm, lartc, netdev, Stephen Hemminger
In-Reply-To: <1150278004.26181.35.camel@localhost.localdomain>
I have taken linux-kernel off the list.
Russell's site is inaccessible to me (I actually think this is related
to some DNS issues i may be having) and your masters is too long to
spend 2 minutes and glean it; so heres a question or two for you:
- Have you tried to do a long-lived session such as a large FTP and
seen how far off the deviation was? That would provide some interesting
data point.
- To be a devil's advocate (and not claim there is no issue), where do
you draw the line with "overhead"?
Example the smallest ethernet packet is 64 bytes of which 14 bytes are
ethernet headers ("overhead" for IP) - and this is not counting CRC etc.
If you were to set an MTU of say 64 bytes and tried to do a http or ftp,
how accurate do you think the calculation would be? I would think not
very different.
Does it matter if it is accurate on the majority of the cases?
- For further reflection: Have you considered the case where the rate
table has already been considered on some link speed in user space and
then somewhere post-config the physical link speed changes? This would
happen in the case where ethernet AN is involved and the partner makes
some changes (use ethtool).
I would say the last bullet is a more interesting problem than a corner
case of some link layer technology that has high overhead.
Your work would be more interesting if it was generic for many link
layers instead of just ATM.
cheers,
jamal
On Wed, 2006-14-06 at 11:40 +0200, Jesper Dangaard Brouer wrote:
> The Linux traffic's control engine inaccurately calculates
> transmission times for packets sent over ADSL links. For
> some packet sizes the error rises to over 50%. This occurs
> because ADSL uses ATM as its link layer transport, and ATM
> transmits packets in fixed sized 53 byte cells.
>
> The following patches to iproute2 and the kernel add an
> option to calculate traffic transmission times over all
> ATM links, including ADSL, with perfect accuracy.
>
> A longer presentation of the patch, its rational, what it
> does and how to use it can be found here:
> http://www.stuart.id.au/russell/files/tc/tc-atm/
>
> A earlier version of the patch, and a _detailed_ empirical
> investigation of its effects can be found here:
> http://www.adsl-optimizer.dk/
>
> The patches are both backwards and forwards compatible.
> This means unpatched kernels will work with a patched
> version of iproute2, and an unpatched iproute2 will work
> on patches kernels.
>
>
> This is a combined effort of Jesper Brouer and Russell Stuart,
> to get these patches into the upstream kernel.
>
> Let the discussion start about what we need to change to get this
> upstream?
>
> We see this as a feature enhancement, as thus hope that it can be
> queued in davem's net-2.6.18.git tree.
>
> ---
> Regards,
> Jesper Brouer & Russell Stuart.
>
^ permalink raw reply
* Re: Refactor Netlink connector?
From: jamal @ 2006-06-14 12:36 UTC (permalink / raw)
To: James Morris
Cc: Stephen Smalley, David S. Miller, netdev, Evgeniy Polyakov,
Thomas Graf
In-Reply-To: <Pine.LNX.4.64.0606011022450.16136@d.namei>
So whats the resolution on this? I actually have some cycles this coming
weekend that i was hopping to spend updating the doc instead.
cheers,
jamal
On Thu, 2006-01-06 at 10:24 -0400, James Morris wrote:
> On Thu, 1 Jun 2006, Thomas Graf wrote:
>
> > It shouldn't be hard to split what is implemented in nlmsg_route_perms[]
> > for NETLINK_ROUTE into the definitions of the generic netlink
> > operations, could look like this:
> >
> > struct genl_ops some_op = {
> > [...]
> > .perm = NETLINK_GENERIC_SOCKET__NLMSG_READ,
> > };
>
> We wouldn't need the socket class outside of SELinux, just the perm, so
> something like:
>
> NL_PERM_READ
>
> > int genl_peek_cmd(struct nlmsghdr *nlh)
> > {
> > struct genlmsghdr *hdr = nlmsg_data(nlh);
> >
> > if (nlh->nlmsglen < nlmsg_msg_sizeo(GENL_HDRLEN))
> > return -EINVAL;
> >
> > return hdr->cmd;
> > }
>
> Unless I'm mistaken, people are already multiplexing commands inside genl
> commands (and if so, why even bother with registerable ops?).
>
>
> I'll look at it in more detail soon.
>
>
^ permalink raw reply
* Re: [PATCH 0/2] NET: Accurate packet scheduling for ATM/ADSL
From: Jesper Dangaard Brouer @ 2006-06-14 12:55 UTC (permalink / raw)
To: hadi; +Cc: hawk, russell-tcatm, lartc, netdev, Stephen Hemminger
In-Reply-To: <1150286766.5233.15.camel@jzny2>
[-- Attachment #1: Type: text/plain, Size: 4519 bytes --]
On Wed, 2006-06-14 at 08:06 -0400, jamal wrote:
> Russell's site is inaccessible to me (I actually think this is related
> to some DNS issues i may be having)
Strange, I have access to Russell's site. Maybe its his redirect
feature that confuses your browser, try:
http://ace-host.stuart.id.au/russell/files/tc/tc-atm/
> and your masters is too long to
> spend 2 minutes and glean it; so heres a question or two for you:
Yes, I is quite long and very detailed. But it worth reading (... says
the author him self ;-))
> - Have you tried to do a long-lived session such as a large FTP and
> seen how far off the deviation was? That would provide some interesting
> data point.
The deviation can be calculated. The impact is of cause small for large
packets. But the argument that bulk TCP transfers is not as badly
affected, is wrong because all the TCP ACK packets gets maximum penalty.
On an ADSL link with more than 8 bytes overhead, a 40 bytes TCP ACK will
use more that one ATM frame, causing 2 ATM frames to be send that
consumes 106 bytes, eg. 62% overhead. On a small upstream ADSL line
that hurts! (See thesis page 53, table 5.3 "Overhead summary").
> - To be a devil's advocate (and not claim there is no issue), where do
> you draw the line with "overhead"?
> Example the smallest ethernet packet is 64 bytes of which 14 bytes are
> ethernet headers ("overhead" for IP) - and this is not counting CRC etc.
> If you were to set an MTU of say 64 bytes and tried to do a http or ftp,
> how accurate do you think the calculation would be? I would think not
> very different.
I do think we handle this situation, but I'm not quite sure that I fully
understand the question (sorry).
> Does it matter if it is accurate on the majority of the cases?
> - For further reflection: Have you considered the case where the rate
> table has already been considered on some link speed in user space and
> then somewhere post-config the physical link speed changes? This would
> happen in the case where ethernet AN is involved and the partner makes
> some changes (use ethtool).
>
> I would say the last bullet is a more interesting problem than a corner
> case of some link layer technology that has high overhead.
We only claim to do magic on ATM/ADSL links... nothing else ;-)
> Your work would be more interesting if it was generic for many link
> layers instead of just ATM.
Well, we did consider to do so, but we though that it would be harder to
get it into the kernel.
Actually thats the reason for the defines:
#define ATM_CELL_SIZE 53
#define ATM_CELL_PAYLOAD 48
Changing these should should make it possible to adapt to any other SAR
(Segment And Reasembly) link layer.
> On Wed, 2006-14-06 at 11:40 +0200, Jesper Dangaard Brouer wrote:
> > The Linux traffic's control engine inaccurately calculates
> > transmission times for packets sent over ADSL links. For
> > some packet sizes the error rises to over 50%. This occurs
> > because ADSL uses ATM as its link layer transport, and ATM
> > transmits packets in fixed sized 53 byte cells.
> >
> > The following patches to iproute2 and the kernel add an
> > option to calculate traffic transmission times over all
> > ATM links, including ADSL, with perfect accuracy.
> >
> > A longer presentation of the patch, its rational, what it
> > does and how to use it can be found here:
> > http://www.stuart.id.au/russell/files/tc/tc-atm/
> >
> > A earlier version of the patch, and a _detailed_ empirical
> > investigation of its effects can be found here:
> > http://www.adsl-optimizer.dk/
> >
> > The patches are both backwards and forwards compatible.
> > This means unpatched kernels will work with a patched
> > version of iproute2, and an unpatched iproute2 will work
> > on patches kernels.
> >
> >
> > This is a combined effort of Jesper Brouer and Russell Stuart,
> > to get these patches into the upstream kernel.
> >
> > Let the discussion start about what we need to change to get this
> > upstream?
> >
> > We see this as a feature enhancement, as thus hope that it can be
> > queued in davem's net-2.6.18.git tree.
> >
> > ---
> > Regards,
> > Jesper Brouer & Russell Stuart.
> >
>
Thanks for your comments :-)
--
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: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: netif_tx_disable and lockless TX
From: jamal @ 2006-06-14 12:52 UTC (permalink / raw)
To: Robert Olsson
Cc: Andi Kleen, David Miller, mchan, jgarzik, netdev, Herbert Xu
In-Reply-To: <17533.55270.172654.98522@robur.slu.se>
On Wed, 2006-31-05 at 19:52 +0200, Robert Olsson wrote:
> jamal writes:
>
> > Latency-wise: TX completion interrupt provides the best latency.
> > Processing in the poll() -aka softirq- was almost close to the hardirq
> > variant. So if you can make things run in a softirq such as transmit
> > one, then the numbers will likely stay the same.
>
> I don't remember we tried tasklet for TX a la Herbert's suggestion but we
> used use tasklets for controlling RX processing to avoid hardirq livelock
> in pre-NAPI versions.
>
Hrm - it may have been a private thing i did then. I could swear we did
that experiment together ...
Perhaps Herbert's motivation was not really to optimize but rather to
get something unstuck in the transmit path state machine maybe in a
context of netconsole? The conditions for which that tasklet would even
run require a CPU collision to the transmit. Sorry, I didnt quiet follow
the motivation/discussion that ended in that patch.
> Had variants of tulip driver with both TX cleaning at ->poll and TX
> cleaning at hardirq and didn't see any performance difference. The
> ->poll was much cleaner but we kept Alexey's original work for tulip.
>
It certainly is cleaner - but i do recall the hardirq variant had better
latency much observable under high packet rates aka small packets.
> > Sorry, I havent been following discussions on netchannels[1] so i am not
> > qualified to comment on the "replacement" part Dave mentioned earlier.
> > What I can say is the tx processing doesnt have to be part of the NAPI
> > poll() and still use hardirq.
>
> Yes true but I see TX numbers with newer boards (wire rate small pakets)
> with cleaing in ->poll. Also now linux is very safe in network "overload"
> situations. Moving work to hardirq may change that.
>
Oh, I am not suggesting a change - i am a lot more conservative than
that ;-> these areas are delicate (not code-delicate Acme ;->) but
rather what seems obvious requires a lot of experimental results first.
Robert, your transmit results Intel or AMD based?
cheers,
jamal
^ permalink raw reply
* Problems with xfrm (IPSec) and multicast
From: Roar Bjørgum Rotvik @ 2006-06-14 13:09 UTC (permalink / raw)
To: netdev
Hi,
I have configured two Linux PC's to use IPSec to encrypt some mcast traffic, using "ip
xfrm". Each PC has two network cards, one connected to a LAN (unencrypted side, also
called red side) and one connected to the other node (encrypted side, also called black side).
Currently the setup uses static keys in the SA entries, so IKE is not a problem.
|<-- RED side -->|<- BLACK network ->|<-- RED side -->|
+-------+ +---------+ +---------+ +-------+
| LAN A +---+ IPSEC-A +-----------+ IPSEC-B +---+ LAN B |
+-------+ +---------+ +---------+ +-------+
10.0.10.0/24 192.168.0.0/24 10.0.20.0/24
Configuration:
Kernel tested: Linux-2.6.16.13 + 2.6.17-rc4
LAN A: 10.0.10.0/24
LAN B: 10.0.20.0/24
IPSEC-A: RED IP: 10.0.10.1, BLACK IP: 192.168.0.1
IPSEC-B: RED IP: 10.0.20.1, BLACK IP: 192.168.0.2
RED mcast group used: 239.192.20.1
BLACK mcast group used: 239.192.10.1
IPSEC-A SA and SP entries:
[root@ipsec-a ~]# ip xfrm state
src 192.168.0.1 dst 239.192.10.1
proto esp spi 0x00000001 reqid 0 mode tunnel
replay-window 4
auth sha1 0x00000000010000000002000000000301
enc aes 0x000000000000000000000000000000000000000000000001
encap type espinudp sport 4500 dport 4500 addr 0.0.0.0
[root@ipsec-a ~]# ip xfrm policy
src 0.0.0.0/0 dst 239.192.20.1/32
dir in priority 2147483648
tmpl src 192.168.0.1 dst 239.192.10.1
proto esp reqid 0 mode tunnel
src 10.0.10.0/24 dst 239.192.20.1/32
dir out priority 2147483648
tmpl src 192.168.0.1 dst 239.192.10.1
proto esp reqid 0 mode tunnel
src 0.0.0.0/0 dst 239.192.20.1/32
dir fwd priority 2147483648
tmpl src 192.168.0.1 dst 239.192.10.1
proto esp reqid 0 mode tunnel
(The entries for IPSEC-B is similar, but the address 192.168.0.1 is changed to the IPSEC-B
BLACK IP 192.168.0.2).
And I have a small userspace app that opens a socket and bind to port 4500 and issue
setsockopt (fd, SOL_UDP, UDP_ENCAP) so that the kernel will accept UDP encap ESP packets.
When I send multicast traffic from IPSEC-A (bound to the RED interface 10.0.10.1) to mcast
group 239.192.20.1, the traffic matches the out SP entry and is encrypted according to the
SA entry and sent as UDP encap ESP to mcast group 239.192.10.1 on the BLACK network.
On IPSEC-B the UDP encap ESP packet is decrypted and is visible for userspace processes.
So far so good.
But then I start sending similar mcast traffic the other way, but from IPSEC-B (bound to
IPSEC-B RED IP 10.0.20.1). This traffic is also encrypted and sent to IPSEC-A.
But this packet is not decrypted at IPSEC-A, it seems to disappear. The IP and UDP SNMP
counters increase for the received UDP encap ESP packet, but I cannot see what happens to
the packet after the UDP layer. Seems like it is dropped somewhere in XFRM?
By sending some more packets from IPSEC-B (roughly 5-8 more packets), these packets
suddenly starts to be decrypted at IPSEC-A and all is well. Until I start traffic the
other way around again, when the same problem occurs at IPSEC-B.
So I cannot make encrypted multicast traffic to flow both ways at the same time, and has
no clue as to why the first packets after changing direction is dropped somewhere.
Anyone have a clue to this observed problem with linux xfrm and multicast or a better
solution for encrypted multicast on linux 2.6.x?
Any help with this is appreciated and more info (tcpdump, snmp stats and so on) can be
obtained if needed.
--
Roar Bjørgum Rotvik
^ permalink raw reply
* Re: [PATCH 2/2] NET: Accurate packet scheduling for ATM/ADSL (userspace)
From: Jesper Dangaard Brouer @ 2006-06-14 13:18 UTC (permalink / raw)
To: Alan Cox
Cc: Stephen Hemminger, Jamal Hadi Salim, netdev, lartc, russell-tcatm,
hawk, linux-kernel, hawk
In-Reply-To: <1150282625.3490.23.camel@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 1552 bytes --]
On Wed, 2006-06-14 at 11:57 +0100, Alan Cox wrote:
> Ar Mer, 2006-06-14 am 11:40 +0200, ysgrifennodd Jesper Dangaard Brouer:
> > option to calculate traffic transmission times (rate table)
> > over all ATM links, including ADSL, with perfect accuracy.
>
> The other problem I see with this code is it is very tightly tied to ATM
> cell sizes, not to solving the generic question of packetisation.
Well, we did consider to do so, but we though that it would be harder to
get it into the kernel.
Actually thats the reason for the defines:
#define ATM_CELL_SIZE 53
#define ATM_CELL_PAYLOAD 48
Changing these should should make it possible to adapt to any other SAR
(Segment And Reasembly) link layer.
> I'm
> not sure if that matters but for modern processors I'm also sceptical
> that the clever computation is actually any faster than just doing the
> maths, especially if something cache intensive is also running.
I guess you are refering to the rate table lookup system, that is based
upon array lookups. I do think that the rate table array lookup system
has been outdated, as memory access is the bottleneck on modern CPUs.
But its design by Alexey for a long time ago where the hardware
restrictions were different. It also avoids floting point operations in
the kernel.
Thanks for your comments.
--
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: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: driver for pptp
From: Harald Welte @ 2006-06-14 13:18 UTC (permalink / raw)
To: xeb; +Cc: netdev
In-Reply-To: <E1FmQDD-0005kY-00.xeb-mail-ru@f51.mail.ru>
[-- Attachment #1: Type: text/plain, Size: 2120 bytes --]
On Sat, Jun 03, 2006 at 11:06:19AM +0400, xeb@mail.ru wrote:
> I have developed the driver for Point-to-Point Tunneling Protocol (PPTP).
great news. something that I always thought of a nice-to-have.
> I have published the project on http://accel-pptp.sourceforge.net/
Please don't expect Linux Kernel networking developers to actually go to
sourceforge download and extract code that you want to have
reviewed/submitted.
Please read Documentation/SubmittingPatches (and CodingStyle) and submit
your kernel patch to netdev.
> Hope this driver will go to a kernel tree and will make linux more productive.
not without you pushing it actively and getting through review cycles
(which I hope you will!).
Some initial comments:
1) why wasn't it possible to use the PPPoX infrastructure of the kernel
which is already being used by PPPoE ? Or at least model it somehow
similar to the existing PPPoE/PPPoX infrastructure?
2) why are you using a timer for asynchronous processing of GRE frames?
First of all, why does it have to happen asynchronously at all?
Secondly, why using a timer when there's nothing time related (or do
I miss something)? If deferred, out-of-context execution is
required, there are other primitives such as tasklets.
3) you conflict with the ip_gre.c genric GRE encapsulation driver. this
is because both want to reigster a proto handler for GRE. Ideally,
there needs to be another demultiplex between the GRE protocl and its
users. The code registered for GRE would look at the packet and
determine whether e.g. it is a PPTP GRE packet and then pass it on to
the pptp module.
4) your code doesn't look nonlinear skb clean
5) why did you chose to implement /dev/pptp rather than a socket family
like the existing pppox/pppoe code?
6) lots of codingstyle issues
--
- Harald Welte <laforge@gnumonks.org> http://gnumonks.org/
============================================================================
We all know Linux is great...it does infinite loops in 5 seconds. -- Linus
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: PATCHv3 2.6.17-rc5 tulip free_irq() called too late
From: Kyle McMartin @ 2006-06-14 13:05 UTC (permalink / raw)
To: Grant Grundler; +Cc: Jeff Garzik, Valerie Henson, Andrew Morton, netdev
In-Reply-To: <20060614044412.GA30552@colo.lackof.org>
On Tue, Jun 13, 2006 at 10:44:12PM -0600, Grant Grundler wrote:
> On Tue, Jun 13, 2006 at 08:33:22PM -0400, Jeff Garzik wrote:
> > Grant Grundler wrote:
> > >o tulip_stop_rxtx() has to be called _after_ free_irq().
> > > ie. v2 patch didn't fix the original race condition
> > > and when under test, dies about as fast as the original code.
> >
> > You made the race window smaller, but it's still there. The chip's DMA
> > engines should be stopped before you unregister the interrupt handler.
>
> Switching the order to be:
> tulip_stop_rxtx(tp); /* Stop DMA */
> free_irq (dev->irq, dev); /* no more races after this */
>
I think the correct sequence would be:
reset tulip interrupt mask
flush posted write
synchronize irq /* make sure we got 'em all */
tulip_stop_rxtx /* turn off dma */
free irq /* bye bye */
The synchronize irq guarantees we shouldn't see another irq
generated by the card because it was held up somewhere.
Cheers,
Kyle M.
^ permalink raw reply
* Re: [RFC/PATCH 1/2] in-kernel sockets API
From: Harald Welte @ 2006-06-14 13:30 UTC (permalink / raw)
To: Daniel Phillips
Cc: bidulock, Stephen Hemminger, Sridhar Samudrala, netdev,
linux-kernel
In-Reply-To: <448F2A49.5020809@google.com>
[-- Attachment #1: Type: text/plain, Size: 955 bytes --]
On Tue, Jun 13, 2006 at 02:12:41PM -0700, Daniel Phillips wrote:
> This has the makings of a nice stable internal kernel api. Why do we want
> to provide this nice stable internal api to proprietary modules?
because there is IMHO legally nothing we can do about it anyway. Use of
an industry-standard API that is provided in multiple operating system
is one of the clearest idnication of some program _not_ being a
derivative work.
Whether we like it or not, it doesn't really matter if we export them
GPL-only or not. Anybody using those scoket API calls will be having an
easy time arguing in favor of non-derivative work.
The GPL doesn't extend beyon what copyright law allows you to do...
--
- Harald Welte <laforge@gnumonks.org> http://gnumonks.org/
============================================================================
We all know Linux is great...it does infinite loops in 5 seconds. -- Linus
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Offering presents for more routing tables!
From: Ben Greear @ 2006-06-14 13:45 UTC (permalink / raw)
To: NetDev
I've asked for this feature several years ago and evidently
it is not trivial to increase the number of routing tables.
But, perhaps someone now has time & inclination?
I would like to have more (a few thousand) routing tables
available in the kernel so that I can use a routing table for
each of my many VLANs. Currently, the netlink protocol only
specifies an 8-bit id for the routing table:
http://www.faqs.org/rfcs/rfc3549.html Section 3.1.1
so a new netlink message would need to be created and the 'ip' tool
updated. I think at least a 16-bit identifier should be used,
possibly a full 32 bits so we don't have to revisit this again for
a while!
The kernel itself would also need to be modified so that it can
have more routing tables. I realize most people don't need a
large number of tables, so the maximum number should be configured
at either compile time or run time.
If I remember right, there are certain tables (253 - 255) that
are currently special in the kernel. For complete backwards compatibility,
this hole would probably have to remain as it is, with the new
tables starting at 256.
I would be willing to help test any resulting patches, and can
also offer bribes of money, hardware, beer, etc.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: http://bugzilla.kernel.org/show_bug.cgi?id=6197
From: Patrick McHardy @ 2006-06-14 14:04 UTC (permalink / raw)
To: Michael Tokarev; +Cc: netdev
In-Reply-To: <448F304A.2020808@tls.msk.ru>
Michael Tokarev wrote:
> Patrick McHardy wrote:
> []
>
>>He patched his kernel with the IMQ device, which is known to cause all
>>kinds of weird problems.
>
>
> Wich problems? Known to whom?
Known to me (who wrote the original implementation of the current IMQ
device) and numerous people who were hit by them. IIRC it does some
invalid skb refcounting hacks which result in crashes in certain
scenarios - but I don't remeber the exact details.
> I was considering using imq for our needs (not done yet), and from the
> FAQ at http://www.linuximq.net/faq.html (item #3, "Is it stable?") it
> seems there's no problems except of gre tunnels and locally generated
> traffic...
>
> Googling for "imq linux problem" shows usual pile of various user
> support questions (how to configure.. what did I do wrong.. etc),
> but nothing relevant.
The lartc list had lots of reports of crashes. I guess "imq crash" or
"imq oops" will give better results.
> So... I'm curious whenever the claim on linuximq.net site about the
> stability is true, or there in fact are some real issue...
>From what I know these problems haven't been fixed. Current kernels
include Jamal's tc actions and the ifb-Device, which obsolete IMQ
anyway.
^ permalink raw reply
* Re: [PATCH 0/2] NET: Accurate packet scheduling for ATM/ADSL
From: Phillip Susi @ 2006-06-14 14:27 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Stephen Hemminger, Jamal Hadi Salim, netdev, lartc, russell-tcatm,
hawk, linux-kernel
In-Reply-To: <1150278004.26181.35.camel@localhost.localdomain>
Jesper Dangaard Brouer wrote:
> The Linux traffic's control engine inaccurately calculates
> transmission times for packets sent over ADSL links. For
> some packet sizes the error rises to over 50%. This occurs
> because ADSL uses ATM as its link layer transport, and ATM
> transmits packets in fixed sized 53 byte cells.
>
I could have sworn that DSL uses its own framing protocol that is
similar to the frame/superframe structure of HDSL ( T1 ) lines and over
that you can run ATM or ethernet. Or is it typically ethernet -> ATM ->
HDSL?
In any case, why does the kernel care about the exact time that the IP
packet has been received and reassembled on the headend?
^ permalink raw reply
* Re: [RFC/PATCH 1/2] in-kernel sockets API
From: Erik Mouw @ 2006-06-14 14:29 UTC (permalink / raw)
To: Harald Welte
Cc: Daniel Phillips, bidulock, Stephen Hemminger, Sridhar Samudrala,
netdev, linux-kernel
In-Reply-To: <20060614133022.GU11863@sunbeam.de.gnumonks.org>
On Wed, Jun 14, 2006 at 03:30:22PM +0200, Harald Welte wrote:
> On Tue, Jun 13, 2006 at 02:12:41PM -0700, Daniel Phillips wrote:
>
> > This has the makings of a nice stable internal kernel api. Why do we want
> > to provide this nice stable internal api to proprietary modules?
>
> because there is IMHO legally nothing we can do about it anyway. Use of
> an industry-standard API that is provided in multiple operating system
> is one of the clearest idnication of some program _not_ being a
> derivative work.
IMHO there is no industry-standard API for in-kernel use of sockets.
There is however one for user space.
Erik
(IANAL, etc)
--
+-- Erik Mouw -- www.harddisk-recovery.com -- +31 70 370 12 90 --
| Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands
^ permalink raw reply
* Re: PATCHv3 2.6.17-rc5 tulip free_irq() called too late
From: Grant Grundler @ 2006-06-14 14:54 UTC (permalink / raw)
To: Kyle McMartin
Cc: Grant Grundler, Jeff Garzik, Valerie Henson, Andrew Morton,
netdev
In-Reply-To: <20060614130506.GA8556@skunkworks.cabal.ca>
On Wed, Jun 14, 2006 at 09:05:06AM -0400, Kyle McMartin wrote:
> I think the correct sequence would be:
>
> reset tulip interrupt mask
> flush posted write
>
> synchronize irq /* make sure we got 'em all */
> tulip_stop_rxtx /* turn off dma */
> free irq /* bye bye */
>
> The synchronize irq guarantees we shouldn't see another irq
> generated by the card because it was held up somewhere.
Kyle,
syncronize_irq() only guarantees currently executing interrupt handler
completes before handing control back to the caller.
It does not guarantee IRQ signals still inflight are "flushed".
Remember that IRQ lines are a "sideband" signal and not subject
to PCI data ordering rules.
thanks,
grant
^ permalink raw reply
* Re: PATCHv3 2.6.17-rc5 tulip free_irq() called too late
From: Jeff Garzik @ 2006-06-14 15:03 UTC (permalink / raw)
To: Grant Grundler; +Cc: Valerie Henson, Andrew Morton, netdev
In-Reply-To: <20060614044412.GA30552@colo.lackof.org>
Grant Grundler wrote:
> On Tue, Jun 13, 2006 at 08:33:22PM -0400, Jeff Garzik wrote:
>> Grant Grundler wrote:
>>> o tulip_stop_rxtx() has to be called _after_ free_irq().
>>> ie. v2 patch didn't fix the original race condition
>>> and when under test, dies about as fast as the original code.
>> You made the race window smaller, but it's still there. The chip's DMA
>> engines should be stopped before you unregister the interrupt handler.
>
> Switching the order to be:
> tulip_stop_rxtx(tp); /* Stop DMA */
> free_irq (dev->irq, dev); /* no more races after this */
>
> still leaves us open to IRQs being delivered _after_ we've stopped DMA.
Correct. And that is the preferred, natural, logical, obvious order:
1) Turn things off.
2) Wait for activity to cease.
> That in turn allows the interrupt handler to re-enable DMA again.
Then that would be a problem to solve... Some interrupt handlers will
test netif_running() or a driver-specific shutting-down flag,
specifically to avoid such behaviors.
Jeff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox