[PATCH iproute2-next 0/9] Add tc-mqprio and tc-taprio support for preemptible traffic classes

[PATCH iproute2-next 0/9] Add tc-mqprio and tc-taprio support for preemptible traffic classes

[Vladimir Oltean]

This is the iproute2 support for the tc program to make use of the
kernel feature for preemptible traffic classes described here:
https://patchwork.kernel.org/project/netdevbpf/cover/20230403103440.2895683-1-vladimir.oltean@nxp.com/

The state of the man pages prior to this work was a bit unsatisfactory,
so patches 03-07 contain some man page cleanup in tc-taprio(8) and
tc-mqprio(8).

Vladimir Oltean (9):
  uapi: add definitions for preemptible traffic classes in mqprio and
    taprio
  utils: add max() definition
  tc/taprio: add max-sdu to the man page SYNOPSIS section
  tc/taprio: add a size table to the examples from the man page
  tc/mqprio: fix stray ] in man page synopsis
  tc/mqprio: use words in man page to express min_rate/max_rate
    dependency on bw_rlimit
  tc/mqprio: break up synopsis into multiple lines
  tc/mqprio: add support for preemptible traffic classes
  tc/taprio: add support for preemptible traffic classes

 include/uapi/linux/pkt_sched.h | 17 ++++++
 include/utils.h                |  8 +++
 man/man8/tc-mqprio.8           | 92 ++++++++++++++++++++++---------
 man/man8/tc-taprio.8           | 27 ++++++++--
 tc/q_mqprio.c                  | 98 +++++++++++++++++++++++++++++++++
 tc/q_taprio.c                  | 99 +++++++++++++++++++++++++---------
 6 files changed, 289 insertions(+), 52 deletions(-)

-- 
2.34.1

[PATCH iproute2-next 1/9] uapi: add definitions for preemptible traffic classes in mqprio and taprio

[Vladimir Oltean]

