[PATCH iproute2-next 0/2] Support RED ECN nodrop mode

[PATCH iproute2-next 0/2] Support RED ECN nodrop mode

[Petr Machata]

The kernel supports a new RED mode, "ECN nodrop". In the nodrop mode,
non-ECT packets are always queued, instead of being subject to early
dropping.

This patchset adds support for the RED ECN nodrop mode to iproute2. In
patch #1, kernel headers are synced. #2 adds support for the new mode and
describes it in the man page.

Petr Machata (2):
  uapi: pkt_sched: Update headers for RED nodrop
  tc: q_red: Support 'nodrop' flag

 include/uapi/linux/pkt_sched.h | 17 +++++++++++++++++
 man/man8/tc-red.8              |  6 +++++-
 tc/q_red.c                     | 22 +++++++++++++++++-----
 tc/tc_red.c                    |  5 +++++
 4 files changed, 44 insertions(+), 6 deletions(-)

-- 
2.20.1

[PATCH iproute2-next 1/2] uapi: pkt_sched: Update headers for RED nodrop

[Petr Machata]

Add the new defines and a comment.

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 include/uapi/linux/pkt_sched.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index bbe791b2..ea39287d 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -256,6 +256,7 @@ enum {
 	TCA_RED_PARMS,
 	TCA_RED_STAB,
 	TCA_RED_MAX_P,
+	TCA_RED_FLAGS,		/* bitfield32 */
 	__TCA_RED_MAX,
 };
 
@@ -268,12 +269,28 @@ struct tc_red_qopt {
 	unsigned char   Wlog;		/* log(W)		*/
 	unsigned char   Plog;		/* log(P_max/(qth_max-qth_min))	*/
 	unsigned char   Scell_log;	/* cell size for idle damping */
+
+	/* This field can be used for flags that a RED-like qdisc has
+	 * historically supported. E.g. when configuring RED, it can be used for
+	 * ECN, HARDDROP and ADAPTATIVE. For SFQ it can be used for ECN,
+	 * HARDDROP. Etc. Because this field has not been validated, and is
+	 * copied back on dump, any bits besides those to which a given qdisc
+	 * has assigned a historical meaning need to be considered for free use
+	 * by userspace tools.
+	 *
+	 * Any further flags need to be passed differently, e.g. through an
+	 * attribute (such as TCA_RED_FLAGS above). Such attribute should allow
+	 * passing both recent and historic flags in one value.
+	 */
 	unsigned char	flags;
 #define TC_RED_ECN		1
 #define TC_RED_HARDDROP		2
 #define TC_RED_ADAPTATIVE	4
+#define TC_RED_NODROP		8
 };
 
+#define TC_RED_HISTORIC_FLAGS (TC_RED_ECN | TC_RED_HARDDROP | TC_RED_ADAPTATIVE)
+
 struct tc_red_xstats {
 	__u32           early;          /* Early drops */
 	__u32           pdrop;          /* Drops due to queue limits */
-- 
2.20.1

[PATCH iproute2-next 2/2] tc: q_red: Support 'nodrop' flag

[Petr Machata]

Recognize the new configuration option of the RED Qdisc, "nodrop". Add
support for passing flags through TCA_RED_FLAGS, and always use it for
passing RED flags.

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 man/man8/tc-red.8 |  6 +++++-
 tc/q_red.c        | 22 +++++++++++++++++-----
 tc/tc_red.c       |  5 +++++
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/man/man8/tc-red.8 b/man/man8/tc-red.8
index dd1ab74c..b5aaa986 100644
--- a/man/man8/tc-red.8
+++ b/man/man8/tc-red.8
@@ -13,7 +13,7 @@ bytes
 bytes
 .B [ burst
 packets
-.B ] [ ecn ] [ harddrop] [ bandwidth
+.B ] [ ecn ] [ harddrop ] [ nodrop ] [ bandwidth
 rate
 .B ] [ probability
 chance
@@ -123,6 +123,10 @@ If average flow queue size is above
 .B max
 bytes, this parameter forces a drop instead of ecn marking.
 .TP
+nodrop
+With this parameter, traffic that should be marked, but is not ECN-capable, is
+enqueued. Without the parameter it is early-dropped.
+.TP
 adaptive
 (Added in linux-3.3) Sets RED in adaptive mode as described in http://icir.org/floyd/papers/adaptiveRed.pdf
 .nf
diff --git a/tc/q_red.c b/tc/q_red.c
index 6256420f..ee81803e 100644
--- a/tc/q_red.c
+++ b/tc/q_red.c
@@ -30,12 +30,17 @@ static void explain(void)
 	fprintf(stderr,
 		"Usage: ... red	limit BYTES [min BYTES] [max BYTES] avpkt BYTES [burst PACKETS]\n"
 		"		[adaptive] [probability PROBABILITY] [bandwidth KBPS]\n"
-		"		[ecn] [harddrop]\n");
+		"		[ecn] [harddrop] [nodrop]\n");
 }
 
+#define RED_SUPPORTED_FLAGS (TC_RED_HISTORIC_FLAGS | TC_RED_NODROP)
+
 static int red_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 			 struct nlmsghdr *n, const char *dev)
 {
+	struct nla_bitfield32 flags_bf = {
+		.selector = RED_SUPPORTED_FLAGS,
+	};
 	struct tc_red_qopt opt = {};
 	unsigned int burst = 0;
 	unsigned int avpkt = 0;
@@ -95,13 +100,15 @@ static int red_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 				return -1;
 			}
 		} else if (strcmp(*argv, "ecn") == 0) {
-			opt.flags |= TC_RED_ECN;
+			flags_bf.value |= TC_RED_ECN;
 		} else if (strcmp(*argv, "harddrop") == 0) {
-			opt.flags |= TC_RED_HARDDROP;
+			flags_bf.value |= TC_RED_HARDDROP;
+		} else if (strcmp(*argv, "nodrop") == 0) {
+			flags_bf.value |= TC_RED_NODROP;
 		} else if (strcmp(*argv, "adaptative") == 0) {
-			opt.flags |= TC_RED_ADAPTATIVE;
+			flags_bf.value |= TC_RED_ADAPTATIVE;
 		} else if (strcmp(*argv, "adaptive") == 0) {
-			opt.flags |= TC_RED_ADAPTATIVE;
+			flags_bf.value |= TC_RED_ADAPTATIVE;
 		} else if (strcmp(*argv, "help") == 0) {
 			explain();
 			return -1;
@@ -154,6 +161,7 @@ static int red_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	addattr_l(n, 1024, TCA_RED_STAB, sbuf, 256);
 	max_P = probability * pow(2, 32);
 	addattr_l(n, 1024, TCA_RED_MAX_P, &max_P, sizeof(max_P));
+	addattr_l(n, 1024, TCA_RED_FLAGS, &flags_bf, sizeof(flags_bf));
 	addattr_nest_end(n, tail);
 	return 0;
 }
@@ -183,6 +191,10 @@ static int red_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 	    RTA_PAYLOAD(tb[TCA_RED_MAX_P]) >= sizeof(__u32))
 		max_P = rta_getattr_u32(tb[TCA_RED_MAX_P]);
 
+	if (tb[TCA_RED_FLAGS] &&
+	    RTA_PAYLOAD(tb[TCA_RED_FLAGS]) >= sizeof(__u32))
+		qopt->flags = rta_getattr_u32(tb[TCA_RED_FLAGS]);
+
 	print_uint(PRINT_JSON, "limit", NULL, qopt->limit);
 	print_string(PRINT_FP, NULL, "limit %s ", sprint_size(qopt->limit, b1));
 	print_uint(PRINT_JSON, "min", NULL, qopt->qth_min);
diff --git a/tc/tc_red.c b/tc/tc_red.c
index 681ca297..88f5ff35 100644
--- a/tc/tc_red.c
+++ b/tc/tc_red.c
@@ -116,4 +116,9 @@ void tc_red_print_flags(__u32 flags)
 		print_bool(PRINT_ANY, "adaptive", "adaptive ", true);
 	else
 		print_bool(PRINT_ANY, "adaptive", NULL, false);
+
+	if (flags & TC_RED_NODROP)
+		print_bool(PRINT_ANY, "nodrop", "nodrop ", true);
+	else
+		print_bool(PRINT_ANY, "nodrop", NULL, false);
 }
-- 
2.20.1

Re: [PATCH iproute2-next 2/2] tc: q_red: Support 'nodrop' flag

[David Ahern]

On 3/18/20 6:18 AM, Petr Machata wrote:
> @@ -154,6 +161,7 @@ static int red_parse_opt(struct qdisc_util *qu, int argc, char **argv,
>  	addattr_l(n, 1024, TCA_RED_STAB, sbuf, 256);
>  	max_P = probability * pow(2, 32);
>  	addattr_l(n, 1024, TCA_RED_MAX_P, &max_P, sizeof(max_P));
> +	addattr_l(n, 1024, TCA_RED_FLAGS, &flags_bf, sizeof(flags_bf));

the attr is a bitfield32 here ...

>  	addattr_nest_end(n, tail);
>  	return 0;
>  }
> @@ -183,6 +191,10 @@ static int red_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
>  	    RTA_PAYLOAD(tb[TCA_RED_MAX_P]) >= sizeof(__u32))
>  		max_P = rta_getattr_u32(tb[TCA_RED_MAX_P]);
>  
> +	if (tb[TCA_RED_FLAGS] &&
> +	    RTA_PAYLOAD(tb[TCA_RED_FLAGS]) >= sizeof(__u32))
> +		qopt->flags = rta_getattr_u32(tb[TCA_RED_FLAGS]);
> +

but a u32 here. These should be consistent.

Re: [PATCH iproute2-next 2/2] tc: q_red: Support 'nodrop' flag

[Petr Machata]

David Ahern <dsahern@gmail.com> writes:

> On 3/18/20 6:18 AM, Petr Machata wrote:
>> @@ -154,6 +161,7 @@ static int red_parse_opt(struct qdisc_util *qu, int argc, char **argv,
>>  	addattr_l(n, 1024, TCA_RED_STAB, sbuf, 256);
>>  	max_P = probability * pow(2, 32);
>>  	addattr_l(n, 1024, TCA_RED_MAX_P, &max_P, sizeof(max_P));
>> +	addattr_l(n, 1024, TCA_RED_FLAGS, &flags_bf, sizeof(flags_bf));
>
> the attr is a bitfield32 here ...
>
>>  	addattr_nest_end(n, tail);
>>  	return 0;
>>  }
>> @@ -183,6 +191,10 @@ static int red_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
>>  	    RTA_PAYLOAD(tb[TCA_RED_MAX_P]) >= sizeof(__u32))
>>  		max_P = rta_getattr_u32(tb[TCA_RED_MAX_P]);
>>
>> +	if (tb[TCA_RED_FLAGS] &&
>> +	    RTA_PAYLOAD(tb[TCA_RED_FLAGS]) >= sizeof(__u32))
>> +		qopt->flags = rta_getattr_u32(tb[TCA_RED_FLAGS]);
>> +
>
> but a u32 here. These should be consistent.

:-/ That's a leftover from previous ABI, thanks for catching it.
Will send a v2.