[Codel] [RFCv2 PATCH] iproute2: Add ecn_target option to codel and fq_codel

["Dave Täht" <dave.taht@bufferbloat.net>]

From: Dave Taht <dave.taht@bufferbloat.net>

ECN was not part of the original codel design and adding support
for it revealed problems in two areas. 1) ECN can be gamed.
2) Dropping packets under overload more rapidly frees up
bandwidth than marking packets.

Two possible scenarios of use - on egress from a network,
ecn_target could be set low, to drop more often, to
ensure lowest latency for other packets.

On ingress, it could be set high to mark packets more often,
to lower data loss while still signalling the end application
that bandwidth is a problem.

ecn_target is not engaged until after codel enters a dropping
state overall.
---
 include/linux/pkt_sched.h |    2 ++
 man/man8/tc-codel.8       |    3 +++
 man/man8/tc-fq_codel.8    |    5 +++++
 tc/q_codel.c              |   17 ++++++++++++++++-
 tc/q_fq_codel.c           |   17 ++++++++++++++++-
 5 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 32aef0a..f0dcf8c 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -663,6 +663,7 @@ enum {
 	TCA_CODEL_LIMIT,
 	TCA_CODEL_INTERVAL,
 	TCA_CODEL_ECN,
+	TCA_CODEL_ECN_TARGET,
 	__TCA_CODEL_MAX
 };
 
@@ -691,6 +692,7 @@ enum {
 	TCA_FQ_CODEL_ECN,
 	TCA_FQ_CODEL_FLOWS,
 	TCA_FQ_CODEL_QUANTUM,
+	TCA_FQ_CODEL_ECN_TARGET,
 	__TCA_FQ_CODEL_MAX
 };
 
diff --git a/man/man8/tc-codel.8 b/man/man8/tc-codel.8
index 61f163f..0e97249 100644
--- a/man/man8/tc-codel.8
+++ b/man/man8/tc-codel.8
@@ -64,6 +64,9 @@ is the acceptable minimum standing/persistent queue delay. This minimum delay
 is identified by tracking the local minimum queue delay that packets experience.
 Default and recommended value is 5ms.
 
+.SS ecn_target
+is the overall delay at which ecn marked packets will be dropped rather than marked.
+
 .SS interval
 is used to ensure that the measured minimum delay does not become too stale. The
 minimum delay must be experienced in the last epoch of length
diff --git a/man/man8/tc-fq_codel.8 b/man/man8/tc-fq_codel.8
index 8b43c10..9cb5be8 100644
--- a/man/man8/tc-fq_codel.8
+++ b/man/man8/tc-fq_codel.8
@@ -49,6 +49,11 @@ and is the acceptable minimum
 standing/persistent queue delay. This minimum delay is identified by tracking
 the local minimum queue delay that packets experience.  Default value is 5ms.
 
+.SS ecn_target
+has the same semantics as
+.B codel
+and is the delay beyond which ecn packets will be dropped rather than marked.
+
 .SS interval
 has the same semantics as
 .B codel
diff --git a/tc/q_codel.c b/tc/q_codel.c
index dc4b3f6..1400680 100644
--- a/tc/q_codel.c
+++ b/tc/q_codel.c
@@ -53,7 +53,7 @@
 
 static void explain(void)
 {
-	fprintf(stderr, "Usage: ... codel [ limit PACKETS ] [ target TIME]\n");
+	fprintf(stderr, "Usage: ... codel [ limit PACKETS ] [ target TIME] [ ecn_target TIME ]\n");
 	fprintf(stderr, "                 [ interval TIME ] [ ecn | noecn ]\n");
 }
 
@@ -62,6 +62,7 @@ static int codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 {
 	unsigned limit = 0;
 	unsigned target = 0;
+	unsigned ecn_target = 0;
 	unsigned interval = 0;
 	int ecn = -1;
 	struct rtattr *tail;
@@ -79,6 +80,12 @@ static int codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 				fprintf(stderr, "Illegal \"target\"\n");
 				return -1;
 			}
+		} else if (strcmp(*argv, "ecn_target") == 0) {
+			NEXT_ARG();
+			if (get_time(&ecn_target, *argv)) {
+				fprintf(stderr, "Illegal \"ecn_target\"\n");
+				return -1;
+			}
 		} else if (strcmp(*argv, "interval") == 0) {
 			NEXT_ARG();
 			if (get_time(&interval, *argv)) {
@@ -108,6 +115,8 @@ static int codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 		addattr_l(n, 1024, TCA_CODEL_INTERVAL, &interval, sizeof(interval));
 	if (target)
 		addattr_l(n, 1024, TCA_CODEL_TARGET, &target, sizeof(target));
+	if (ecn_target)
+		addattr_l(n, 1024, TCA_CODEL_ECN_TARGET, &ecn_target, sizeof(ecn_target));
 	if (ecn != -1)
 		addattr_l(n, 1024, TCA_CODEL_ECN, &ecn, sizeof(ecn));
 	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
@@ -120,6 +129,7 @@ static int codel_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 	unsigned limit;
 	unsigned interval;
 	unsigned target;
+	unsigned ecn_target;
 	unsigned ecn;
 	SPRINT_BUF(b1);
 
@@ -138,6 +148,11 @@ static int codel_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 		target = rta_getattr_u32(tb[TCA_CODEL_TARGET]);
 		fprintf(f, "target %s ", sprint_time(target, b1));
 	}
+	if (tb[TCA_CODEL_ECN_TARGET] &&
+	    RTA_PAYLOAD(tb[TCA_CODEL_ECN_TARGET]) >= sizeof(__u32)) {
+		target = rta_getattr_u32(tb[TCA_CODEL_ECN_TARGET]);
+		fprintf(f, "ecn_target %s ", sprint_time(ecn_target, b1));
+	}
 	if (tb[TCA_CODEL_INTERVAL] &&
 	    RTA_PAYLOAD(tb[TCA_CODEL_INTERVAL]) >= sizeof(__u32)) {
 		interval = rta_getattr_u32(tb[TCA_CODEL_INTERVAL]);
diff --git a/tc/q_fq_codel.c b/tc/q_fq_codel.c
index 1d3bfa2..db14d52 100644
--- a/tc/q_fq_codel.c
+++ b/tc/q_fq_codel.c
@@ -51,7 +51,7 @@
 static void explain(void)
 {
 	fprintf(stderr, "Usage: ... fq_codel [ limit PACKETS ] [ flows NUMBER ]\n");
-	fprintf(stderr, "                    [ target TIME] [ interval TIME ]\n");
+	fprintf(stderr, "                    [ target TIME] [ ecn_target TIME ] [ interval TIME ]\n");
 	fprintf(stderr, "                    [ quantum BYTES ] [ [no]ecn ]\n");
 }
 
@@ -61,6 +61,7 @@ static int fq_codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	unsigned limit = 0;
 	unsigned flows = 0;
 	unsigned target = 0;
+	unsigned ecn_target = 0;
 	unsigned interval = 0;
 	unsigned quantum = 0;
 	int ecn = -1;
@@ -91,6 +92,12 @@ static int fq_codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 				fprintf(stderr, "Illegal \"target\"\n");
 				return -1;
 			}
+		} else if (strcmp(*argv, "ecn_target") == 0) {
+			NEXT_ARG();
+			if (get_time(&ecn_target, *argv)) {
+				fprintf(stderr, "Illegal \"ecn_target\"\n");
+				return -1;
+			}
 		} else if (strcmp(*argv, "interval") == 0) {
 			NEXT_ARG();
 			if (get_time(&interval, *argv)) {
@@ -124,6 +131,8 @@ static int fq_codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 		addattr_l(n, 1024, TCA_FQ_CODEL_INTERVAL, &interval, sizeof(interval));
 	if (target)
 		addattr_l(n, 1024, TCA_FQ_CODEL_TARGET, &target, sizeof(target));
+	if (ecn_target)
+		addattr_l(n, 1024, TCA_FQ_CODEL_ECN_TARGET, &ecn_target, sizeof(ecn_target));
 	if (ecn != -1)
 		addattr_l(n, 1024, TCA_FQ_CODEL_ECN, &ecn, sizeof(ecn));
 	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
@@ -137,6 +146,7 @@ static int fq_codel_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt
 	unsigned flows;
 	unsigned interval;
 	unsigned target;
+	unsigned ecn_target;
 	unsigned ecn;
 	unsigned quantum;
 	SPRINT_BUF(b1);
@@ -166,6 +176,11 @@ static int fq_codel_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt
 		target = rta_getattr_u32(tb[TCA_FQ_CODEL_TARGET]);
 		fprintf(f, "target %s ", sprint_time(target, b1));
 	}
+	if (tb[TCA_FQ_CODEL_ECN_TARGET] &&
+	    RTA_PAYLOAD(tb[TCA_FQ_CODEL_ECN_TARGET]) >= sizeof(__u32)) {
+		target = rta_getattr_u32(tb[TCA_FQ_CODEL_ECN_TARGET]);
+		fprintf(f, "ecn_target %s ", sprint_time(ecn_target, b1));
+	}
 	if (tb[TCA_FQ_CODEL_INTERVAL] &&
 	    RTA_PAYLOAD(tb[TCA_FQ_CODEL_INTERVAL]) >= sizeof(__u32)) {
 		interval = rta_getattr_u32(tb[TCA_FQ_CODEL_INTERVAL]);
-- 
1.7.9.5