Get the definitions from the linux/pkt_sched.h which allow us to add
netlink attributes to the TCA_OPTIONS of mqprio and taprio that specify
which traffic classes are express and which are preemptible.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.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 000eec106856..51a7addc56c6 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -719,6 +719,11 @@ enum {
 
 #define __TC_MQPRIO_SHAPER_MAX (__TC_MQPRIO_SHAPER_MAX - 1)
 
+enum {
+	TC_FP_EXPRESS = 1,
+	TC_FP_PREEMPTIBLE = 2,
+};
+
 struct tc_mqprio_qopt {
 	__u8	num_tc;
 	__u8	prio_tc_map[TC_QOPT_BITMASK + 1];
@@ -732,12 +737,23 @@ struct tc_mqprio_qopt {
 #define TC_MQPRIO_F_MIN_RATE		0x4
 #define TC_MQPRIO_F_MAX_RATE		0x8
 
+enum {
+	TCA_MQPRIO_TC_ENTRY_UNSPEC,
+	TCA_MQPRIO_TC_ENTRY_INDEX,		/* u32 */
+	TCA_MQPRIO_TC_ENTRY_FP,			/* u32 */
+
+	/* add new constants above here */
+	__TCA_MQPRIO_TC_ENTRY_CNT,
+	TCA_MQPRIO_TC_ENTRY_MAX = (__TCA_MQPRIO_TC_ENTRY_CNT - 1)
+};
+
 enum {
 	TCA_MQPRIO_UNSPEC,
 	TCA_MQPRIO_MODE,
 	TCA_MQPRIO_SHAPER,
 	TCA_MQPRIO_MIN_RATE64,
 	TCA_MQPRIO_MAX_RATE64,
+	TCA_MQPRIO_TC_ENTRY,
 	__TCA_MQPRIO_MAX,
 };
 
@@ -1236,6 +1252,7 @@ enum {
 	TCA_TAPRIO_TC_ENTRY_UNSPEC,
 	TCA_TAPRIO_TC_ENTRY_INDEX,		/* u32 */
 	TCA_TAPRIO_TC_ENTRY_MAX_SDU,		/* u32 */
+	TCA_TAPRIO_TC_ENTRY_FP,			/* u32 */
 
 	/* add new constants above here */
 	__TCA_TAPRIO_TC_ENTRY_CNT,
-- 
2.34.1

[PATCH iproute2-next 2/9] utils: add max() definition

[Vladimir Oltean]

There is already a min() definition, add this below it.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 include/utils.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/utils.h b/include/utils.h
index 2eb80b3e487c..0f1b3bef34d8 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -284,6 +284,14 @@ unsigned int print_name_and_link(const char *fmt,
 	_min1 < _min2 ? _min1 : _min2; })
 #endif
 
+#ifndef max
+# define max(x, y) ({			\
+	typeof(x) _max1 = (x);		\
+	typeof(y) _max2 = (y);		\
+	(void) (&_max1 == &_max2);	\
+	_max1 < _max2 ? _max2 : _max1; })
+#endif
+
 #ifndef __check_format_string
 # define __check_format_string(pos_str, pos_args) \
 	__attribute__ ((format (printf, (pos_str), (pos_args))))
-- 
2.34.1

[PATCH iproute2-next 3/9] tc/taprio: add max-sdu to the man page SYNOPSIS section

[Vladimir Oltean]

Although the max-sdu argument is documented in the PARAMETERS section,
it is absent from the SYNOPSIS. Add it there too.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 man/man8/tc-taprio.8 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/man/man8/tc-taprio.8 b/man/man8/tc-taprio.8
index e1f32e73bab0..9adee7fd8dde 100644
--- a/man/man8/tc-taprio.8
+++ b/man/man8/tc-taprio.8
@@ -32,6 +32,10 @@ clockid
 .ti +8
 .B sched-entry
 <command N> <gate mask N> <interval N>
+.ti +8
+[
+.B max-sdu
+<queueMaxSDU[TC 0]> <queueMaxSDU[TC 1]> <queueMaxSDU[TC N]> ]
 
 .SH DESCRIPTION
 The TAPRIO qdisc implements a simplified version of the scheduling
-- 
2.34.1

[PATCH iproute2-next 4/9] tc/taprio: add a size table to the examples from the man page

[Vladimir Oltean]

Since kernel commit a3d91b2c6f6b ("net/sched: taprio: warn about missing
size table"), the kernel emits a warning netlink extack if the user
doesn't specify a stab. We want the user be aware of the fact that the
L1 overhead is determined by taprio exactly based on the overhead of the
stab, so we want to encourage users to add a size table to the Qdisc.
Teach them how.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 man/man8/tc-taprio.8 | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/man/man8/tc-taprio.8 b/man/man8/tc-taprio.8
index 9adee7fd8dde..c3ccefea9c8a 100644
--- a/man/man8/tc-taprio.8
+++ b/man/man8/tc-taprio.8
@@ -177,7 +177,7 @@ reference CLOCK_TAI. The schedule is composed of three entries each of
 300us duration.
 
 .EX
-# tc qdisc replace dev eth0 parent root handle 100 taprio \\
+# tc qdisc replace dev eth0 parent root handle 100 stab overhead 24 taprio \\
               num_tc 3 \\
               map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \\
               queues 1@0 1@1 2@2 \\
@@ -193,7 +193,7 @@ Following is an example to enable the txtime offload mode in taprio. See
 for more information about configuring the ETF qdisc.
 
 .EX
-# tc qdisc replace dev eth0 parent root handle 100 taprio \\
+# tc qdisc replace dev eth0 parent root handle 100 stab overhead 24 taprio \\
               num_tc 3 \\
               map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \\
               queues 1@0 1@0 1@0 \\
@@ -222,10 +222,11 @@ NIC's current PTP time. In addition, the MTU for traffic class 5 is limited to
 200 octets, so that the interference this creates upon traffic class 7 during
 the time window when their gates are both open is bounded. The interference is
 determined by the transmit time of the max SDU, plus the L2 header length, plus
-the L1 overhead.
+the L1 overhead (determined from the size table specified using
+.BR tc-stab(8)).
 
 .EX
-# tc qdisc add dev eth0 parent root taprio \\
+# tc qdisc add dev eth0 parent root stab overhead 24 taprio \\
               num_tc 8 \\
               map 0 1 2 3 4 5 6 7 \\
               queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 \\
@@ -237,5 +238,8 @@ the L1 overhead.
               flags 0x2
 .EE
 
+.SH SEE ALSO
+.BR tc-stab(8)
+
 .SH AUTHORS
 Vinicius Costa Gomes <vinicius.gomes@intel.com>
-- 
2.34.1

[PATCH iproute2-next 5/9] tc/mqprio: fix stray ] in man page synopsis

[Vladimir Oltean]

The closing ] bracket doesn't close anything, it is extraneous.
Remove it.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 man/man8/tc-mqprio.8 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/man8/tc-mqprio.8 b/man/man8/tc-mqprio.8
index 16ecb9a1ddea..51c5644c36bd 100644
--- a/man/man8/tc-mqprio.8
+++ b/man/man8/tc-mqprio.8
@@ -17,7 +17,7 @@ count1@offset1 count2@offset2 ...
 .B ] [ hw
 1|0
 .B ] [ mode
-dcb|channel]
+dcb|channel
 .B ] [ shaper
 dcb|
 .B [ bw_rlimit
-- 
2.34.1

[PATCH iproute2-next 6/9] tc/mqprio: use words in man page to express min_rate/max_rate dependency on bw_rlimit

[Vladimir Oltean]

It is confusing and easy to get lost in the soup of brackets when trying
to explain that min_rate and max_rate are only accepted as optional
arguments when "shaper" takes the value "bw_rlimit".

Before (synopsis):

[ shaper dcb| [ bw_rlimit min_rate min_rate1 min_rate2 ...  max_rate max_rate1 max_rate2 ...  ]]

After (synopsis):

[ shaper dcb|bw_rlimit ] [ min_rate min_rate1 min_rate2 ... ] [ max_rate max_rate1 max_rate2 ...  ]

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 man/man8/tc-mqprio.8 | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/man/man8/tc-mqprio.8 b/man/man8/tc-mqprio.8
index 51c5644c36bd..e17c50621af0 100644
--- a/man/man8/tc-mqprio.8
+++ b/man/man8/tc-mqprio.8
@@ -19,13 +19,12 @@ count1@offset1 count2@offset2 ...
 .B ] [ mode
 dcb|channel
 .B ] [ shaper
-dcb|
-.B [ bw_rlimit
+dcb|bw_rlimit ] [
 .B min_rate
-min_rate1 min_rate2 ...
+min_rate1 min_rate2 ... ] [
 .B max_rate
 max_rate1 max_rate2 ...
-.B ]]
+.B ]
 
 
 .SH DESCRIPTION
@@ -142,11 +141,19 @@ for hardware QOS defaults. Supported with 'hw' set to 1 only.
 
 .TP
 min_rate
-Minimum value of bandwidth rate limit for a traffic class.
+Minimum value of bandwidth rate limit for a traffic class. Supported only when
+the
+.B 'shaper'
+argument is set to
+.B 'bw_rlimit'.
 
 .TP
 max_rate
-Maximum value of bandwidth rate limit for a traffic class.
+Maximum value of bandwidth rate limit for a traffic class. Supported only when
+the
+.B 'shaper'
+argument is set to
+.B 'bw_rlimit'.
 
 
 .SH EXAMPLE
-- 
2.34.1

[PATCH iproute2-next 7/9] tc/mqprio: break up synopsis into multiple lines

[Vladimir Oltean]

tc-taprio(8) has a synopsis which is much easier to follow, because it
breaks up the command line arguments on multiple lines. Do this in
tc-mqprio(8) too.

Also, the highlighting (bold) of the keywords is all wrong. Take the
opportunity to fix that.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 man/man8/tc-mqprio.8 | 41 ++++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/man/man8/tc-mqprio.8 b/man/man8/tc-mqprio.8
index e17c50621af0..3441cb68a27f 100644
--- a/man/man8/tc-mqprio.8
+++ b/man/man8/tc-mqprio.8
@@ -3,23 +3,30 @@
 MQPRIO \- Multiqueue Priority Qdisc (Offloaded Hardware QOS)
 .SH SYNOPSIS
 .B tc qdisc ... dev
-dev
-.B  ( parent
-classid
-.B | root) [ handle
-major:
-.B ] mqprio [ num_tc
-tcs
-.B ] [ map
-P0 P1 P2...
-.B ] [ queues
-count1@offset1 count2@offset2 ...
-.B ] [ hw
-1|0
-.B ] [ mode
-dcb|channel
-.B ] [ shaper
-dcb|bw_rlimit ] [
+dev (
+.B parent
+classid | root) [
+.B handle
+major: ]
+.B mqprio
+.ti +8
+[
+.B num_tc
+tcs ] [
+.B map
+P0 P1 P2... ] [
+.B queues
+count1@offset1 count2@offset2 ... ]
+.ti +8
+[
+.B hw
+1|0 ] [
+.B mode
+dcb|channel ] [
+.B shaper
+dcb|bw_rlimit ]
+.ti +8
+[
 .B min_rate
 min_rate1 min_rate2 ... ] [
 .B max_rate
-- 
2.34.1

[PATCH iproute2-next 8/9] tc/mqprio: add support for preemptible traffic classes

[Vladimir Oltean]

Add support for the "fp" argument in tc-mqprio, which takes an array
of letters "E" (for express) or "P" (for preemptible), one per traffic
class, and transforms them into TCA_MQPRIO_TC_ENTRY_FP u32 attributes of
the TCA_MQPRIO_TC_ENTRY nest. We also dump these new netlink attributes
when they come from the kernel.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 man/man8/tc-mqprio.8 | 36 ++++++++++++++--
 tc/q_mqprio.c        | 98 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 131 insertions(+), 3 deletions(-)

diff --git a/man/man8/tc-mqprio.8 b/man/man8/tc-mqprio.8
index 3441cb68a27f..724ef906090c 100644
--- a/man/man8/tc-mqprio.8
+++ b/man/man8/tc-mqprio.8
@@ -30,9 +30,11 @@ dcb|bw_rlimit ]
 .B min_rate
 min_rate1 min_rate2 ... ] [
 .B max_rate
-max_rate1 max_rate2 ...
-.B ]
-
+max_rate1 max_rate2 ... ]
+.ti +8
+[
+.B fp
+FP0 FP1 FP2 ... ]
 
 .SH DESCRIPTION
 The MQPRIO qdisc is a simple queuing discipline that allows mapping
@@ -162,6 +164,34 @@ the
 argument is set to
 .B 'bw_rlimit'.
 
+.TP
+fp
+Selects whether traffic classes are express (deliver packets via the eMAC) or
+preemptible (deliver packets via the pMAC), according to IEEE 802.1Q-2018
+clause 6.7.2 Frame preemption. Takes the form of an array (one element per
+traffic class) with values being
+.B 'E'
+(for express) or
+.B 'P'
+(for preemptible).
+
+Multiple priorities which map to the same traffic class, as well as multiple
+TXQs which map to the same traffic class, must have the same FP attributes.
+To interpret the FP as an attribute per priority, the
+.B 'map'
+argument can be used for translation. To interpret FP as an attribute per TXQ,
+the
+.B 'queues'
+argument can be used for translation.
+
+Traffic classes are express by default. The argument is supported only with
+.B 'hw'
+set to 1. Preemptible traffic classes are accepted only if the device has a MAC
+Merge layer configurable through
+.BR ethtool(8).
+
+.SH SEE ALSO
+.BR ethtool(8)
 
 .SH EXAMPLE
 
diff --git a/tc/q_mqprio.c b/tc/q_mqprio.c
index 99c43491e0be..0ecb05a5613a 100644
--- a/tc/q_mqprio.c
+++ b/tc/q_mqprio.c
@@ -29,6 +29,22 @@ static void explain(void)
 		"			  max_rate MAX_RATE1 MAX_RATE2 ... }\n");
 }
 
+static void add_tc_entries(struct nlmsghdr *n, __u32 fp[TC_QOPT_MAX_QUEUE],
+			   int num_fp_entries)
+{
+	struct rtattr *l;
+	__u32 tc;
+
+	for (tc = 0; tc < num_fp_entries; tc++) {
+		l = addattr_nest(n, 1024, TCA_MQPRIO_TC_ENTRY | NLA_F_NESTED);
+
+		addattr32(n, 1024, TCA_MQPRIO_TC_ENTRY_INDEX, tc);
+		addattr32(n, 1024, TCA_MQPRIO_TC_ENTRY_FP, fp[tc]);
+
+		addattr_nest_end(n, l);
+	}
+}
+
 static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
 			    char **argv, struct nlmsghdr *n, const char *dev)
 {
@@ -43,7 +59,10 @@ static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
 	__u64 min_rate64[TC_QOPT_MAX_QUEUE] = {0};
 	__u64 max_rate64[TC_QOPT_MAX_QUEUE] = {0};
 	__u16 shaper = TC_MQPRIO_SHAPER_DCB;
+	__u32 fp[TC_QOPT_MAX_QUEUE] = { };
 	__u16 mode = TC_MQPRIO_MODE_DCB;
+	bool have_tc_entries = false;
+	int num_fp_entries = 0;
 	int cnt_off_pairs = 0;
 	struct rtattr *tail;
 	__u32 flags = 0;
@@ -93,6 +112,21 @@ static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
 				idx++;
 				cnt_off_pairs++;
 			}
+		} else if (strcmp(*argv, "fp") == 0) {
+			while (idx < TC_QOPT_MAX_QUEUE && NEXT_ARG_OK()) {
+				NEXT_ARG();
+				if (strcmp(*argv, "E") == 0) {
+					fp[idx] = TC_FP_EXPRESS;
+				} else if (strcmp(*argv, "P") == 0) {
+					fp[idx] = TC_FP_PREEMPTIBLE;
+				} else {
+					PREV_ARG();
+					break;
+				}
+				num_fp_entries++;
+				idx++;
+			}
+			have_tc_entries = true;
 		} else if (strcmp(*argv, "hw") == 0) {
 			NEXT_ARG();
 			if (get_u8(&opt.hw, *argv, 10)) {
@@ -187,6 +221,9 @@ static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
 		addattr_l(n, 1024, TCA_MQPRIO_SHAPER,
 			  &shaper, sizeof(shaper));
 
+	if (have_tc_entries)
+		add_tc_entries(n, fp, num_fp_entries);
+
 	if (flags & TC_MQPRIO_F_MIN_RATE) {
 		struct rtattr *start;
 
@@ -218,6 +255,64 @@ static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
 	return 0;
 }
 
+static void dump_tc_entry(struct rtattr *rta, __u32 fp[TC_QOPT_MAX_QUEUE],
+			  int *max_tc_fp)
+{
+	struct rtattr *tb[TCA_MQPRIO_TC_ENTRY_MAX + 1];
+	__u32 tc, val = 0;
+
+	parse_rtattr_nested(tb, TCA_MQPRIO_TC_ENTRY_MAX, rta);
+
+	if (!tb[TCA_MQPRIO_TC_ENTRY_INDEX]) {
+		fprintf(stderr, "Missing tc entry index\n");
+		return;
+	}
+
+	tc = rta_getattr_u32(tb[TCA_MQPRIO_TC_ENTRY_INDEX]);
+	/* Prevent array out of bounds access */
+	if (tc >= TC_QOPT_MAX_QUEUE) {
+		fprintf(stderr, "Unexpected tc entry index %d\n", tc);
+		return;
+	}
+
+	if (tb[TCA_MQPRIO_TC_ENTRY_FP]) {
+		val = rta_getattr_u32(tb[TCA_MQPRIO_TC_ENTRY_FP]);
+		fp[tc] = val;
+
+		if (*max_tc_fp < (int)tc)
+			*max_tc_fp = tc;
+	}
+}
+
+static void dump_tc_entries(FILE *f, struct rtattr *opt, int len)
+{
+	__u32 fp[TC_QOPT_MAX_QUEUE] = {};
+	int max_tc_fp = -1;
+	struct rtattr *rta;
+	int tc;
+
+	for (rta = opt; RTA_OK(rta, len); rta = RTA_NEXT(rta, len)) {
+		if (rta->rta_type != (TCA_MQPRIO_TC_ENTRY | NLA_F_NESTED))
+			continue;
+
+		dump_tc_entry(rta, fp, &max_tc_fp);
+	}
+
+	if (max_tc_fp >= 0) {
+		open_json_array(PRINT_ANY,
+				is_json_context() ? "fp" : "\n             fp:");
+		for (tc = 0; tc <= max_tc_fp; tc++) {
+			print_string(PRINT_ANY, NULL, " %s",
+				     fp[tc] == TC_FP_PREEMPTIBLE ? "P" :
+				     fp[tc] == TC_FP_EXPRESS ? "E" :
+				     "?");
+		}
+		close_json_array(PRINT_ANY, "");
+
+		print_nl();
+	}
+}
+
 static int mqprio_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 {
 	int i;
@@ -309,7 +404,10 @@ static int mqprio_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 				tc_print_rate(PRINT_ANY, NULL, "%s ", max_rate64[i]);
 			close_json_array(PRINT_ANY, "");
 		}
+
+		dump_tc_entries(f, RTA_DATA(opt) + RTA_ALIGN(sizeof(*qopt)), len);
 	}
+
 	return 0;
 }
 
-- 
2.34.1

[PATCH iproute2-next 9/9] tc/taprio: add support for preemptible traffic classes

[Vladimir Oltean]

Add support for the same kind of "fp" array argument as in mqprio,
except here we already have some handling for per-tc entries (max-sdu).
We just need to expand that logic such that we also add (and parse) the
FP adminStatus property of each traffic class.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 man/man8/tc-taprio.8 | 11 +++++
 tc/q_taprio.c        | 99 +++++++++++++++++++++++++++++++++-----------
 2 files changed, 86 insertions(+), 24 deletions(-)

diff --git a/man/man8/tc-taprio.8 b/man/man8/tc-taprio.8
index c3ccefea9c8a..bf489b032a7e 100644
--- a/man/man8/tc-taprio.8
+++ b/man/man8/tc-taprio.8
@@ -36,6 +36,10 @@ clockid
 [
 .B max-sdu
 <queueMaxSDU[TC 0]> <queueMaxSDU[TC 1]> <queueMaxSDU[TC N]> ]
+.ti +8
+[
+.B fp
+<adminStatus[TC 0]> <adminStatus[TC 1]> <adminStatus[TC N]> ]
 
 .SH DESCRIPTION
 The TAPRIO qdisc implements a simplified version of the scheduling
@@ -163,6 +167,13 @@ represents the maximum L2 payload size that can egress that traffic class.
 Elements that are not filled in default to 0. The value 0 means that the
 traffic class can send packets up to the port's maximum MTU in size.
 
+.TP
+fp
+.br
+Selects whether traffic classes are express or preemptible. See
+.BR tc-mqprio(8)
+for details.
+
 .SH EXAMPLES
 
 The following example shows how an traffic schedule with three traffic
diff --git a/tc/q_taprio.c b/tc/q_taprio.c
index e00d2aa9a842..23386aa7d82f 100644
--- a/tc/q_taprio.c
+++ b/tc/q_taprio.c
@@ -148,17 +148,29 @@ static struct sched_entry *create_entry(uint32_t gatemask, uint32_t interval, ui
 }
 
 static void add_tc_entries(struct nlmsghdr *n, __u32 max_sdu[TC_QOPT_MAX_QUEUE],
-			   int num_max_sdu_entries)
+			   int num_max_sdu_entries, __u32 fp[TC_QOPT_MAX_QUEUE],
+			   int num_fp_entries)
 {
 	struct rtattr *l;
+	int num_tc;
 	__u32 tc;
 
-	for (tc = 0; tc < num_max_sdu_entries; tc++) {
+	num_tc = max(num_max_sdu_entries, num_fp_entries);
+
+	for (tc = 0; tc < num_tc; tc++) {
 		l = addattr_nest(n, 1024, TCA_TAPRIO_ATTR_TC_ENTRY | NLA_F_NESTED);
 
 		addattr_l(n, 1024, TCA_TAPRIO_TC_ENTRY_INDEX, &tc, sizeof(tc));
-		addattr_l(n, 1024, TCA_TAPRIO_TC_ENTRY_MAX_SDU,
-			  &max_sdu[tc], sizeof(max_sdu[tc]));
+
+		if (tc < num_max_sdu_entries) {
+			addattr_l(n, 1024, TCA_TAPRIO_TC_ENTRY_MAX_SDU,
+				  &max_sdu[tc], sizeof(max_sdu[tc]));
+		}
+
+		if (tc < num_fp_entries) {
+			addattr_l(n, 1024, TCA_TAPRIO_TC_ENTRY_FP, &fp[tc],
+				  sizeof(fp[tc]));
+		}
 
 		addattr_nest_end(n, l);
 	}
@@ -168,6 +180,7 @@ static int taprio_parse_opt(struct qdisc_util *qu, int argc,
 			    char **argv, struct nlmsghdr *n, const char *dev)
 {
 	__u32 max_sdu[TC_QOPT_MAX_QUEUE] = { };
+	__u32 fp[TC_QOPT_MAX_QUEUE] = { };
 	__s32 clockid = CLOCKID_INVALID;
 	struct tc_mqprio_qopt opt = { };
 	__s64 cycle_time_extension = 0;
@@ -175,6 +188,7 @@ static int taprio_parse_opt(struct qdisc_util *qu, int argc,
 	bool have_tc_entries = false;
 	int num_max_sdu_entries = 0;
 	struct rtattr *tail, *l;
+	int num_fp_entries = 0;
 	__u32 taprio_flags = 0;
 	__u32 txtime_delay = 0;
 	__s64 cycle_time = 0;
@@ -227,6 +241,23 @@ static int taprio_parse_opt(struct qdisc_util *qu, int argc,
 				free(tmp);
 				idx++;
 			}
+		} else if (strcmp(*argv, "fp") == 0) {
+			while (idx < TC_QOPT_MAX_QUEUE && NEXT_ARG_OK()) {
+				NEXT_ARG();
+				if (strcmp(*argv, "E") == 0) {
+					fp[idx] = TC_FP_EXPRESS;
+				} else if (strcmp(*argv, "P") == 0) {
+					fp[idx] = TC_FP_PREEMPTIBLE;
+				} else {
+					fprintf(stderr,
+						"Illegal \"fp\" value \"%s\", expected \"E\" or \"P\"\n",
+						*argv);
+					return -1;
+				}
+				num_fp_entries++;
+				idx++;
+			}
+			have_tc_entries = true;
 		} else if (strcmp(*argv, "max-sdu") == 0) {
 			while (idx < TC_QOPT_MAX_QUEUE && NEXT_ARG_OK()) {
 				NEXT_ARG();
@@ -369,7 +400,7 @@ static int taprio_parse_opt(struct qdisc_util *qu, int argc,
 			  &cycle_time_extension, sizeof(cycle_time_extension));
 
 	if (have_tc_entries)
-		add_tc_entries(n, max_sdu, num_max_sdu_entries);
+		add_tc_entries(n, max_sdu, num_max_sdu_entries, fp, num_fp_entries);
 
 	l = addattr_nest(n, 1024, TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST | NLA_F_NESTED);
 
@@ -460,9 +491,10 @@ static int print_schedule(FILE *f, struct rtattr **tb)
 	return 0;
 }
 
-static void dump_tc_entry(__u32 max_sdu[TC_QOPT_MAX_QUEUE],
-			  struct rtattr *item, bool *have_tc_entries,
-			  int *max_tc_index)
+static void dump_tc_entry(struct rtattr *item,
+			  __u32 max_sdu[TC_QOPT_MAX_QUEUE],
+			  __u32 fp[TC_QOPT_MAX_QUEUE],
+			  int *max_tc_max_sdu, int *max_tc_fp)
 {
 	struct rtattr *tb[TCA_TAPRIO_TC_ENTRY_MAX + 1];
 	__u32 tc, val = 0;
@@ -481,23 +513,30 @@ static void dump_tc_entry(__u32 max_sdu[TC_QOPT_MAX_QUEUE],
 		return;
 	}
 
-	if (*max_tc_index < tc)
-		*max_tc_index = tc;
-
-	if (tb[TCA_TAPRIO_TC_ENTRY_MAX_SDU])
+	if (tb[TCA_TAPRIO_TC_ENTRY_MAX_SDU]) {
 		val = rta_getattr_u32(tb[TCA_TAPRIO_TC_ENTRY_MAX_SDU]);
+		max_sdu[tc] = val;
+		if (*max_tc_max_sdu < (int)tc)
+			*max_tc_max_sdu = tc;
+	}
 
-	max_sdu[tc] = val;
+	if (tb[TCA_TAPRIO_TC_ENTRY_FP]) {
+		val = rta_getattr_u32(tb[TCA_TAPRIO_TC_ENTRY_FP]);
+		fp[tc] = val;
 
-	*have_tc_entries = true;
+		if (*max_tc_fp < (int)tc)
+			*max_tc_fp = tc;
+	}
 }
 
 static void dump_tc_entries(FILE *f, struct rtattr *opt)
 {
 	__u32 max_sdu[TC_QOPT_MAX_QUEUE] = {};
-	int tc, rem, max_tc_index = 0;
-	bool have_tc_entries = false;
+	__u32 fp[TC_QOPT_MAX_QUEUE] = {};
+	int max_tc_max_sdu = -1;
+	int max_tc_fp = -1;
 	struct rtattr *i;
+	int tc, rem;
 
 	rem = RTA_PAYLOAD(opt);
 
@@ -505,18 +544,30 @@ static void dump_tc_entries(FILE *f, struct rtattr *opt)
 		if (i->rta_type != (TCA_TAPRIO_ATTR_TC_ENTRY | NLA_F_NESTED))
 			continue;
 
-		dump_tc_entry(max_sdu, i, &have_tc_entries, &max_tc_index);
+		dump_tc_entry(i, max_sdu, fp, &max_tc_max_sdu, &max_tc_fp);
 	}
 
-	if (!have_tc_entries)
-		return;
+	if (max_tc_max_sdu >= 0) {
+		open_json_array(PRINT_ANY, "max-sdu");
+		for (tc = 0; tc <= max_tc_max_sdu; tc++)
+			print_uint(PRINT_ANY, NULL, " %u", max_sdu[tc]);
+		close_json_array(PRINT_ANY, "");
 
-	open_json_array(PRINT_ANY, "max-sdu");
-	for (tc = 0; tc <= max_tc_index; tc++)
-		print_uint(PRINT_ANY, NULL, " %u", max_sdu[tc]);
-	close_json_array(PRINT_ANY, "");
+		print_nl();
+	}
 
-	print_nl();
+	if (max_tc_fp >= 0) {
+		open_json_array(PRINT_ANY, "fp");
+		for (tc = 0; tc <= max_tc_fp; tc++) {
+			print_string(PRINT_ANY, NULL, " %s",
+				     fp[tc] == TC_FP_PREEMPTIBLE ? "P" :
+				     fp[tc] == TC_FP_EXPRESS ? "E" :
+				     "?");
+		}
+		close_json_array(PRINT_ANY, "");
+
+		print_nl();
+	}
 }
 
 static int taprio_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
-- 
2.34.1

Re: [PATCH iproute2-next 0/9] Add tc-mqprio and tc-taprio support for preemptible traffic classes

[Ferenc Fejes]

Hi Vladimir!

On Mon, 2023-04-03 at 13:52 +0300, Vladimir Oltean wrote:
> This is the iproute2 support for the tc program to make use of the
> kernel feature for preemptible traffic classes described here:
> https://patchwork.kernel.org/project/netdevbpf/cover/20230403103440.2895683-1-vladimir.oltean@nxp.com/
> 
> The state of the man pages prior to this work was a bit
> unsatisfactory,
> so patches 03-07 contain some man page cleanup in tc-taprio(8) and
> tc-mqprio(8).
> 
> Vladimir Oltean (9):
>   uapi: add definitions for preemptible traffic classes in mqprio and
>     taprio
>   utils: add max() definition
>   tc/taprio: add max-sdu to the man page SYNOPSIS section
>   tc/taprio: add a size table to the examples from the man page

Seems like Stephen merged Péter's manpages patch [1] but IMO your
version [2] is a better overhaul of that, also Péter ACK-ed to go
forward with that version. Looks like you rebased this work on the new
manpages, you have any plan to submit the changes from [2] separately?
Probably Stephen missed the whole discussion and about [2] and I'm
admit that putting acked/reviewed into a mail inside the discussion
might be misleading (probably thats show up for the original patch in
patchwork). Sorry for making it complicated.

>   tc/mqprio: fix stray ] in man page synopsis
>   tc/mqprio: use words in man page to express min_rate/max_rate
>     dependency on bw_rlimit
>   tc/mqprio: break up synopsis into multiple lines
>   tc/mqprio: add support for preemptible traffic classes
>   tc/taprio: add support for preemptible traffic classes
> 
>  include/uapi/linux/pkt_sched.h | 17 ++++++
>  include/utils.h                |  8 +++
>  man/man8/tc-mqprio.8           | 92 ++++++++++++++++++++++---------
>  man/man8/tc-taprio.8           | 27 ++++++++--
>  tc/q_mqprio.c                  | 98
> +++++++++++++++++++++++++++++++++
>  tc/q_taprio.c                  | 99 +++++++++++++++++++++++++-------
> --
>  6 files changed, 289 insertions(+), 52 deletions(-)
> 

[1]
https://lore.kernel.org/netdev/167789641838.26474.2747633103367439718.git-patchwork-notify@kernel.org/

[2]
https://lore.kernel.org/netdev/20230220161809.t2vj6daixio7uzbw@skbuf/

Best,
Ferenc

Re: [PATCH iproute2-next 0/9] Add tc-mqprio and tc-taprio support for preemptible traffic classes

[Vladimir Oltean]

Hi Ferenc,

On Mon, Apr 03, 2023 at 01:18:07PM +0200, Ferenc Fejes wrote:
> Seems like Stephen merged Péter's manpages patch [1] but IMO your
> version [2] is a better overhaul of that, also Péter ACK-ed to go
> forward with that version. Looks like you rebased this work on the new
> manpages, you have any plan to submit the changes from [2] separately?
> Probably Stephen missed the whole discussion and about [2] and I'm
> admit that putting acked/reviewed into a mail inside the discussion
> might be misleading (probably thats show up for the original patch in
> patchwork). Sorry for making it complicated.
> [1]
> https://lore.kernel.org/netdev/167789641838.26474.2747633103367439718.git-patchwork-notify@kernel.org/
> 
> [2]
> https://lore.kernel.org/netdev/20230220161809.t2vj6daixio7uzbw@skbuf/

Yes, this is true. I still have the delta between Péter's merged version
and my suggested changes, but it needs to be broken up into a gazillion
smaller patches which I haven't done yet. I now also doubt the value of
some of those changes as standalone patches. I wanted to get the preemption
stuff over with first, and this is why I've submitted only what I have.

Re: [PATCH iproute2-next 0/9] Add tc-mqprio and tc-taprio support for preemptible traffic classes

[Ferenc Fejes]

Hi Vladimir!

On Mon, 2023-04-03 at 14:23 +0300, Vladimir Oltean wrote:
> Hi Ferenc,
> 
> On Mon, Apr 03, 2023 at 01:18:07PM +0200, Ferenc Fejes wrote:
> > Seems like Stephen merged Péter's manpages patch [1] but IMO your
> > version [2] is a better overhaul of that, also Péter ACK-ed to go
> > forward with that version. Looks like you rebased this work on the
> > new
> > manpages, you have any plan to submit the changes from [2]
> > separately?
> > Probably Stephen missed the whole discussion and about [2] and I'm
> > admit that putting acked/reviewed into a mail inside the discussion
> > might be misleading (probably thats show up for the original patch
> > in
> > patchwork). Sorry for making it complicated.
> > [1]
> > https://lore.kernel.org/netdev/167789641838.26474.2747633103367439718.git-patchwork-notify@kernel.org/
> > 
> > [2]
> > https://lore.kernel.org/netdev/20230220161809.t2vj6daixio7uzbw@skbuf/
> 
> Yes, this is true. I still have the delta between Péter's merged
> version
> and my suggested changes, but it needs to be broken up into a
> gazillion
> smaller patches which I haven't done yet. I now also doubt the value
> of
> some of those changes as standalone patches. I wanted to get the
> preemption
> stuff over with first, and this is why I've submitted only what I
> have.

Understandable, thanks for the info. With all respect of your time, it
would be nice if those changes make their way into the manpages in one
way or another. I think it has enough value in its own too, but I agree
it has much lower prio than having FP merged and running :-)

Ferenc