[PATCH v2 iproute2-next 00/10] Add tc-mqprio and tc-taprio support for preemptible traffic classes

[PATCH v2 iproute2-next 00/10] 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 features added in commit f7d29571ab0a ("Merge branch
'add-kernel-tc-mqprio-and-tc-taprio-support-for-preemptible-traffic-classes'").

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).

I don't know exactly what's the deal with syncing the main branch
between iproute2.git and iproute2-next.git. This patch set applies on
top of today's iproute2-next.git main branch, *merged* with today's
iproute2.git main branch. If I had formatted it directly on
iproute2-next, patch 04 would have conflicted with iproute2 change
ce4068f22db4 ("man: tc-mqprio: extend prio-tc-queue mapping with
examples"). I would recommend merging the 2 trees before applying this
series to iproute2-next.

It may be desirable for patches 01-06 to go to iproute2.git, so I've
sorted those to be first, in order to make that possible.

I also dared to sync the kernel headers and provide a commit (07/10) in
the same form as David Ahern does it. The automated script was:

  #!/bin/bash

  UAPI_FOLDER=include/uapi/
  # Built with "make -j 8 headers_install O=headers"
  KERNEL_HEADERS=/opt/net-next/headers/usr/include

  for file in $(find ${UAPI_FOLDER} -type f); do
  	filename="${file##$UAPI_FOLDER}"
  	rsync -avr "$KERNEL_HEADERS/$filename" "$file"
  done

FWIW, this kernel patch depends on frame preemption being available in
iproute2:
https://lore.kernel.org/netdev/20230418111459.811553-1-vladimir.oltean@nxp.com/

Vladimir Oltean (10):
  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/taprio: break up help text into multiple lines
  Update kernel headers
  utils: add max() definition
  tc/mqprio: add support for preemptible traffic classes
  tc/taprio: add support for preemptible traffic classes

 include/uapi/linux/bpf.h       |  61 +++++++++++++++----
 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                  |  99 ++++++++++++++++++++++++++++++
 tc/q_taprio.c                  | 108 ++++++++++++++++++++++++---------
 7 files changed, 345 insertions(+), 67 deletions(-)

-- 
2.34.1

[PATCH v2 iproute2-next 01/10] 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>
---
v1->v2: none

 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 v2 iproute2-next 02/10] 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>
---
v1->v2: none

 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 v2 iproute2-next 03/10] 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>
---
v1->v2: none

 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 v2 iproute2-next 04/10] 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 v2 iproute2-next 05/10] 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>
---
v1->v2: none

 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 v2 iproute2-next 06/10] tc/taprio: break up help text into multiple lines

[Vladimir Oltean]

Currently, the output of "tc qdisc add dev lo taprio help" looks
absolutely horrible, it looks better in the source code. Put new lines
in the output everywhere where the text switches to a new line in the
source code.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: patch is new

 tc/q_taprio.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tc/q_taprio.c b/tc/q_taprio.c
index e00d2aa9a842..c0da65fe3744 100644
--- a/tc/q_taprio.c
+++ b/tc/q_taprio.c
@@ -45,10 +45,10 @@ static void explain(void)
 {
 	fprintf(stderr,
 		"Usage: ... taprio clockid CLOCKID\n"
-		"		[num_tc NUMBER] [map P0 P1 ...] "
-		"		[queues COUNT@OFFSET COUNT@OFFSET COUNT@OFFSET ...] "
-		"		[ [sched-entry index cmd gate-mask interval] ... ] "
-		"		[base-time time] [txtime-delay delay]"
+		"		[num_tc NUMBER] [map P0 P1 ...]\n"
+		"		[queues COUNT@OFFSET COUNT@OFFSET COUNT@OFFSET ...]\n"
+		"		[ [sched-entry index cmd gate-mask interval] ... ]\n"
+		"		[base-time time] [txtime-delay delay]\n"
 		"\n"
 		"CLOCKID must be a valid SYS-V id (i.e. CLOCK_TAI)\n");
 }
-- 
2.34.1

[PATCH v2 iproute2-next 07/10] Update kernel headers

[Vladimir Oltean]

Update kernel headers to commit:
    3684a23b5aff ("Merge branch 'ocelot-felix-driver-support-for-preemptible-traffic-classes'")

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: delta generated automatically

 include/uapi/linux/bpf.h       | 61 ++++++++++++++++++++++++++++------
 include/uapi/linux/pkt_sched.h | 17 ++++++++++
 2 files changed, 67 insertions(+), 11 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index eb0588567cf8..a0a2273d7974 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1033,6 +1033,7 @@ enum bpf_attach_type {
 	BPF_PERF_EVENT,
 	BPF_TRACE_KPROBE_MULTI,
 	BPF_LSM_CGROUP,
+	BPF_STRUCT_OPS,
 	__MAX_BPF_ATTACH_TYPE
 };
 
@@ -1108,7 +1109,7 @@ enum bpf_link_type {
  */
 #define BPF_F_STRICT_ALIGNMENT	(1U << 0)
 
-/* If BPF_F_ANY_ALIGNMENT is used in BPF_PROF_LOAD command, the
+/* If BPF_F_ANY_ALIGNMENT is used in BPF_PROG_LOAD command, the
  * verifier will allow any alignment whatsoever.  On platforms
  * with strict alignment requirements for loads ands stores (such
  * as sparc and mips) the verifier validates that all loads and
@@ -1266,6 +1267,9 @@ enum {
 
 /* Create a map that is suitable to be an inner map with dynamic max entries */
 	BPF_F_INNER_MAP		= (1U << 12),
+
+/* Create a map that will be registered/unregesitered by the backed bpf_link */
+	BPF_F_LINK		= (1U << 13),
 };
 
 /* Flags for BPF_PROG_QUERY. */
@@ -1403,6 +1407,11 @@ union bpf_attr {
 		__aligned_u64	fd_array;	/* array of FDs */
 		__aligned_u64	core_relos;
 		__u32		core_relo_rec_size; /* sizeof(struct bpf_core_relo) */
+		/* output: actual total log contents size (including termintaing zero).
+		 * It could be both larger than original log_size (if log was
+		 * truncated), or smaller (if log buffer wasn't filled completely).
+		 */
+		__u32		log_true_size;
 	};
 
 	struct { /* anonymous struct used by BPF_OBJ_* commands */
@@ -1488,6 +1497,11 @@ union bpf_attr {
 		__u32		btf_size;
 		__u32		btf_log_size;
 		__u32		btf_log_level;
+		/* output: actual total log contents size (including termintaing zero).
+		 * It could be both larger than original log_size (if log was
+		 * truncated), or smaller (if log buffer wasn't filled completely).
+		 */
+		__u32		btf_log_true_size;
 	};
 
 	struct {
@@ -1507,7 +1521,10 @@ union bpf_attr {
 	} task_fd_query;
 
 	struct { /* struct used by BPF_LINK_CREATE command */
-		__u32		prog_fd;	/* eBPF program to attach */
+		union {
+			__u32		prog_fd;	/* eBPF program to attach */
+			__u32		map_fd;		/* struct_ops to attach */
+		};
 		union {
 			__u32		target_fd;	/* object to attach to */
 			__u32		target_ifindex; /* target ifindex */
@@ -1548,12 +1565,23 @@ union bpf_attr {
 
 	struct { /* struct used by BPF_LINK_UPDATE command */
 		__u32		link_fd;	/* link fd */
-		/* new program fd to update link with */
-		__u32		new_prog_fd;
+		union {
+			/* new program fd to update link with */
+			__u32		new_prog_fd;
+			/* new struct_ops map fd to update link with */
+			__u32           new_map_fd;
+		};
 		__u32		flags;		/* extra flags */
-		/* expected link's program fd; is specified only if
-		 * BPF_F_REPLACE flag is set in flags */
-		__u32		old_prog_fd;
+		union {
+			/* expected link's program fd; is specified only if
+			 * BPF_F_REPLACE flag is set in flags.
+			 */
+			__u32		old_prog_fd;
+			/* expected link's map fd; is specified only
+			 * if BPF_F_REPLACE flag is set.
+			 */
+			__u32           old_map_fd;
+		};
 	} link_update;
 
 	struct {
@@ -1647,17 +1675,17 @@ union bpf_attr {
  * 	Description
  * 		This helper is a "printk()-like" facility for debugging. It
  * 		prints a message defined by format *fmt* (of size *fmt_size*)
- * 		to file *\/sys/kernel/debug/tracing/trace* from DebugFS, if
+ * 		to file *\/sys/kernel/tracing/trace* from TraceFS, if
  * 		available. It can take up to three additional **u64**
  * 		arguments (as an eBPF helpers, the total number of arguments is
  * 		limited to five).
  *
  * 		Each time the helper is called, it appends a line to the trace.
- * 		Lines are discarded while *\/sys/kernel/debug/tracing/trace* is
- * 		open, use *\/sys/kernel/debug/tracing/trace_pipe* to avoid this.
+ * 		Lines are discarded while *\/sys/kernel/tracing/trace* is
+ * 		open, use *\/sys/kernel/tracing/trace_pipe* to avoid this.
  * 		The format of the trace is customizable, and the exact output
  * 		one will get depends on the options set in
- * 		*\/sys/kernel/debug/tracing/trace_options* (see also the
+ * 		*\/sys/kernel/tracing/trace_options* (see also the
  * 		*README* file under the same directory). However, it usually
  * 		defaults to something like:
  *
@@ -6379,6 +6407,9 @@ struct bpf_link_info {
 		struct {
 			__u32 ifindex;
 		} xdp;
+		struct {
+			__u32 map_id;
+		} struct_ops;
 	};
 } __attribute__((aligned(8)));
 
@@ -7112,4 +7143,12 @@ enum {
 	BPF_F_TIMER_ABS = (1ULL << 0),
 };
 
+/* BPF numbers iterator state */
+struct bpf_iter_num {
+	/* opaque iterator state; having __u64 here allows to preserve correct
+	 * alignment requirements in vmlinux.h, generated from BTF
+	 */
+	__u64 __opaque[1];
+} __attribute__((aligned(8)));
+
 #endif /* __LINUX_BPF_H__ */
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 v2 iproute2-next 08/10] 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>
---
v1->v2: none

 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 v2 iproute2-next 09/10] 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>
---
v1->v2: amended help text so that user space (kselftests) could detect
        the presence of the new feature

 man/man8/tc-mqprio.8 | 36 ++++++++++++++--
 tc/q_mqprio.c        | 99 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 132 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..7a4417f5363b 100644
--- a/tc/q_mqprio.c
+++ b/tc/q_mqprio.c
@@ -23,12 +23,29 @@ static void explain(void)
 		"Usage: ... mqprio	[num_tc NUMBER] [map P0 P1 ...]\n"
 		"			[queues count1@offset1 count2@offset2 ...] "
 		"[hw 1|0]\n"
+		"			[fp FP0 FP1 FP2 ...]\n"
 		"			[mode dcb|channel]\n"
 		"			[shaper bw_rlimit SHAPER_PARAMS]\n"
 		"Where: SHAPER_PARAMS := { min_rate MIN_RATE1 MIN_RATE2 ...|\n"
 		"			  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 +60,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 +113,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 +222,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 +256,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 +405,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 v2 iproute2-next 10/10] 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>
---
v1->v2: amended help text so that user space (kselftests) could detect
        the presence of the new feature

 man/man8/tc-taprio.8 |  11 +++++
 tc/q_taprio.c        | 100 ++++++++++++++++++++++++++++++++-----------
 2 files changed, 87 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 c0da65fe3744..bc29710c4686 100644
--- a/tc/q_taprio.c
+++ b/tc/q_taprio.c
@@ -49,6 +49,7 @@ static void explain(void)
 		"		[queues COUNT@OFFSET COUNT@OFFSET COUNT@OFFSET ...]\n"
 		"		[ [sched-entry index cmd gate-mask interval] ... ]\n"
 		"		[base-time time] [txtime-delay delay]\n"
+		"		[fp FP0 FP1 FP2 ...]\n"
 		"\n"
 		"CLOCKID must be a valid SYS-V id (i.e. CLOCK_TAI)\n");
 }
@@ -148,17 +149,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 +181,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 +189,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 +242,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 +401,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 +492,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 +514,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 +545,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 v2 iproute2-next 00/10] Add tc-mqprio and tc-taprio support for preemptible traffic classes

[David Ahern]

On 4/18/23 5:39 AM, Vladimir Oltean wrote:
> This is the iproute2 support for the tc program to make use of the
> kernel features added in commit f7d29571ab0a ("Merge branch
> 'add-kernel-tc-mqprio-and-tc-taprio-support-for-preemptible-traffic-classes'").
> 
> 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).

Thanks for updating the man pages. These should go through main first; I
can sync to main after those are applied and before your set if needed.


> 
> I don't know exactly what's the deal with syncing the main branch
> between iproute2.git and iproute2-next.git. This patch set applies on
> top of today's iproute2-next.git main branch, *merged* with today's
> iproute2.git main branch. If I had formatted it directly on
> iproute2-next, patch 04 would have conflicted with iproute2 change
> ce4068f22db4 ("man: tc-mqprio: extend prio-tc-queue mapping with
> examples"). I would recommend merging the 2 trees before applying this
> series to iproute2-next.

I merge main from time to time; just did that ...

> 
> It may be desirable for patches 01-06 to go to iproute2.git, so I've
> sorted those to be first, in order to make that possible.
> 
> I also dared to sync the kernel headers and provide a commit (07/10) in
> the same form as David Ahern does it. The automated script was:
> 
>   #!/bin/bash
> 
>   UAPI_FOLDER=include/uapi/
>   # Built with "make -j 8 headers_install O=headers"
>   KERNEL_HEADERS=/opt/net-next/headers/usr/include
> 
>   for file in $(find ${UAPI_FOLDER} -type f); do
>   	filename="${file##$UAPI_FOLDER}"
>   	rsync -avr "$KERNEL_HEADERS/$filename" "$file"
>   done

... and updated headers.

Repost the patches as 2 sets with the man page fixes targeted at main
and the new preemptible work for -next.

Re: [PATCH v2 iproute2-next 00/10] Add tc-mqprio and tc-taprio support for preemptible traffic classes

[Vladimir Oltean]

On Sat, Apr 22, 2023 at 10:14:01AM -0600, David Ahern wrote:
> Thanks for updating the man pages. These should go through main first; I
> can sync to main after those are applied and before your set if needed.
> 
> ... and updated headers.
> 
> Repost the patches as 2 sets with the man page fixes targeted at main
> and the new preemptible work for -next.

So the status is as follows: patches 01-06 apply cleanly to the current
iproute2/main, and after another iproute2 -> iproute2-next merge, patch
07 can be skipped and patches 08-10 also apply cleanly to the resulting
iproute2-next branch.

Unless there are changes I need to make to the contents of the patches,
could you take these from the lists, or is that a no-no?

Re: [PATCH v2 iproute2-next 00/10] Add tc-mqprio and tc-taprio support for preemptible traffic classes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to iproute2/iproute2.git (main)
by Stephen Hemminger <stephen@networkplumber.org>:

On Tue, 18 Apr 2023 14:39:43 +0300 you wrote:
> This is the iproute2 support for the tc program to make use of the
> kernel features added in commit f7d29571ab0a ("Merge branch
> 'add-kernel-tc-mqprio-and-tc-taprio-support-for-preemptible-traffic-classes'").
> 
> 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).
> 
> [...]

Here is the summary with links:
  - [v2,iproute2-next,01/10] tc/taprio: add max-sdu to the man page SYNOPSIS section
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=bad08997cfec
  - [v2,iproute2-next,02/10] tc/taprio: add a size table to the examples from the man page
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=201e2f968bd2
  - [v2,iproute2-next,03/10] tc/mqprio: fix stray ] in man page synopsis
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=4f4e2481e3a5
  - [v2,iproute2-next,04/10] tc/mqprio: use words in man page to express min_rate/max_rate dependency on bw_rlimit
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=82289b7addab
  - [v2,iproute2-next,05/10] tc/mqprio: break up synopsis into multiple lines
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=8c028693cd5f
  - [v2,iproute2-next,06/10] tc/taprio: break up help text into multiple lines
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=b54a4c9fc0cb
  - [v2,iproute2-next,07/10] Update kernel headers
    (no matching commit)
  - [v2,iproute2-next,08/10] utils: add max() definition
    (no matching commit)
  - [v2,iproute2-next,09/10] tc/mqprio: add support for preemptible traffic classes
    (no matching commit)
  - [v2,iproute2-next,10/10] tc/taprio: add support for preemptible traffic classes
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v2 iproute2-next 00/10] Add tc-mqprio and tc-taprio support for preemptible traffic classes

[David Ahern]

On 4/22/23 10:59 AM, Vladimir Oltean wrote:
> Unless there are changes I need to make to the contents of the patches,
> could you take these from the lists, or is that a no-no?

iproute2 follows the netdev dev model with a main tree for bug fixes and
-next tree for features. In the future please separate out the patches
and send with proper targets. If a merge is needed you can state that in
the cover letter of the set for -next.

Re: [PATCH v2 iproute2-next 00/10] Add tc-mqprio and tc-taprio support for preemptible traffic classes

[Vladimir Oltean]

On Mon, Apr 24, 2023 at 07:47:31PM -0600, David Ahern wrote:
> On 4/22/23 10:59 AM, Vladimir Oltean wrote:
> > Unless there are changes I need to make to the contents of the patches,
> > could you take these from the lists, or is that a no-no?
> 
> iproute2 follows the netdev dev model with a main tree for bug fixes and
> -next tree for features. In the future please separate out the patches
> and send with proper targets. If a merge is needed you can state that in
> the cover letter of the set for -next.

I know that the trees are split and it is no coincidence that my patches
were sorted in the correct order. I've been working for 10 months on
this small feature and I was impatient to get it over with, so I wanted
to eliminate one round-trip time if possible (send to "iproute2", ask
for merge, send to "iproute2-next"). I requested this honestly thinking
that there would be no difference to the end result, only less pretentious
in terms of the process. If there is any automation (I didn't see any in
Patchwork at least) or any other reason that would justify the more
pretentious process, then again, my excuses, I plead ignorance and I
will follow it more strictly next time, but I'd also like to know it :)

Re: [PATCH v2 iproute2-next 00/10] Add tc-mqprio and tc-taprio support for preemptible traffic classes

[David Ahern]

On 4/25/23 6:55 AM, Vladimir Oltean wrote:
> On Mon, Apr 24, 2023 at 07:47:31PM -0600, David Ahern wrote:
>> On 4/22/23 10:59 AM, Vladimir Oltean wrote:
>>> Unless there are changes I need to make to the contents of the patches,
>>> could you take these from the lists, or is that a no-no?
>>
>> iproute2 follows the netdev dev model with a main tree for bug fixes and
>> -next tree for features. In the future please separate out the patches
>> and send with proper targets. If a merge is needed you can state that in
>> the cover letter of the set for -next.
> 
> I know that the trees are split and it is no coincidence that my patches
> were sorted in the correct order. I've been working for 10 months on
> this small feature and I was impatient to get it over with, so I wanted
> to eliminate one round-trip time if possible (send to "iproute2", ask
> for merge, send to "iproute2-next"). I requested this honestly thinking
> that there would be no difference to the end result, only less pretentious
> in terms of the process. If there is any automation (I didn't see any in
> Patchwork at least) or any other reason that would justify the more
> pretentious process, then again, my excuses, I plead ignorance and I
> will follow it more strictly next time, but I'd also like to know it :)

Maybe the word choice here is a language issue, but it is not a
'pretentious' process, it is "the" process for submitting patches to
both networking trees and iproute2 trees. You would not send a mixed
patch set to the netdev maintainers, so don't do it for iproute2.

Re: [PATCH v2 iproute2-next 09/10] tc/mqprio: add support for preemptible traffic classes

[Stephen Hemminger]

On Tue, 18 Apr 2023 14:39:52 +0300
Vladimir Oltean <vladimir.oltean@nxp.com> wrote:

> +	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;
> +	}

This creates a ABI dependency, what if kernel config changes?

Re: [PATCH v2 iproute2-next 09/10] tc/mqprio: add support for preemptible traffic classes

[Vladimir Oltean]

On Sat, Apr 29, 2023 at 05:42:55PM -0700, Stephen Hemminger wrote:
> On Tue, 18 Apr 2023 14:39:52 +0300
> Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> 
> > +	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;
> > +	}
> 
> This creates a ABI dependency, what if kernel config changes?
>

If TC_QOPT_MAX_QUEUE changes its value, struct tc_mqprio_qopt changes
its binary layout in an incompatible way.