* [Codel] [PATCH 1/2] codel: Controlled Delay AQM
@ 2012-05-05 19:32 Dave Taht
2012-05-05 19:32 ` [Codel] [PATCH 2/2] codel: RED is dead Dave Taht
` (2 more replies)
0 siblings, 3 replies; 32+ messages in thread
From: Dave Taht @ 2012-05-05 19:32 UTC (permalink / raw)
To: codel; +Cc: Eric Dumazet, Dave Täht
From: Dave Täht <dave.taht@bufferbloat.net>
tc qdisc ... codel [ limit PACKETS ] [ target TIME]
[ interval TIME ] [ minbytes BYTES ]
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Dave Täht <dave.taht@bufferbloat.net>
---
include/linux/pkt_sched.h | 14 +++++
tc/Makefile | 1 +
tc/q_codel.c | 136 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 151 insertions(+)
create mode 100644 tc/q_codel.c
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 410b33d..62a73bf 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -509,6 +509,7 @@ enum {
TCA_NETEM_CORRUPT,
TCA_NETEM_LOSS,
TCA_NETEM_RATE,
+ TCA_NETEM_ECN,
__TCA_NETEM_MAX,
};
@@ -654,4 +655,17 @@ struct tc_qfq_stats {
__u32 lmax;
};
+/* CODEL */
+
+enum {
+ TCA_CODEL_UNSPEC,
+ TCA_CODEL_TARGET,
+ TCA_CODEL_LIMIT,
+ TCA_CODEL_MINBYTES,
+ TCA_CODEL_INTERVAL,
+ __TCA_CODEL_MAX
+};
+
+#define TCA_CODEL_MAX (__TCA_CODEL_MAX - 1)
+
#endif
diff --git a/tc/Makefile b/tc/Makefile
index be8cd5a..8a7cc8d 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -47,6 +47,7 @@ TCMODULES += em_cmp.o
TCMODULES += em_u32.o
TCMODULES += em_meta.o
TCMODULES += q_mqprio.o
+TCMODULES += q_codel.o
TCSO :=
ifeq ($(TC_CONFIG_ATM),y)
diff --git a/tc/q_codel.c b/tc/q_codel.c
new file mode 100644
index 0000000..0a7cf9f
--- /dev/null
+++ b/tc/q_codel.c
@@ -0,0 +1,136 @@
+/*
+ * q_codel.c Codel.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Authors: Eric Dumazet <edumazet@google.com>
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <syslog.h>
+#include <fcntl.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <string.h>
+#include <math.h>
+
+#include "utils.h"
+#include "tc_util.h"
+
+static void explain(void)
+{
+ fprintf(stderr, "Usage: ... codel [ limit PACKETS ] [ target TIME]\n");
+ fprintf(stderr, " [ interval TIME ] [ minbytes BYTES ]\n");
+}
+
+static int codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
+ struct nlmsghdr *n)
+{
+ struct tc_red_qopt opt;
+ unsigned limit = 0;
+ unsigned target = 0;
+ unsigned interval = 0;
+ unsigned minbytes = 0;
+ struct rtattr *tail;
+
+ while (argc > 0) {
+ if (strcmp(*argv, "limit") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&limit, *argv, 0)) {
+ fprintf(stderr, "Illegal \"limit\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "minbytes") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&minbytes, *argv, 0)) {
+ fprintf(stderr, "Illegal \"minbytes\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "target") == 0) {
+ NEXT_ARG();
+ if (get_time(&target, *argv)) {
+ fprintf(stderr, "Illegal \"target\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "interval") == 0) {
+ NEXT_ARG();
+ if (get_time(&interval, *argv)) {
+ fprintf(stderr, "Illegal \"interval\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "help") == 0) {
+ explain();
+ return -1;
+ } else {
+ fprintf(stderr, "What is \"%s\"?\n", *argv);
+ explain();
+ return -1;
+ }
+ argc--; argv++;
+ }
+
+ tail = NLMSG_TAIL(n);
+ addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+
+ if (limit)
+ addattr_l(n, 1024, TCA_CODEL_LIMIT, &limit, sizeof(limit));
+ if (minbytes)
+ addattr_l(n, 1024, TCA_CODEL_MINBYTES, &minbytes, sizeof(minbytes));
+ if (interval)
+ addattr_l(n, 1024, TCA_CODEL_INTERVAL, &interval, sizeof(interval));
+ if (target)
+ addattr_l(n, 1024, TCA_CODEL_TARGET, &target, sizeof(target));
+ tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+ return 0;
+}
+
+static int codel_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
+{
+ struct rtattr *tb[TCA_CODEL_MAX + 1];
+ unsigned limit;
+ unsigned interval;
+ unsigned target;
+ unsigned minbytes;
+ SPRINT_BUF(b1);
+
+ if (opt == NULL)
+ return 0;
+
+ parse_rtattr_nested(tb, TCA_CODEL_MAX, opt);
+
+ if (tb[TCA_CODEL_LIMIT] &&
+ RTA_PAYLOAD(tb[TCA_CODEL_LIMIT]) >= sizeof(__u32)) {
+ limit = rta_getattr_u32(tb[TCA_CODEL_LIMIT]);
+ fprintf(f, "limit %up ", limit);
+ }
+ if (tb[TCA_CODEL_MINBYTES] &&
+ RTA_PAYLOAD(tb[TCA_CODEL_MINBYTES]) >= sizeof(__u32)) {
+ minbytes = rta_getattr_u32(tb[TCA_CODEL_MINBYTES]);
+ fprintf(f, "minbytes %u ", minbytes);
+ }
+ if (tb[TCA_CODEL_TARGET] &&
+ RTA_PAYLOAD(tb[TCA_CODEL_TARGET]) >= sizeof(__u32)) {
+ target = rta_getattr_u32(tb[TCA_CODEL_TARGET]);
+ fprintf(f, "target %s ", sprint_time(target, b1));
+ }
+ if (tb[TCA_CODEL_INTERVAL] &&
+ RTA_PAYLOAD(tb[TCA_CODEL_INTERVAL]) >= sizeof(__u32)) {
+ interval = rta_getattr_u32(tb[TCA_CODEL_INTERVAL]);
+ fprintf(f, "interval %s ", sprint_time(interval, b1));
+ }
+
+ return 0;
+}
+
+
+struct qdisc_util codel_qdisc_util = {
+ .id = "codel",
+ .parse_qopt = codel_parse_opt,
+ .print_qopt = codel_print_opt,
+};
--
1.7.9.5
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Codel] [PATCH 2/2] codel: RED is dead
2012-05-05 19:32 [Codel] [PATCH 1/2] codel: Controlled Delay AQM Dave Taht
@ 2012-05-05 19:32 ` Dave Taht
2012-05-05 19:47 ` [Codel] [PATCH 1/2] codel: Controlled Delay AQM Dave Taht
2012-05-07 16:43 ` Rick Jones
2 siblings, 0 replies; 32+ messages in thread
From: Dave Taht @ 2012-05-05 19:32 UTC (permalink / raw)
To: codel; +Cc: Dave Täht
From: Dave Täht <dave.taht@bufferbloat.net>
Removed unused struct referencing now-obsolete queuing discipline.
Signed-off-by: Dave Täht <dave.taht@bufferbloat.net>
---
tc/q_codel.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tc/q_codel.c b/tc/q_codel.c
index 0a7cf9f..63cc583 100644
--- a/tc/q_codel.c
+++ b/tc/q_codel.c
@@ -32,7 +32,6 @@ static void explain(void)
static int codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
struct nlmsghdr *n)
{
- struct tc_red_qopt opt;
unsigned limit = 0;
unsigned target = 0;
unsigned interval = 0;
--
1.7.9.5
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-05 19:32 [Codel] [PATCH 1/2] codel: Controlled Delay AQM Dave Taht
2012-05-05 19:32 ` [Codel] [PATCH 2/2] codel: RED is dead Dave Taht
@ 2012-05-05 19:47 ` Dave Taht
2012-05-05 20:05 ` Eric Dumazet
2012-05-07 16:43 ` Rick Jones
2 siblings, 1 reply; 32+ messages in thread
From: Dave Taht @ 2012-05-05 19:47 UTC (permalink / raw)
To: Dave Taht; +Cc: Eric Dumazet, codel
On Sat, May 5, 2012 at 12:32 PM, Dave Taht <dave.taht@bufferbloat.net> wrote:
> From: Dave Täht <dave.taht@bufferbloat.net>
>
> tc qdisc ... codel [ limit PACKETS ] [ target TIME]
> [ interval TIME ] [ minbytes BYTES ]
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Dave Täht <dave.taht@bufferbloat.net>
> ---
> include/linux/pkt_sched.h | 14 +++++
> tc/Makefile | 1 +
> tc/q_codel.c | 136 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 151 insertions(+)
> create mode 100644 tc/q_codel.c
>
> diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
> index 410b33d..62a73bf 100644
> --- a/include/linux/pkt_sched.h
> +++ b/include/linux/pkt_sched.h
> @@ -509,6 +509,7 @@ enum {
> TCA_NETEM_CORRUPT,
> TCA_NETEM_LOSS,
> TCA_NETEM_RATE,
> + TCA_NETEM_ECN,
> __TCA_NETEM_MAX,
> };
>
> @@ -654,4 +655,17 @@ struct tc_qfq_stats {
> __u32 lmax;
> };
>
> +/* CODEL */
> +
> +enum {
> + TCA_CODEL_UNSPEC,
> + TCA_CODEL_TARGET,
> + TCA_CODEL_LIMIT,
> + TCA_CODEL_MINBYTES,
> + TCA_CODEL_INTERVAL,
> + __TCA_CODEL_MAX
> +};
> +
> +#define TCA_CODEL_MAX (__TCA_CODEL_MAX - 1)
> +
> #endif
> diff --git a/tc/Makefile b/tc/Makefile
> index be8cd5a..8a7cc8d 100644
> --- a/tc/Makefile
> +++ b/tc/Makefile
> @@ -47,6 +47,7 @@ TCMODULES += em_cmp.o
> TCMODULES += em_u32.o
> TCMODULES += em_meta.o
> TCMODULES += q_mqprio.o
> +TCMODULES += q_codel.o
>
> TCSO :=
> ifeq ($(TC_CONFIG_ATM),y)
> diff --git a/tc/q_codel.c b/tc/q_codel.c
> new file mode 100644
> index 0000000..0a7cf9f
> --- /dev/null
> +++ b/tc/q_codel.c
> @@ -0,0 +1,136 @@
> +/*
> + * q_codel.c Codel.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + * Authors: Eric Dumazet <edumazet@google.com>
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <syslog.h>
> +#include <fcntl.h>
> +#include <sys/socket.h>
> +#include <netinet/in.h>
> +#include <arpa/inet.h>
> +#include <string.h>
> +#include <math.h>
> +
> +#include "utils.h"
> +#include "tc_util.h"
> +
> +static void explain(void)
> +{
> + fprintf(stderr, "Usage: ... codel [ limit PACKETS ] [ target TIME]\n");
> + fprintf(stderr, " [ interval TIME ] [ minbytes BYTES ]\n");
> +}
> +
> +static int codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
> + struct nlmsghdr *n)
> +{
> + struct tc_red_qopt opt;
> + unsigned limit = 0;
> + unsigned target = 0;
> + unsigned interval = 0;
> + unsigned minbytes = 0;
> + struct rtattr *tail;
> +
> + while (argc > 0) {
> + if (strcmp(*argv, "limit") == 0) {
> + NEXT_ARG();
> + if (get_unsigned(&limit, *argv, 0)) {
> + fprintf(stderr, "Illegal \"limit\"\n");
> + return -1;
> + }
> + } else if (strcmp(*argv, "minbytes") == 0) {
> + NEXT_ARG();
> + if (get_unsigned(&minbytes, *argv, 0)) {
> + fprintf(stderr, "Illegal \"minbytes\"\n");
> + return -1;
> + }
> + } else if (strcmp(*argv, "target") == 0) {
> + NEXT_ARG();
> + if (get_time(&target, *argv)) {
> + fprintf(stderr, "Illegal \"target\"\n");
> + return -1;
> + }
> + } else if (strcmp(*argv, "interval") == 0) {
> + NEXT_ARG();
> + if (get_time(&interval, *argv)) {
> + fprintf(stderr, "Illegal \"interval\"\n");
> + return -1;
> + }
> + } else if (strcmp(*argv, "help") == 0) {
> + explain();
> + return -1;
> + } else {
> + fprintf(stderr, "What is \"%s\"?\n", *argv);
> + explain();
> + return -1;
> + }
> + argc--; argv++;
> + }
> +
> + tail = NLMSG_TAIL(n);
> + addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
> +
> + if (limit)
> + addattr_l(n, 1024, TCA_CODEL_LIMIT, &limit, sizeof(limit));
> + if (minbytes)
> + addattr_l(n, 1024, TCA_CODEL_MINBYTES, &minbytes, sizeof(minbytes));
> + if (interval)
> + addattr_l(n, 1024, TCA_CODEL_INTERVAL, &interval, sizeof(interval));
> + if (target)
> + addattr_l(n, 1024, TCA_CODEL_TARGET, &target, sizeof(target));
> + tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
> + return 0;
> +}
> +
> +static int codel_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
> +{
> + struct rtattr *tb[TCA_CODEL_MAX + 1];
> + unsigned limit;
> + unsigned interval;
> + unsigned target;
> + unsigned minbytes;
> + SPRINT_BUF(b1);
> +
> + if (opt == NULL)
> + return 0;
> +
> + parse_rtattr_nested(tb, TCA_CODEL_MAX, opt);
> +
> + if (tb[TCA_CODEL_LIMIT] &&
> + RTA_PAYLOAD(tb[TCA_CODEL_LIMIT]) >= sizeof(__u32)) {
> + limit = rta_getattr_u32(tb[TCA_CODEL_LIMIT]);
> + fprintf(f, "limit %up ", limit);
> + }
> + if (tb[TCA_CODEL_MINBYTES] &&
> + RTA_PAYLOAD(tb[TCA_CODEL_MINBYTES]) >= sizeof(__u32)) {
> + minbytes = rta_getattr_u32(tb[TCA_CODEL_MINBYTES]);
> + fprintf(f, "minbytes %u ", minbytes);
> + }
> + if (tb[TCA_CODEL_TARGET] &&
> + RTA_PAYLOAD(tb[TCA_CODEL_TARGET]) >= sizeof(__u32)) {
> + target = rta_getattr_u32(tb[TCA_CODEL_TARGET]);
> + fprintf(f, "target %s ", sprint_time(target, b1));
> + }
> + if (tb[TCA_CODEL_INTERVAL] &&
> + RTA_PAYLOAD(tb[TCA_CODEL_INTERVAL]) >= sizeof(__u32)) {
> + interval = rta_getattr_u32(tb[TCA_CODEL_INTERVAL]);
> + fprintf(f, "interval %s ", sprint_time(interval, b1));
> + }
> +
> + return 0;
> +}
> +
> +
> +struct qdisc_util codel_qdisc_util = {
> + .id = "codel",
> + .parse_qopt = codel_parse_opt,
> + .print_qopt = codel_print_opt,
> +};
> --
> 1.7.9.5
I note that your recent work on ECN for netem crept in, and it should
probably stay in whatever patches exist out of tree for it.
Please feel free to rework these two patches into one, add my
signed-off-by, and resubmit.
--
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
http://www.bufferbloat.net
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-05 19:47 ` [Codel] [PATCH 1/2] codel: Controlled Delay AQM Dave Taht
@ 2012-05-05 20:05 ` Eric Dumazet
0 siblings, 0 replies; 32+ messages in thread
From: Eric Dumazet @ 2012-05-05 20:05 UTC (permalink / raw)
To: Dave Taht; +Cc: Eric Dumazet, codel, Dave Taht
On Sat, 2012-05-05 at 12:47 -0700, Dave Taht wrote:
> I note that your recent work on ECN for netem crept in, and it should
> probably stay in whatever patches exist out of tree for it.
> Please feel free to rework these two patches into one, add my
> signed-off-by, and resubmit.
>
Dont worry : Stephen usually syncs the include file with the kernel one,
he will do this before applying the netem bits or codel bits.
Also I redid the whole tc patch copying choke file (instead of trying to
fix your first tc codel patch), but I'll add you as co-author of course.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-05 19:32 [Codel] [PATCH 1/2] codel: Controlled Delay AQM Dave Taht
2012-05-05 19:32 ` [Codel] [PATCH 2/2] codel: RED is dead Dave Taht
2012-05-05 19:47 ` [Codel] [PATCH 1/2] codel: Controlled Delay AQM Dave Taht
@ 2012-05-07 16:43 ` Rick Jones
2012-05-07 17:10 ` Eric Dumazet
2 siblings, 1 reply; 32+ messages in thread
From: Rick Jones @ 2012-05-07 16:43 UTC (permalink / raw)
To: Dave Taht; +Cc: Eric Dumazet, codel
On 05/05/2012 12:32 PM, Dave Taht wrote:
> From: Dave Täht<dave.taht@bufferbloat.net>
>
> tc qdisc ... codel [ limit PACKETS ] [ target TIME]
> [ interval TIME ] [ minbytes BYTES ]
Usability suggestions - "target_latency" or "target_lat" and
"action_interval" or "decision_interval" perhaps?
rick
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 16:43 ` Rick Jones
@ 2012-05-07 17:10 ` Eric Dumazet
2012-05-07 17:18 ` Dave Taht
2012-05-07 17:27 ` Rick Jones
0 siblings, 2 replies; 32+ messages in thread
From: Eric Dumazet @ 2012-05-07 17:10 UTC (permalink / raw)
To: Rick Jones; +Cc: Eric Dumazet, codel, Dave Taht
On Mon, 2012-05-07 at 09:43 -0700, Rick Jones wrote:
> On 05/05/2012 12:32 PM, Dave Taht wrote:
> > From: Dave Täht<dave.taht@bufferbloat.net>
> >
> > tc qdisc ... codel [ limit PACKETS ] [ target TIME]
> > [ interval TIME ] [ minbytes BYTES ]
>
> Usability suggestions - "target_latency" or "target_lat" and
> "action_interval" or "decision_interval" perhaps?
We took the terms used in the codel paper.
By the way, target is not target_latency, its more a target threshold
(packets under it are not dropped), but packets above might be dropped.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 17:10 ` Eric Dumazet
@ 2012-05-07 17:18 ` Dave Taht
2012-05-07 17:27 ` Rick Jones
1 sibling, 0 replies; 32+ messages in thread
From: Dave Taht @ 2012-05-07 17:18 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, codel, Dave Taht
On Mon, May 7, 2012 at 10:10 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2012-05-07 at 09:43 -0700, Rick Jones wrote:
>> On 05/05/2012 12:32 PM, Dave Taht wrote:
>> > From: Dave Täht<dave.taht@bufferbloat.net>
>> >
>> > tc qdisc ... codel [ limit PACKETS ] [ target TIME]
>> > [ interval TIME ] [ minbytes BYTES ]
>>
>> Usability suggestions - "target_latency" or "target_lat" and
>> "action_interval" or "decision_interval" perhaps?
>
> We took the terms used in the codel paper.
>
> By the way, target is not target_latency, its more a target threshold
> (packets under it are not dropped), but packets above might be dropped.
Well, we did change from the paper which used 'maxpacket' rather than 'minbytes'
Minbytes is basically the mtu. I wanted something more abstract than 'mtu',
I felt maxpacket was confusing as it indicated packets
max_packet_size sort of makes sense, but... I went with minbytes.
Perhaps the paper can be changed to suit.
>
>
>
> _______________________________________________
> Codel mailing list
> Codel@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/codel
--
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
http://www.bufferbloat.net
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 17:10 ` Eric Dumazet
2012-05-07 17:18 ` Dave Taht
@ 2012-05-07 17:27 ` Rick Jones
2012-05-07 17:34 ` Eric Dumazet
1 sibling, 1 reply; 32+ messages in thread
From: Rick Jones @ 2012-05-07 17:27 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, codel, Dave Taht
On 05/07/2012 10:10 AM, Eric Dumazet wrote:
> On Mon, 2012-05-07 at 09:43 -0700, Rick Jones wrote:
>> On 05/05/2012 12:32 PM, Dave Taht wrote:
>>> From: Dave Täht<dave.taht@bufferbloat.net>
>>>
>>> tc qdisc ... codel [ limit PACKETS ] [ target TIME]
>>> [ interval TIME ] [ minbytes BYTES ]
>>
>> Usability suggestions - "target_latency" or "target_lat" and
>> "action_interval" or "decision_interval" perhaps?
>
> We took the terms used in the codel paper.
>
> By the way, target is not target_latency, its more a target threshold
> (packets under it are not dropped), but packets above might be dropped.
I don't wish to rathole, and someone may want to declare Bikeshed! but
the target audience of the paper is a bit different from the Linux
system administrator base yes? If a parameter name enhancement makes
something more clear to an administrator is it really a bad thing to
have a name that is not a strict duplicate of the paper's?
rick
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 17:27 ` Rick Jones
@ 2012-05-07 17:34 ` Eric Dumazet
2012-05-07 17:52 ` Dave Taht
0 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2012-05-07 17:34 UTC (permalink / raw)
To: Rick Jones; +Cc: Eric Dumazet, codel, Dave Taht
On Mon, 2012-05-07 at 10:27 -0700, Rick Jones wrote:
>
> I don't wish to rathole, and someone may want to declare Bikeshed! but
> the target audience of the paper is a bit different from the Linux
> system administrator base yes? If a parameter name enhancement makes
> something more clear to an administrator is it really a bad thing to
> have a name that is not a strict duplicate of the paper's?
>
> rick
I dont know, we have no precedence in linux for 'target' and 'interval'
So picking the codel words seems ok
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 17:34 ` Eric Dumazet
@ 2012-05-07 17:52 ` Dave Taht
2012-05-07 18:01 ` Eric Dumazet
2012-05-07 18:32 ` Jim Gettys
0 siblings, 2 replies; 32+ messages in thread
From: Dave Taht @ 2012-05-07 17:52 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, codel, Dave Taht
On Mon, May 7, 2012 at 10:34 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2012-05-07 at 10:27 -0700, Rick Jones wrote:
>
>>
>> I don't wish to rathole, and someone may want to declare Bikeshed! but
>> the target audience of the paper is a bit different from the Linux
>> system administrator base yes? If a parameter name enhancement makes
>> something more clear to an administrator is it really a bad thing to
>> have a name that is not a strict duplicate of the paper's?
>>
>> rick
>
> I dont know, we have no precedence in linux for 'target' and 'interval'
>
> So picking the codel words seems ok
I like short names from a readability perspective. I'm not big on underscores,
they are hard to see. (I'm getting old)
I note that codel is intended to run in most cases without any parameters.
limit: used elsewhere in linux for a packet limit. A packet limit is there for
practical reasons, shrinking the queue has overhead. However, codel will
run just fine with infinite queue (yea!), and I suspect the default
1000 is too small
on 10GigE. If there was a way to autotune it based on the underlying
device, that would be good.
target: At the moment this is a knob because 5ms is an eternity at 10GigE
interval: Same reason
I'm not big on changing these from the paper, nor on _s, nor on length.
However, I'm open to further discussion.
maxpacket: really confusing usages, so I went with minbytes which is explicit
Any objections?
(min was overloaded with red's usage, so I felt that would be confusing, too)
ecn:
ecn was not in the paper (and it's working well) and I'd like to live
in a world where ecn was the default. I think. Others disagree.
so perhaps ecn and noecn? And ecn be the default (after some serious testing?)
Parameterless is a good goal.
>
>
>
> _______________________________________________
> Codel mailing list
> Codel@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/codel
--
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
http://www.bufferbloat.net
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 17:52 ` Dave Taht
@ 2012-05-07 18:01 ` Eric Dumazet
2012-05-07 18:09 ` Eric Dumazet
2012-05-07 18:32 ` Jim Gettys
1 sibling, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2012-05-07 18:01 UTC (permalink / raw)
To: Dave Taht; +Cc: Eric Dumazet, codel, Dave Taht
On Mon, 2012-05-07 at 10:52 -0700, Dave Taht wrote:
> so perhaps ecn and noecn? And ecn be the default (after some serious testing?)
This is what I coded.
Oh I forgot to send iproute2 patch it seems
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 18:01 ` Eric Dumazet
@ 2012-05-07 18:09 ` Eric Dumazet
2012-05-07 18:15 ` Eric Dumazet
0 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2012-05-07 18:09 UTC (permalink / raw)
To: Dave Taht; +Cc: Eric Dumazet, codel, Dave Taht
On Mon, 2012-05-07 at 20:01 +0200, Eric Dumazet wrote:
> On Mon, 2012-05-07 at 10:52 -0700, Dave Taht wrote:
>
> > so perhaps ecn and noecn? And ecn be the default (after some serious testing?)
>
> This is what I coded.
>
> Oh I forgot to send iproute2 patch it seems
>
include/linux/pkt_sched.h | 27 ++++++
tc/Makefile | 1
tc/q_codel.c | 160 ++++++++++++++++++++++++++++++++++++
tc/q_sfq.c | 14 +--
4 files changed, 196 insertions(+), 6 deletions(-)
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 410b33d..3c2a239 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -654,4 +654,31 @@ struct tc_qfq_stats {
__u32 lmax;
};
+/* CODEL */
+
+enum {
+ TCA_CODEL_UNSPEC,
+ TCA_CODEL_TARGET,
+ TCA_CODEL_LIMIT,
+ TCA_CODEL_MINBYTES,
+ TCA_CODEL_INTERVAL,
+ TCA_CODEL_ECN,
+ __TCA_CODEL_MAX
+};
+
+#define TCA_CODEL_MAX (__TCA_CODEL_MAX - 1)
+
+struct tc_codel_xstats {
+ __u32 count;
+ __u32 delay; /* time elapsed since next packet was queued (in us) */
+ __u32 drop_next;
+ __u32 drop_overlimit;
+ __u32 ecn_mark;
+ __u32 dropping;
+ __u32 state1;
+ __u32 state2;
+ __u32 state3;
+ __u32 states;
+};
+
#endif
diff --git a/tc/Makefile b/tc/Makefile
index be8cd5a..8a7cc8d 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -47,6 +47,7 @@ TCMODULES += em_cmp.o
TCMODULES += em_u32.o
TCMODULES += em_meta.o
TCMODULES += q_mqprio.o
+TCMODULES += q_codel.o
TCSO :=
ifeq ($(TC_CONFIG_ATM),y)
diff --git a/tc/q_codel.c b/tc/q_codel.c
new file mode 100644
index 0000000..0175e18
--- /dev/null
+++ b/tc/q_codel.c
@@ -0,0 +1,160 @@
+/*
+ * q_codel.c Codel.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Authors: Eric Dumazet <edumazet@google.com>
+ * Dave Taht <dave.taht@bufferbloat.net>
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <syslog.h>
+#include <fcntl.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <string.h>
+
+#include "utils.h"
+#include "tc_util.h"
+
+static void explain(void)
+{
+ fprintf(stderr, "Usage: ... codel [ limit PACKETS ] [ target TIME]\n");
+ fprintf(stderr, " [ interval TIME ] [ minbytes BYTES ]\n");
+}
+
+static int codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
+ struct nlmsghdr *n)
+{
+ unsigned limit = 0;
+ unsigned target = 0;
+ unsigned interval = 0;
+ unsigned minbytes = 0;
+ struct rtattr *tail;
+
+ while (argc > 0) {
+ if (strcmp(*argv, "limit") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&limit, *argv, 0)) {
+ fprintf(stderr, "Illegal \"limit\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "minbytes") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&minbytes, *argv, 0)) {
+ fprintf(stderr, "Illegal \"minbytes\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "target") == 0) {
+ NEXT_ARG();
+ if (get_time(&target, *argv)) {
+ fprintf(stderr, "Illegal \"target\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "interval") == 0) {
+ NEXT_ARG();
+ if (get_time(&interval, *argv)) {
+ fprintf(stderr, "Illegal \"interval\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "help") == 0) {
+ explain();
+ return -1;
+ } else {
+ fprintf(stderr, "What is \"%s\"?\n", *argv);
+ explain();
+ return -1;
+ }
+ argc--; argv++;
+ }
+
+ tail = NLMSG_TAIL(n);
+ addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+ if (limit)
+ addattr_l(n, 1024, TCA_CODEL_LIMIT, &limit, sizeof(limit));
+ if (minbytes)
+ addattr_l(n, 1024, TCA_CODEL_MINBYTES, &minbytes, sizeof(minbytes));
+ if (interval)
+ addattr_l(n, 1024, TCA_CODEL_INTERVAL, &interval, sizeof(interval));
+ if (target)
+ addattr_l(n, 1024, TCA_CODEL_TARGET, &target, sizeof(target));
+ tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+ return 0;
+}
+
+static int codel_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
+{
+ struct rtattr *tb[TCA_CODEL_MAX + 1];
+ unsigned limit;
+ unsigned interval;
+ unsigned target;
+ unsigned minbytes;
+ SPRINT_BUF(b1);
+
+ if (opt == NULL)
+ return 0;
+
+ parse_rtattr_nested(tb, TCA_CODEL_MAX, opt);
+
+ if (tb[TCA_CODEL_LIMIT] &&
+ RTA_PAYLOAD(tb[TCA_CODEL_LIMIT]) >= sizeof(__u32)) {
+ limit = rta_getattr_u32(tb[TCA_CODEL_LIMIT]);
+ fprintf(f, "limit %up ", limit);
+ }
+ if (tb[TCA_CODEL_MINBYTES] &&
+ RTA_PAYLOAD(tb[TCA_CODEL_MINBYTES]) >= sizeof(__u32)) {
+ minbytes = rta_getattr_u32(tb[TCA_CODEL_MINBYTES]);
+ fprintf(f, "minbytes %u ", minbytes);
+ }
+ if (tb[TCA_CODEL_TARGET] &&
+ RTA_PAYLOAD(tb[TCA_CODEL_TARGET]) >= sizeof(__u32)) {
+ target = rta_getattr_u32(tb[TCA_CODEL_TARGET]);
+ fprintf(f, "target %s ", sprint_time(target, b1));
+ }
+ if (tb[TCA_CODEL_INTERVAL] &&
+ RTA_PAYLOAD(tb[TCA_CODEL_INTERVAL]) >= sizeof(__u32)) {
+ interval = rta_getattr_u32(tb[TCA_CODEL_INTERVAL]);
+ fprintf(f, "interval %s ", sprint_time(interval, b1));
+ }
+
+ return 0;
+}
+
+static int codel_print_xstats(struct qdisc_util *qu, FILE *f,
+ struct rtattr *xstats)
+{
+ struct tc_codel_xstats *st;
+ SPRINT_BUF(b1);
+
+ if (xstats == NULL)
+ return 0;
+
+ if (RTA_PAYLOAD(xstats) < sizeof(*st))
+ return -1;
+
+ st = RTA_DATA(xstats);
+ fprintf(f, " count %u delay %s",
+ st->count, sprint_time(st->delay, b1));
+ if (st->dropping)
+ fprintf(f, " dropping");
+ if (st->drop_next)
+ fprintf(f, " drop_next %s", sprint_time(st->drop_next, b1));
+ fprintf(f, "\n drop_overlimit %u", st->drop_overlimit);
+ fprintf(f, " states %u : %u %u %u",
+ st->states, st->state1, st->state2, st->state3);
+ return 0;
+
+}
+
+struct qdisc_util codel_qdisc_util = {
+ .id = "codel",
+ .parse_qopt = codel_parse_opt,
+ .print_qopt = codel_print_opt,
+ .print_xstats = codel_print_xstats,
+};
diff --git a/tc/q_sfq.c b/tc/q_sfq.c
index 96f63ff..e1a57d4 100644
--- a/tc/q_sfq.c
+++ b/tc/q_sfq.c
@@ -231,12 +231,14 @@ static int sfq_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
fprintf(f, "divisor %u ", qopt->divisor);
if (qopt->perturb_period)
fprintf(f, "perturb %dsec ", qopt->perturb_period);
- if (qopt_ext && qopt_ext->qth_min) {
- fprintf(f, "\n ewma %u ", qopt_ext->Wlog);
- fprintf(f, "min %s max %s probability %g ",
- sprint_size(qopt_ext->qth_min, b2),
- sprint_size(qopt_ext->qth_max, b3),
- qopt_ext->max_P / pow(2, 32));
+ if (qopt_ext) {
+ if (qopt_ext->qth_min) {
+ fprintf(f, "\n ewma %u ", qopt_ext->Wlog);
+ fprintf(f, "min %s max %s probability %g ",
+ sprint_size(qopt_ext->qth_min, b2),
+ sprint_size(qopt_ext->qth_max, b3),
+ qopt_ext->max_P / pow(2, 32));
+ }
if (qopt_ext->flags & TC_RED_ECN)
fprintf(f, "ecn ");
if (show_stats) {
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 18:09 ` Eric Dumazet
@ 2012-05-07 18:15 ` Eric Dumazet
0 siblings, 0 replies; 32+ messages in thread
From: Eric Dumazet @ 2012-05-07 18:15 UTC (permalink / raw)
To: Dave Taht; +Cc: Eric Dumazet, codel, Dave Taht
On Mon, 2012-05-07 at 20:09 +0200, Eric Dumazet wrote:
> On Mon, 2012-05-07 at 20:01 +0200, Eric Dumazet wrote:
> > On Mon, 2012-05-07 at 10:52 -0700, Dave Taht wrote:
> >
> > > so perhaps ecn and noecn? And ecn be the default (after some serious testing?)
> >
> > This is what I coded.
> >
> > Oh I forgot to send iproute2 patch it seems
> >
>
> include/linux/pkt_sched.h | 27 ++++++
> tc/Makefile | 1
> tc/q_codel.c | 160 ++++++++++++++++++++++++++++++++++++
> tc/q_sfq.c | 14 +--
> 4 files changed, 196 insertions(+), 6 deletions(-)
Sorry forgot one updated file... Here is good one
include/linux/pkt_sched.h | 27 +++++
tc/Makefile | 1
tc/q_codel.c | 176 ++++++++++++++++++++++++++++++++++++
tc/q_sfq.c | 14 +-
4 files changed, 212 insertions(+), 6 deletions(-)
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 410b33d..3c2a239 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -654,4 +654,31 @@ struct tc_qfq_stats {
__u32 lmax;
};
+/* CODEL */
+
+enum {
+ TCA_CODEL_UNSPEC,
+ TCA_CODEL_TARGET,
+ TCA_CODEL_LIMIT,
+ TCA_CODEL_MINBYTES,
+ TCA_CODEL_INTERVAL,
+ TCA_CODEL_ECN,
+ __TCA_CODEL_MAX
+};
+
+#define TCA_CODEL_MAX (__TCA_CODEL_MAX - 1)
+
+struct tc_codel_xstats {
+ __u32 count;
+ __u32 delay; /* time elapsed since next packet was queued (in us) */
+ __u32 drop_next;
+ __u32 drop_overlimit;
+ __u32 ecn_mark;
+ __u32 dropping;
+ __u32 state1;
+ __u32 state2;
+ __u32 state3;
+ __u32 states;
+};
+
#endif
diff --git a/tc/Makefile b/tc/Makefile
index be8cd5a..8a7cc8d 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -47,6 +47,7 @@ TCMODULES += em_cmp.o
TCMODULES += em_u32.o
TCMODULES += em_meta.o
TCMODULES += q_mqprio.o
+TCMODULES += q_codel.o
TCSO :=
ifeq ($(TC_CONFIG_ATM),y)
diff --git a/tc/q_codel.c b/tc/q_codel.c
new file mode 100644
index 0000000..8547d19
--- /dev/null
+++ b/tc/q_codel.c
@@ -0,0 +1,176 @@
+/*
+ * q_codel.c Codel.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Authors: Eric Dumazet <edumazet@google.com>
+ * Dave Taht <dave.taht@bufferbloat.net>
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <syslog.h>
+#include <fcntl.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <string.h>
+
+#include "utils.h"
+#include "tc_util.h"
+
+static void explain(void)
+{
+ fprintf(stderr, "Usage: ... codel [ limit PACKETS ] [ target TIME]\n");
+ fprintf(stderr, " [ interval TIME ] [ minbytes BYTES ]\n");
+}
+
+static int codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
+ struct nlmsghdr *n)
+{
+ unsigned limit = 0;
+ unsigned target = 0;
+ unsigned interval = 0;
+ unsigned minbytes = 0;
+ int ecn = -1;
+ struct rtattr *tail;
+
+ while (argc > 0) {
+ if (strcmp(*argv, "limit") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&limit, *argv, 0)) {
+ fprintf(stderr, "Illegal \"limit\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "minbytes") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&minbytes, *argv, 0)) {
+ fprintf(stderr, "Illegal \"minbytes\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "target") == 0) {
+ NEXT_ARG();
+ if (get_time(&target, *argv)) {
+ fprintf(stderr, "Illegal \"target\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "interval") == 0) {
+ NEXT_ARG();
+ if (get_time(&interval, *argv)) {
+ fprintf(stderr, "Illegal \"interval\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "ecn") == 0) {
+ ecn = 1;
+ } else if (strcmp(*argv, "noecn") == 0) {
+ ecn = 0;
+ } else if (strcmp(*argv, "help") == 0) {
+ explain();
+ return -1;
+ } else {
+ fprintf(stderr, "What is \"%s\"?\n", *argv);
+ explain();
+ return -1;
+ }
+ argc--; argv++;
+ }
+
+ tail = NLMSG_TAIL(n);
+ addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+ if (limit)
+ addattr_l(n, 1024, TCA_CODEL_LIMIT, &limit, sizeof(limit));
+ if (minbytes)
+ addattr_l(n, 1024, TCA_CODEL_MINBYTES, &minbytes, sizeof(minbytes));
+ if (interval)
+ addattr_l(n, 1024, TCA_CODEL_INTERVAL, &interval, sizeof(interval));
+ if (target)
+ addattr_l(n, 1024, TCA_CODEL_TARGET, &target, sizeof(target));
+ if (ecn != -1)
+ addattr_l(n, 1024, TCA_CODEL_ECN, &ecn, sizeof(ecn));
+ tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+ return 0;
+}
+
+static int codel_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
+{
+ struct rtattr *tb[TCA_CODEL_MAX + 1];
+ unsigned limit;
+ unsigned interval;
+ unsigned target;
+ unsigned minbytes;
+ unsigned ecn;
+ SPRINT_BUF(b1);
+
+ if (opt == NULL)
+ return 0;
+
+ parse_rtattr_nested(tb, TCA_CODEL_MAX, opt);
+
+ if (tb[TCA_CODEL_ECN] &&
+ RTA_PAYLOAD(tb[TCA_CODEL_ECN]) >= sizeof(__u32)) {
+ ecn = rta_getattr_u32(tb[TCA_CODEL_ECN]);
+ if (ecn)
+ fprintf(f, "ecn ");
+ }
+ if (tb[TCA_CODEL_LIMIT] &&
+ RTA_PAYLOAD(tb[TCA_CODEL_LIMIT]) >= sizeof(__u32)) {
+ limit = rta_getattr_u32(tb[TCA_CODEL_LIMIT]);
+ fprintf(f, "limit %up ", limit);
+ }
+ if (tb[TCA_CODEL_MINBYTES] &&
+ RTA_PAYLOAD(tb[TCA_CODEL_MINBYTES]) >= sizeof(__u32)) {
+ minbytes = rta_getattr_u32(tb[TCA_CODEL_MINBYTES]);
+ fprintf(f, "minbytes %u ", minbytes);
+ }
+ if (tb[TCA_CODEL_TARGET] &&
+ RTA_PAYLOAD(tb[TCA_CODEL_TARGET]) >= sizeof(__u32)) {
+ target = rta_getattr_u32(tb[TCA_CODEL_TARGET]);
+ fprintf(f, "target %s ", sprint_time(target, b1));
+ }
+ if (tb[TCA_CODEL_INTERVAL] &&
+ RTA_PAYLOAD(tb[TCA_CODEL_INTERVAL]) >= sizeof(__u32)) {
+ interval = rta_getattr_u32(tb[TCA_CODEL_INTERVAL]);
+ fprintf(f, "interval %s ", sprint_time(interval, b1));
+ }
+
+ return 0;
+}
+
+static int codel_print_xstats(struct qdisc_util *qu, FILE *f,
+ struct rtattr *xstats)
+{
+ struct tc_codel_xstats *st;
+ SPRINT_BUF(b1);
+
+ if (xstats == NULL)
+ return 0;
+
+ if (RTA_PAYLOAD(xstats) < sizeof(*st))
+ return -1;
+
+ st = RTA_DATA(xstats);
+ fprintf(f, " count %u delay %s",
+ st->count, sprint_time(st->delay, b1));
+ if (st->dropping)
+ fprintf(f, " dropping");
+ if (st->drop_next)
+ fprintf(f, " drop_next %s", sprint_time(st->drop_next, b1));
+ if (st->ecn_mark)
+ fprintf(f, " ecn_mark %u", st->ecn_mark);
+ fprintf(f, "\n drop_overlimit %u", st->drop_overlimit);
+ fprintf(f, " states %u : %u %u %u",
+ st->states, st->state1, st->state2, st->state3);
+ return 0;
+
+}
+
+struct qdisc_util codel_qdisc_util = {
+ .id = "codel",
+ .parse_qopt = codel_parse_opt,
+ .print_qopt = codel_print_opt,
+ .print_xstats = codel_print_xstats,
+};
diff --git a/tc/q_sfq.c b/tc/q_sfq.c
index 96f63ff..e1a57d4 100644
--- a/tc/q_sfq.c
+++ b/tc/q_sfq.c
@@ -231,12 +231,14 @@ static int sfq_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
fprintf(f, "divisor %u ", qopt->divisor);
if (qopt->perturb_period)
fprintf(f, "perturb %dsec ", qopt->perturb_period);
- if (qopt_ext && qopt_ext->qth_min) {
- fprintf(f, "\n ewma %u ", qopt_ext->Wlog);
- fprintf(f, "min %s max %s probability %g ",
- sprint_size(qopt_ext->qth_min, b2),
- sprint_size(qopt_ext->qth_max, b3),
- qopt_ext->max_P / pow(2, 32));
+ if (qopt_ext) {
+ if (qopt_ext->qth_min) {
+ fprintf(f, "\n ewma %u ", qopt_ext->Wlog);
+ fprintf(f, "min %s max %s probability %g ",
+ sprint_size(qopt_ext->qth_min, b2),
+ sprint_size(qopt_ext->qth_max, b3),
+ qopt_ext->max_P / pow(2, 32));
+ }
if (qopt_ext->flags & TC_RED_ECN)
fprintf(f, "ecn ");
if (show_stats) {
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 17:52 ` Dave Taht
2012-05-07 18:01 ` Eric Dumazet
@ 2012-05-07 18:32 ` Jim Gettys
2012-05-07 18:44 ` Dave Taht
1 sibling, 1 reply; 32+ messages in thread
From: Jim Gettys @ 2012-05-07 18:32 UTC (permalink / raw)
To: Dave Taht; +Cc: Eric Dumazet, codel, Dave Taht
On 05/07/2012 01:52 PM, Dave Taht wrote:
> ecn:
>
> ecn was not in the paper (and it's working well) and I'd like to live
> in a world where ecn was the default. I think. Others disagree.
>
> so perhaps ecn and noecn? And ecn be the default (after some serious testing?)
>
I think noecn *must* be the default at the moment. There is still too
much brokenness out there (though it is improving); Steve Bauer can
enlighten us how much.
- Jim
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 18:32 ` Jim Gettys
@ 2012-05-07 18:44 ` Dave Taht
2012-05-07 18:50 ` Jim Gettys
0 siblings, 1 reply; 32+ messages in thread
From: Dave Taht @ 2012-05-07 18:44 UTC (permalink / raw)
To: Jim Gettys; +Cc: Eric Dumazet, codel, Dave Taht
On Mon, May 7, 2012 at 11:32 AM, Jim Gettys <jg@freedesktop.org> wrote:
> On 05/07/2012 01:52 PM, Dave Taht wrote:
>> ecn:
>>
>> ecn was not in the paper (and it's working well) and I'd like to live
>> in a world where ecn was the default. I think. Others disagree.
>>
>> so perhaps ecn and noecn? And ecn be the default (after some serious testing?)
>>
> I think noecn *must* be the default at the moment. There is still too
> much brokenness out there (though it is improving); Steve Bauer can
> enlighten us how much.
Possibly influenced by how hard it is to cycle builds on embedded
hardware and test one thing at a time,
as well as the space program's major success at shortening the path to
the moon, I am an advocate of all-up-testing.
http://www.nasa.gov/offices/oce/appel/ask-academy/issues/ask-oce/AO_1-7_F_snapshot.html
Going with ecn enabled by default, and being aware that it might cause
problems, will
result in less time being spent testing each individually.
I note that on an ingress qdisc ecn has mildly less risk as it only
tests the router and host device.
On egress, yes, you're testing the whole internet.
> - Jim
>
--
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
http://www.bufferbloat.net
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 18:44 ` Dave Taht
@ 2012-05-07 18:50 ` Jim Gettys
2012-05-07 19:16 ` Eric Dumazet
0 siblings, 1 reply; 32+ messages in thread
From: Jim Gettys @ 2012-05-07 18:50 UTC (permalink / raw)
To: Dave Taht; +Cc: Eric Dumazet, codel, Dave Taht
On 05/07/2012 02:44 PM, Dave Taht wrote:
> On Mon, May 7, 2012 at 11:32 AM, Jim Gettys <jg@freedesktop.org> wrote:
>> On 05/07/2012 01:52 PM, Dave Taht wrote:
>>> ecn:
>>>
>>> ecn was not in the paper (and it's working well) and I'd like to live
>>> in a world where ecn was the default. I think. Others disagree.
>>>
>>> so perhaps ecn and noecn? And ecn be the default (after some serious testing?)
>>>
>> I think noecn *must* be the default at the moment. There is still too
>> much brokenness out there (though it is improving); Steve Bauer can
>> enlighten us how much.
> Possibly influenced by how hard it is to cycle builds on embedded
> hardware and test one thing at a time,
> as well as the space program's major success at shortening the path to
> the moon, I am an advocate of all-up-testing.
>
> http://www.nasa.gov/offices/oce/appel/ask-academy/issues/ask-oce/AO_1-7_F_snapshot.html
>
> Going with ecn enabled by default, and being aware that it might cause
> problems, will
> result in less time being spent testing each individually.
>
> I note that on an ingress qdisc ecn has mildly less risk as it only
> tests the router and host device.
>
> On egress, yes, you're testing the whole internet.
You want to turn it on in CeroWrt: sure. Expert audience, who may be
able to notice when things go sour, fine....
Neither you, nor I are expert at the current state of ECN in the global
Internet: but Steve Bauer is. We can have that conversation with him
shortly.
So for now "First, do no harm" should be our mantra....
So the upstream patch default state should be off. We don't want to mix
"codel doesn't work" with "ecn is busted someplace", as it gets its trials.
- Jim
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 18:50 ` Jim Gettys
@ 2012-05-07 19:16 ` Eric Dumazet
2012-05-07 19:36 ` Jim Gettys
0 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2012-05-07 19:16 UTC (permalink / raw)
To: Jim Gettys; +Cc: Eric Dumazet, codel, Dave Taht
On Mon, 2012-05-07 at 14:50 -0400, Jim Gettys wrote:
> You want to turn it on in CeroWrt: sure. Expert audience, who may be
> able to notice when things go sour, fine....
>
> Neither you, nor I are expert at the current state of ECN in the global
> Internet: but Steve Bauer is. We can have that conversation with him
> shortly.
>
> So for now "First, do no harm" should be our mantra....
>
> So the upstream patch default state should be off. We don't want to mix
> "codel doesn't work" with "ecn is busted someplace", as it gets its trials.
>
ecn is negociated by peers, not a qdisc.
ecn at qdisc level is only marking an already enabled ECN packet.
Since most packets are not ECN enabled, they are dropped instead.
If you read my ECN addition to Codel, you can see its disabled by
default already. Its a toy for early adopters.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 19:16 ` Eric Dumazet
@ 2012-05-07 19:36 ` Jim Gettys
2012-05-07 20:03 ` Eric Dumazet
0 siblings, 1 reply; 32+ messages in thread
From: Jim Gettys @ 2012-05-07 19:36 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, codel, Dave Taht
On 05/07/2012 03:16 PM, Eric Dumazet wrote:
> On Mon, 2012-05-07 at 14:50 -0400, Jim Gettys wrote:
>
>> You want to turn it on in CeroWrt: sure. Expert audience, who may be
>> able to notice when things go sour, fine....
>>
>> Neither you, nor I are expert at the current state of ECN in the global
>> Internet: but Steve Bauer is. We can have that conversation with him
>> shortly.
>>
>> So for now "First, do no harm" should be our mantra....
>>
>> So the upstream patch default state should be off. We don't want to mix
>> "codel doesn't work" with "ecn is busted someplace", as it gets its trials.
>>
> ecn is negociated by peers, not a qdisc.
>
> ecn at qdisc level is only marking an already enabled ECN packet.
>
> Since most packets are not ECN enabled, they are dropped instead.
That's certainly the way ECN is supposed to work right now.
>
> If you read my ECN addition to Codel, you can see its disabled by
> default already. Its a toy for early adopters.
I think it is safe for it to behave the rest of the way Linux ECN
support does right now: it only gets used if the peer requests it.
Not clear to me there needs to be/should be any option at all: the last
conversation I had with Steve Bauer was that something north of 20% of
conversations were ECN capable. Is there one for the other instances of
ECN support in Linux? If so, it should be keyed by the same variable,
and not be a one-off for codel.
If you wanted to test ECN separately from drop with codel, then you'd
just request ECN in the conversation (by default, OS's don't normally
request ECN today, as the remaining brokenness gets sorted out).
- Jim
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 19:36 ` Jim Gettys
@ 2012-05-07 20:03 ` Eric Dumazet
2012-05-07 20:55 ` dave taht
2012-05-08 1:15 ` Jim Gettys
0 siblings, 2 replies; 32+ messages in thread
From: Eric Dumazet @ 2012-05-07 20:03 UTC (permalink / raw)
To: Jim Gettys; +Cc: Eric Dumazet, codel, Dave Taht
On Mon, 2012-05-07 at 15:36 -0400, Jim Gettys wrote:
> I think it is safe for it to behave the rest of the way Linux ECN
> support does right now: it only gets used if the peer requests it.
>
> Not clear to me there needs to be/should be any option at all: the last
> conversation I had with Steve Bauer was that something north of 20% of
> conversations were ECN capable. Is there one for the other instances of
> ECN support in Linux? If so, it should be keyed by the same variable,
> and not be a one-off for codel.
>
SFB, one of the latest qdisc added in linux has ECN support enabled.
There is no option to disable it, because I felt it was safe. Maybe I
was a fool, but problem is I am not sure SFB is even used.
> If you wanted to test ECN separately from drop with codel, then you'd
> just request ECN in the conversation (by default, OS's don't normally
> request ECN today, as the remaining brokenness gets sorted out).
Since ECN is not mentioned in Codel paper, this means no simulation was
done to study the possible effects.
So its probably better to leave ECN as an option. We can change the
default later.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 20:03 ` Eric Dumazet
@ 2012-05-07 20:55 ` dave taht
2012-05-08 1:15 ` Jim Gettys
1 sibling, 0 replies; 32+ messages in thread
From: dave taht @ 2012-05-07 20:55 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, codel, Dave Taht
On 05/07/2012 01:03 PM, Eric Dumazet wrote:
> On Mon, 2012-05-07 at 15:36 -0400, Jim Gettys wrote:
>
>> I think it is safe for it to behave the rest of the way Linux ECN
>> support does right now: it only gets used if the peer requests it.
>>
>> Not clear to me there needs to be/should be any option at all: the last
>> conversation I had with Steve Bauer was that something north of 20% of
>> conversations were ECN capable. Is there one for the other instances of
>> ECN support in Linux? If so, it should be keyed by the same variable,
>> and not be a one-off for codel.
>>
> SFB, one of the latest qdisc added in linux has ECN support enabled.
>
> There is no option to disable it, because I felt it was safe. Maybe I
> was a fool, but problem is I am not sure SFB is even used.
My own testing with SFB as done was simultaneously with
the "GSO by default" change to linux in Febuary of last year.
The results were dismal.
But: At the time I was unaware of the GSO change.
I only stumbled across that commit months later, and I made
sure all future testing disabled all offloading of any kind via
the debloat script.
At that point I'd thrown out all the data and started looking at
other alternatives. I no longer feel Blue works worth a darn
in the first place, anyway. The bloom filter is neat, and
I like the idea of punishing unresponsive flows (particularly
packets marked ecn but not responding to ecn indicators),
but it just plain doesn't work.
My low opinion of TSO/GSO/UFO GRO etc is well known,
although I'd not mind them at all so long as
a working AQM was in place on the card itself.
I am glad to see that these offloads have improved a lot
over the past year, however.
>> If you wanted to test ECN separately from drop with codel, then you'd
>> just request ECN in the conversation (by default, OS's don't normally
>> request ECN today, as the remaining brokenness gets sorted out).
> Since ECN is not mentioned in Codel paper, this means no simulation was
> done to study the possible effects.
Yes, I'd like simulations with long RTTs.
> So its probably better to leave ECN as an option. We can change the
> default later.
The incremental approach has been tried re this option for
a decade. I'd prefer "on by default", and then find more ways
to make ECN work, to automagically detect and workaround
problems, but I realize I'm in a minority here, and am not going to
push the point for mainline. CeroWrt, yes. On by default,
and pursue this goal.
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-07 20:03 ` Eric Dumazet
2012-05-07 20:55 ` dave taht
@ 2012-05-08 1:15 ` Jim Gettys
2012-05-08 1:20 ` Andrew McGregor
1 sibling, 1 reply; 32+ messages in thread
From: Jim Gettys @ 2012-05-08 1:15 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, codel, Dave Taht
On 05/07/2012 04:03 PM, Eric Dumazet wrote:
> On Mon, 2012-05-07 at 15:36 -0400, Jim Gettys wrote:
>
>> I think it is safe for it to behave the rest of the way Linux ECN
>> support does right now: it only gets used if the peer requests it.
>>
>> Not clear to me there needs to be/should be any option at all: the last
>> conversation I had with Steve Bauer was that something north of 20% of
>> conversations were ECN capable. Is there one for the other instances of
>> ECN support in Linux? If so, it should be keyed by the same variable,
>> and not be a one-off for codel.
>>
> SFB, one of the latest qdisc added in linux has ECN support enabled.
>
> There is no option to disable it, because I felt it was safe. Maybe I
> was a fool, but problem is I am not sure SFB is even used.
Client initiated ECN may still be problematic. It is *fine* if a system
responds to ECN that is sent to it. We know that is fine, as Linux has
had the server side of ECN enabled for quite a few years, and those
systems now are > 20% of servers on the internet.
The issue is there are both networks and middleboxes that are
misconfigured/broken. In some cases, it black-holes a connection and
your data just doesn't go through; in others, ECN is ignored,; in others
the ECN bits are cleared or mangled; and, most worryingly, there are
some devices of order a decade old that just crash if they see an ECN
marked packet. We do not know how common the last of these problems are.
Anecdotal evidence: OpenWrt tried turning on the client side of ECN
several years ago, and had to back off due to too many bug reports.
CeroWrt is small enough (and time has passed), so I am all for Dave
turning on ECN for CeroWrt and we can get a feeling for how common the
problem still is.
Steve Bauer has been studying ECN for the last couple years (and, at
times, getting various networks fixed, beginning with MIT's own network,
and later, some of the biggest commercial and academic networks fixed.
There are several papers on what Steve (et. al.) have found, but I don't
have the references handy.
But it's crashing the end user boxes that has been most problematic, and
we don't know how common that is. There *used* to be a database of such
broken hardware, but it has bit-rotted.
So let's check with Steve Bauer on this years results.
>
>> If you wanted to test ECN separately from drop with codel, then you'd
>> just request ECN in the conversation (by default, OS's don't normally
>> request ECN today, as the remaining brokenness gets sorted out).
> Since ECN is not mentioned in Codel paper, this means no simulation was
> done to study the possible effects.
>
> So its probably better to leave ECN as an option. We can change the
> default later.
Yup. Once we know if the current state of the net is good enough.
What we don't want is to get a pile of codel bug reports that are really
ECN related problems, of which we know there are quite a few.
- Jim
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-08 1:15 ` Jim Gettys
@ 2012-05-08 1:20 ` Andrew McGregor
2012-05-08 1:45 ` Jim Gettys
0 siblings, 1 reply; 32+ messages in thread
From: Andrew McGregor @ 2012-05-08 1:20 UTC (permalink / raw)
To: Jim Gettys; +Cc: Eric Dumazet, codel, Dave Taht
[-- Attachment #1: Type: text/plain, Size: 1631 bytes --]
On 8/05/2012, at 1:15 PM, Jim Gettys wrote:
> On 05/07/2012 04:03 PM, Eric Dumazet wrote:
>> On Mon, 2012-05-07 at 15:36 -0400, Jim Gettys wrote:
>>
>>> I think it is safe for it to behave the rest of the way Linux ECN
>>> support does right now: it only gets used if the peer requests it.
>>>
>>> Not clear to me there needs to be/should be any option at all: the last
>>> conversation I had with Steve Bauer was that something north of 20% of
>>> conversations were ECN capable. Is there one for the other instances of
>>> ECN support in Linux? If so, it should be keyed by the same variable,
>>> and not be a one-off for codel.
>>>
>> SFB, one of the latest qdisc added in linux has ECN support enabled.
>>
>> There is no option to disable it, because I felt it was safe. Maybe I
>> was a fool, but problem is I am not sure SFB is even used.
>
> Client initiated ECN may still be problematic. It is *fine* if a system
> responds to ECN that is sent to it. We know that is fine, as Linux has
> had the server side of ECN enabled for quite a few years, and those
> systems now are > 20% of servers on the internet.
>
>
> What we don't want is to get a pile of codel bug reports that are really
> ECN related problems, of which we know there are quite a few.
> - Jim
>
Sure... but making the Linux codel ECN capable, and even on by default with no configuration, will have no effect if the clients don't use ECN. So there's no need to even make it optional, let alone default to off.
That way, we don't get bug reports of 'codel doesn't do ECN'.
Andrew
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2330 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-08 1:20 ` Andrew McGregor
@ 2012-05-08 1:45 ` Jim Gettys
2012-05-08 2:16 ` Rick Jones
0 siblings, 1 reply; 32+ messages in thread
From: Jim Gettys @ 2012-05-08 1:45 UTC (permalink / raw)
To: Andrew McGregor; +Cc: Eric Dumazet, codel, Dave Taht
On 05/07/2012 09:20 PM, Andrew McGregor wrote:
> On 8/05/2012, at 1:15 PM, Jim Gettys wrote:
>
>> On 05/07/2012 04:03 PM, Eric Dumazet wrote:
>>> On Mon, 2012-05-07 at 15:36 -0400, Jim Gettys wrote:
>>>
>>>> I think it is safe for it to behave the rest of the way Linux ECN
>>>> support does right now: it only gets used if the peer requests it.
>>>>
>>>> Not clear to me there needs to be/should be any option at all: the last
>>>> conversation I had with Steve Bauer was that something north of 20% of
>>>> conversations were ECN capable. Is there one for the other instances of
>>>> ECN support in Linux? If so, it should be keyed by the same variable,
>>>> and not be a one-off for codel.
>>>>
>>> SFB, one of the latest qdisc added in linux has ECN support enabled.
>>>
>>> There is no option to disable it, because I felt it was safe. Maybe I
>>> was a fool, but problem is I am not sure SFB is even used.
>> Client initiated ECN may still be problematic. It is *fine* if a system
>> responds to ECN that is sent to it. We know that is fine, as Linux has
>> had the server side of ECN enabled for quite a few years, and those
>> systems now are > 20% of servers on the internet.
>>
>>
>> What we don't want is to get a pile of codel bug reports that are really
>> ECN related problems, of which we know there are quite a few.
>> - Jim
>>
> Sure... but making the Linux codel ECN capable, and even on by default with no configuration, will have no effect if the clients don't use ECN. So there's no need to even make it optional, let alone default to off.
>
>
The problem is that the client may be managing it's outbound queue with
codel. So you have to negotiate ECN and if you start marking, you may
run into trouble.
- Jim
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-08 1:45 ` Jim Gettys
@ 2012-05-08 2:16 ` Rick Jones
2012-05-08 3:14 ` Jim Gettys
0 siblings, 1 reply; 32+ messages in thread
From: Rick Jones @ 2012-05-08 2:16 UTC (permalink / raw)
To: Jim Gettys; +Cc: Eric Dumazet, Dave Taht, codel
On 05/07/2012 06:45 PM, Jim Gettys wrote:
> On 05/07/2012 09:20 PM, Andrew McGregor wrote:
>> Sure... but making the Linux codel ECN capable, and even on by
>> default with no configuration, will have no effect if the clients
>> don't use ECN. So there's no need to even make it optional, let
>> alone default to off.
>>
>>
> The problem is that the client may be managing it's outbound queue with
> codel. So you have to negotiate ECN and if you start marking, you may
> run into trouble.
Are the odds really all that high that when the admin of the client set
tcp_ecn = 1 (or its non-Linux equivalent) that codel running on his
system would be the only thing actually marking and so "the" reason for
trouble? I'm inclined to agree with Andrew. If the client is going to
have trouble with ECN, it will have it once it sets tcp_ecn = 1, codel
being there or not.
rick
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-08 2:16 ` Rick Jones
@ 2012-05-08 3:14 ` Jim Gettys
2012-05-08 3:32 ` Rick Jones
0 siblings, 1 reply; 32+ messages in thread
From: Jim Gettys @ 2012-05-08 3:14 UTC (permalink / raw)
To: Rick Jones; +Cc: Eric Dumazet, Dave Taht, codel
On 05/07/2012 10:16 PM, Rick Jones wrote:
> On 05/07/2012 06:45 PM, Jim Gettys wrote:
>> On 05/07/2012 09:20 PM, Andrew McGregor wrote:
>>> Sure... but making the Linux codel ECN capable, and even on by
>>> default with no configuration, will have no effect if the clients
>>> don't use ECN. So there's no need to even make it optional, let
>>> alone default to off.
>>>
>>>
>> The problem is that the client may be managing it's outbound queue with
>> codel. So you have to negotiate ECN and if you start marking, you may
>> run into trouble.
>
> Are the odds really all that high that when the admin of the client
> set tcp_ecn = 1 (or its non-Linux equivalent) that codel running on
> his system would be the only thing actually marking and so "the"
> reason for trouble? I'm inclined to agree with Andrew. If the client
> is going to have trouble with ECN, it will have it once it sets
> tcp_ecn = 1, codel being there or not.
There are three cases here:
1. you initiate a TCP session;
2. you receive a TCP session;
3. you mark a packet in transit through a system (you are routing).
Case 2 is known safe, and is deploying rapidly in the Internet. Linux
defaults to 2): if you talk to it asking for ECN, it responds.
The problem is that some networks screw up the ecn bits. And worse yet,
there at least used to be some hardware out in the great internet that
would go belly up if it saw marked packets.
So it's 1 and 3 that we might get in trouble about. There is some way
to turn on 1( in Linux already; using ECN is negotiated as part of the open.
But we better not do 3) (mark for ECN rather than drop) without knowing
if it "safe" to do so. Steve Bauer and his collaborators have more
understanding than anyone yet on this list.
- Jim
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-08 3:14 ` Jim Gettys
@ 2012-05-08 3:32 ` Rick Jones
2012-05-08 3:40 ` Kathleen Nichols
2012-05-08 4:14 ` Jim Gettys
0 siblings, 2 replies; 32+ messages in thread
From: Rick Jones @ 2012-05-08 3:32 UTC (permalink / raw)
To: Jim Gettys; +Cc: Eric Dumazet, Dave Taht, codel
On 05/07/2012 08:14 PM, Jim Gettys wrote:
> There are three cases here:
>
> 1. you initiate a TCP session;
> 2. you receive a TCP session;
> 3. you mark a packet in transit through a system (you are routing).
>
> Case 2 is known safe, and is deploying rapidly in the Internet. Linux
> defaults to 2): if you talk to it asking for ECN, it responds.
>
> The problem is that some networks screw up the ecn bits. And worse yet,
> there at least used to be some hardware out in the great internet that
> would go belly up if it saw marked packets.
>
> So it's 1 and 3 that we might get in trouble about. There is some way
> to turn on 1( in Linux already; using ECN is negotiated as part of the open.
>
> But we better not do 3) (mark for ECN rather than drop) without knowing
> if it "safe" to do so. Steve Bauer and his collaborators have more
> understanding than anyone yet on this list.
Then get the folks at queue.acm.org on the ball and get the paper
published already :) :) :)
But I'm still confused. If Case 2 is "safe" is it safe only because
no-one initiates ECN making Case 2 a noop? IE, it is not known to be
safe to set tcp_ecn=1? That being the case, I'd still think that codel
going "if I see ECT and would drop, I'll mark" is OK because it will
only see ECT if the client set tcp_ecn=1, and presumably then, the owner
of the client will not have set tcp_ecn=1 unless they know it is "safe"
to do so. Particularly with Eric's recent patch to upstream to only
accept an ECN request in a SYN if the ECN bits in the IP header are clear.
rick
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-08 3:32 ` Rick Jones
@ 2012-05-08 3:40 ` Kathleen Nichols
2012-05-08 4:14 ` Jim Gettys
1 sibling, 0 replies; 32+ messages in thread
From: Kathleen Nichols @ 2012-05-08 3:40 UTC (permalink / raw)
To: codel
On 5/7/12 8:32 PM, Rick Jones wrote:
...
> Then get the folks at queue.acm.org on the ball and get the paper
> published already :) :) :)
>
The folks at queue.acm.org have been wonderful. It is the authors who
have been clueless.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-08 3:32 ` Rick Jones
2012-05-08 3:40 ` Kathleen Nichols
@ 2012-05-08 4:14 ` Jim Gettys
2012-05-08 5:10 ` Eric Dumazet
1 sibling, 1 reply; 32+ messages in thread
From: Jim Gettys @ 2012-05-08 4:14 UTC (permalink / raw)
To: Rick Jones; +Cc: Eric Dumazet, Dave Taht, codel
On May 7, 2012, at 11:32 PM, Rick Jones <rick.jones2@hp.com> wrote:
> On 05/07/2012 08:14 PM, Jim Gettys wrote:
>> There are three cases here:
>>
>> 1. you initiate a TCP session;
>> 2. you receive a TCP session;
>> 3. you mark a packet in transit through a system (you are routing).
>>
>> Case 2 is known safe, and is deploying rapidly in the Internet. Linux
>> defaults to 2): if you talk to it asking for ECN, it responds.
>>
>> The problem is that some networks screw up the ecn bits. And worse yet,
>> there at least used to be some hardware out in the great internet that
>> would go belly up if it saw marked packets.
>>
>> So it's 1 and 3 that we might get in trouble about. There is some way
>> to turn on 1( in Linux already; using ECN is negotiated as part of the open.
>>
>> But we better not do 3) (mark for ECN rather than drop) without knowing
>> if it "safe" to do so. Steve Bauer and his collaborators have more
>> understanding than anyone yet on this list.
>
> Then get the folks at queue.acm.org on the ball and get the paper published already :) :) :)
>
> But I'm still confused. If Case 2 is "safe" is it safe only because no-one initiates ECN making Case 2 a noop? IE, it is not known to be safe to set tcp_ecn=1? That being the case, I'd still think that codel going "if I see ECT and would drop, I'll mark" is OK because it will only see ECT if the client set tcp_ecn=1, and presumably then, the owner of the client will not have set tcp_ecn=1 unless they know it is "safe" to do so. Particularly with Eric's recent patch to upstream to only accept an ECN request in a SYN if the ECN bits in the IP header are clear.
Too late tonight to try to resurrect what I remember from Steve. Barring unforeseen events, he should be able to join the list then and better coming from him anyway.
Jim
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-08 4:14 ` Jim Gettys
@ 2012-05-08 5:10 ` Eric Dumazet
2012-05-08 10:25 ` Jim Gettys
0 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2012-05-08 5:10 UTC (permalink / raw)
To: Jim Gettys; +Cc: Eric Dumazet, codel, Dave Taht
On Tue, 2012-05-08 at 00:14 -0400, Jim Gettys wrote:
> Too late tonight to try to resurrect what I remember from Steve.
> Barring unforeseen events, he should be able to join the list then and
> better coming from him anyway.
>
I never heard of Steve, yet we did a lot of ECN related work on linux
kernel to fix various issues (or allowing a safe ECN activation) these
past years.
commit bd14b1b2e29bd6812597f896dde06eaf7c6d2f24
tcp: be more strict before accepting ECN negociation
commit e4ae004b84b315dd4b762e474f97403eac70f76a
netem: add ECN capability
commit ddecf0f4db44ef94847a62d6ecf74456b4dcc66f
net_sched: sfq: add optional RED on top of SFQ
commit b5c5693bb723a019deac3cd1345f3e7233c8a67e
tcp: report ECN_SEEN in tcp_info
commit 7a269ffad72f3604b8982fa09c387670e0d2ee14
tcp: ECN blackhole should not force quickack mode
commit 5173cc057787560c127c6e9737f308c833dc4ff3
ipv4: more compliant RFC 3168 support
commit ca06707022d6ba4744198a8ebbe4994786b0c613
ipv6: restore correct ECN handling on TCP xmit
commit 4a2b9c3756077c05dd8666e458a751d2248b61b6
net_sched: fix ip_tos2prio
commit 6623e3b24a5ebb07e81648c478d286a1329ab891
ipv4: IP defragmentation must be ECN aware
commit 172d69e63c7f1e8300d0e1c1bbd8eb0f630faa15
syncookies: add support for ECN
Yes, some middlebox might be broken.
But far more users will be stuck in July when FBI stop DNSChanger
servers for good.
Far more users will have problems at IPv6 launch in June
And so on.
Of course we could try to focus on AQM problems for the time being ;)
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-08 5:10 ` Eric Dumazet
@ 2012-05-08 10:25 ` Jim Gettys
2012-05-08 13:57 ` Kathleen Nichols
0 siblings, 1 reply; 32+ messages in thread
From: Jim Gettys @ 2012-05-08 10:25 UTC (permalink / raw)
To: Eric Dumazet; +Cc: codel, Eric Dumazet, Jim Gettys, Dave Taht
On 05/08/2012 01:10 AM, Eric Dumazet wrote:
> On Tue, 2012-05-08 at 00:14 -0400, Jim Gettys wrote:
>
>> Too late tonight to try to resurrect what I remember from Steve.
>> Barring unforeseen events, he should be able to join the list then and
>> better coming from him anyway.
>>
> I never heard of Steve, yet we did a lot of ECN related work on linux
> kernel to fix various issues (or allowing a safe ECN activation) these
> past years.
Steve Bauer works for Dave Clark at MIT. He's been studying ECN
deployment in the global internet the last couple years, by probing of
order 1,000,000 sites (and the network paths to them).
- Jim
>
> commit bd14b1b2e29bd6812597f896dde06eaf7c6d2f24
> tcp: be more strict before accepting ECN negociation
>
> commit e4ae004b84b315dd4b762e474f97403eac70f76a
> netem: add ECN capability
>
> commit ddecf0f4db44ef94847a62d6ecf74456b4dcc66f
> net_sched: sfq: add optional RED on top of SFQ
>
> commit b5c5693bb723a019deac3cd1345f3e7233c8a67e
> tcp: report ECN_SEEN in tcp_info
>
> commit 7a269ffad72f3604b8982fa09c387670e0d2ee14
> tcp: ECN blackhole should not force quickack mode
>
> commit 5173cc057787560c127c6e9737f308c833dc4ff3
> ipv4: more compliant RFC 3168 support
>
> commit ca06707022d6ba4744198a8ebbe4994786b0c613
> ipv6: restore correct ECN handling on TCP xmit
>
> commit 4a2b9c3756077c05dd8666e458a751d2248b61b6
> net_sched: fix ip_tos2prio
>
> commit 6623e3b24a5ebb07e81648c478d286a1329ab891
> ipv4: IP defragmentation must be ECN aware
>
> commit 172d69e63c7f1e8300d0e1c1bbd8eb0f630faa15
> syncookies: add support for ECN
>
>
>
> Yes, some middlebox might be broken.
>
> But far more users will be stuck in July when FBI stop DNSChanger
> servers for good.
>
> Far more users will have problems at IPv6 launch in June
>
> And so on.
>
> Of course we could try to focus on AQM problems for the time being ;)
>
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-08 10:25 ` Jim Gettys
@ 2012-05-08 13:57 ` Kathleen Nichols
2012-05-08 14:04 ` Jim Gettys
0 siblings, 1 reply; 32+ messages in thread
From: Kathleen Nichols @ 2012-05-08 13:57 UTC (permalink / raw)
To: codel
I think there are two different kinds of "ECN work" going on: one to
work on the algorithm and the other to see how widely deployed
it is.
On 5/8/12 3:25 AM, Jim Gettys wrote:
> On 05/08/2012 01:10 AM, Eric Dumazet wrote:
>> On Tue, 2012-05-08 at 00:14 -0400, Jim Gettys wrote:
>>
>>> Too late tonight to try to resurrect what I remember from Steve.
>>> Barring unforeseen events, he should be able to join the list then and
>>> better coming from him anyway.
>>>
>> I never heard of Steve, yet we did a lot of ECN related work on linux
>> kernel to fix various issues (or allowing a safe ECN activation) these
>> past years.
> Steve Bauer works for Dave Clark at MIT. He's been studying ECN
> deployment in the global internet the last couple years, by probing of
> order 1,000,000 sites (and the network paths to them).
> - Jim
>
>>
>> commit bd14b1b2e29bd6812597f896dde06eaf7c6d2f24
>> tcp: be more strict before accepting ECN negociation
>>
>> commit e4ae004b84b315dd4b762e474f97403eac70f76a
>> netem: add ECN capability
>>
>> commit ddecf0f4db44ef94847a62d6ecf74456b4dcc66f
>> net_sched: sfq: add optional RED on top of SFQ
>>
>> commit b5c5693bb723a019deac3cd1345f3e7233c8a67e
>> tcp: report ECN_SEEN in tcp_info
>>
>> commit 7a269ffad72f3604b8982fa09c387670e0d2ee14
>> tcp: ECN blackhole should not force quickack mode
>>
>> commit 5173cc057787560c127c6e9737f308c833dc4ff3
>> ipv4: more compliant RFC 3168 support
>>
>> commit ca06707022d6ba4744198a8ebbe4994786b0c613
>> ipv6: restore correct ECN handling on TCP xmit
>>
>> commit 4a2b9c3756077c05dd8666e458a751d2248b61b6
>> net_sched: fix ip_tos2prio
>>
>> commit 6623e3b24a5ebb07e81648c478d286a1329ab891
>> ipv4: IP defragmentation must be ECN aware
>>
>> commit 172d69e63c7f1e8300d0e1c1bbd8eb0f630faa15
>> syncookies: add support for ECN
>>
>>
>>
>> Yes, some middlebox might be broken.
>>
>> But far more users will be stuck in July when FBI stop DNSChanger
>> servers for good.
>>
>> Far more users will have problems at IPv6 launch in June
>>
>> And so on.
>>
>> Of course we could try to focus on AQM problems for the time being ;)
>>
>>
>>
>
> _______________________________________________
> Codel mailing list
> Codel@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/codel
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Codel] [PATCH 1/2] codel: Controlled Delay AQM
2012-05-08 13:57 ` Kathleen Nichols
@ 2012-05-08 14:04 ` Jim Gettys
0 siblings, 0 replies; 32+ messages in thread
From: Jim Gettys @ 2012-05-08 14:04 UTC (permalink / raw)
To: Kathleen Nichols; +Cc: codel
On 05/08/2012 09:57 AM, Kathleen Nichols wrote:
> I think there are two different kinds of "ECN work" going on: one to
> work on the algorithm and the other to see how widely deployed
> it is.
Yup. Steve Bauer and others have been studying ECN deployment, along
with the problems they have encountered. Figuring out if we can safely
enable ECN is the interesting question right now (the server side
deployment is going quite well at this date.
- Jim
>
> On 5/8/12 3:25 AM, Jim Gettys wrote:
>> On 05/08/2012 01:10 AM, Eric Dumazet wrote:
>>> On Tue, 2012-05-08 at 00:14 -0400, Jim Gettys wrote:
>>>
>>>> Too late tonight to try to resurrect what I remember from Steve.
>>>> Barring unforeseen events, he should be able to join the list then and
>>>> better coming from him anyway.
>>>>
>>> I never heard of Steve, yet we did a lot of ECN related work on linux
>>> kernel to fix various issues (or allowing a safe ECN activation) these
>>> past years.
>> Steve Bauer works for Dave Clark at MIT. He's been studying ECN
>> deployment in the global internet the last couple years, by probing of
>> order 1,000,000 sites (and the network paths to them).
>> - Jim
>>
>>> commit bd14b1b2e29bd6812597f896dde06eaf7c6d2f24
>>> tcp: be more strict before accepting ECN negociation
>>>
>>> commit e4ae004b84b315dd4b762e474f97403eac70f76a
>>> netem: add ECN capability
>>>
>>> commit ddecf0f4db44ef94847a62d6ecf74456b4dcc66f
>>> net_sched: sfq: add optional RED on top of SFQ
>>>
>>> commit b5c5693bb723a019deac3cd1345f3e7233c8a67e
>>> tcp: report ECN_SEEN in tcp_info
>>>
>>> commit 7a269ffad72f3604b8982fa09c387670e0d2ee14
>>> tcp: ECN blackhole should not force quickack mode
>>>
>>> commit 5173cc057787560c127c6e9737f308c833dc4ff3
>>> ipv4: more compliant RFC 3168 support
>>>
>>> commit ca06707022d6ba4744198a8ebbe4994786b0c613
>>> ipv6: restore correct ECN handling on TCP xmit
>>>
>>> commit 4a2b9c3756077c05dd8666e458a751d2248b61b6
>>> net_sched: fix ip_tos2prio
>>>
>>> commit 6623e3b24a5ebb07e81648c478d286a1329ab891
>>> ipv4: IP defragmentation must be ECN aware
>>>
>>> commit 172d69e63c7f1e8300d0e1c1bbd8eb0f630faa15
>>> syncookies: add support for ECN
>>>
>>>
>>>
>>> Yes, some middlebox might be broken.
>>>
>>> But far more users will be stuck in July when FBI stop DNSChanger
>>> servers for good.
>>>
>>> Far more users will have problems at IPv6 launch in June
>>>
>>> And so on.
>>>
>>> Of course we could try to focus on AQM problems for the time being ;)
>>>
>>>
>>>
>> _______________________________________________
>> Codel mailing list
>> Codel@lists.bufferbloat.net
>> https://lists.bufferbloat.net/listinfo/codel
> _______________________________________________
> Codel mailing list
> Codel@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/codel
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2012-05-08 14:04 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-05 19:32 [Codel] [PATCH 1/2] codel: Controlled Delay AQM Dave Taht
2012-05-05 19:32 ` [Codel] [PATCH 2/2] codel: RED is dead Dave Taht
2012-05-05 19:47 ` [Codel] [PATCH 1/2] codel: Controlled Delay AQM Dave Taht
2012-05-05 20:05 ` Eric Dumazet
2012-05-07 16:43 ` Rick Jones
2012-05-07 17:10 ` Eric Dumazet
2012-05-07 17:18 ` Dave Taht
2012-05-07 17:27 ` Rick Jones
2012-05-07 17:34 ` Eric Dumazet
2012-05-07 17:52 ` Dave Taht
2012-05-07 18:01 ` Eric Dumazet
2012-05-07 18:09 ` Eric Dumazet
2012-05-07 18:15 ` Eric Dumazet
2012-05-07 18:32 ` Jim Gettys
2012-05-07 18:44 ` Dave Taht
2012-05-07 18:50 ` Jim Gettys
2012-05-07 19:16 ` Eric Dumazet
2012-05-07 19:36 ` Jim Gettys
2012-05-07 20:03 ` Eric Dumazet
2012-05-07 20:55 ` dave taht
2012-05-08 1:15 ` Jim Gettys
2012-05-08 1:20 ` Andrew McGregor
2012-05-08 1:45 ` Jim Gettys
2012-05-08 2:16 ` Rick Jones
2012-05-08 3:14 ` Jim Gettys
2012-05-08 3:32 ` Rick Jones
2012-05-08 3:40 ` Kathleen Nichols
2012-05-08 4:14 ` Jim Gettys
2012-05-08 5:10 ` Eric Dumazet
2012-05-08 10:25 ` Jim Gettys
2012-05-08 13:57 ` Kathleen Nichols
2012-05-08 14:04 ` Jim Gettys
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox