[v3,net-next,14/15] net/sched: taprio: mask off bits in gate mask that exceed number of TCs

Commit Message

Vladimir Oltean Jan. 27, 2023, 12:15 a.m. UTC

"man tc-taprio" says:

| each gate state allows outgoing traffic for a subset (potentially
| empty) of traffic classes.

So it makes sense to not allow gate actions to have bits set for traffic
classes that exceed the number of TCs of the device (according to the
mqprio configuration).

Validating precisely that would risk introducing breakage in commands
that worked (because taprio ignores the upper bits). OTOH, the user may
not immediately realize that taprio ignores the upper bits (may confuse
the gate mask to be per TXQ rather than per TC). So at least warn to
dmesg, mask off the excess bits and continue.

For this patch to work, we need to move the assignment of the mqprio
queue configuration to the netdev above the parse_taprio_schedule()
call, because we make use of netdev_get_num_tc().

Signed-off-by: Vladimir Oltean <[e-mail]>
Reviewed-by: Jacob Keller <[e-mail]>
---
v2->v3: warn and mask off instead of failing
v1->v2: none

 net/sched/sch_taprio.c | 46 +++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 16 deletions(-)

Patch

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 6533200c5962..0415f0dbfcc8 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -789,15 +789,29 @@  static int fill_sched_entry(struct taprio_sched *q, struct nlattr **tb,
 			    struct netlink_ext_ack *extack)
 {
 	int min_duration = length_to_duration(q, ETH_ZLEN);
+	struct net_device *dev = qdisc_dev(q->root);
+	int num_tc = netdev_get_num_tc(dev);
+	u32 max_gate_mask = 0;
 	u32 interval = 0;
 
+	if (num_tc)
+		max_gate_mask = GENMASK(num_tc - 1, 0);
+
 	if (tb[TCA_TAPRIO_SCHED_ENTRY_CMD])
 		entry->command = nla_get_u8(
 			tb[TCA_TAPRIO_SCHED_ENTRY_CMD]);
 
-	if (tb[TCA_TAPRIO_SCHED_ENTRY_GATE_MASK])
+	if (tb[TCA_TAPRIO_SCHED_ENTRY_GATE_MASK]) {
 		entry->gate_mask = nla_get_u32(
 			tb[TCA_TAPRIO_SCHED_ENTRY_GATE_MASK]);
+		if (entry->gate_mask & ~max_gate_mask) {
+			netdev_warn(dev,
+				    "Gate mask 0x%x contains bits for non-existent TCs (device has %d), truncating to 0x%x",
+				    entry->gate_mask, num_tc,
+				    entry->gate_mask & max_gate_mask);
+			entry->gate_mask &= max_gate_mask;
+		}
+	}
 
 	if (tb[TCA_TAPRIO_SCHED_ENTRY_INTERVAL])
 		interval = nla_get_u32(
@@ -1605,6 +1619,21 @@  static int taprio_change(struct Qdisc *sch, struct nlattr *opt,
 		goto free_sched;
 	}
 
+	if (mqprio) {
+		err = netdev_set_num_tc(dev, mqprio->num_tc);
+		if (err)
+			goto free_sched;
+		for (i = 0; i < mqprio->num_tc; i++)
+			netdev_set_tc_queue(dev, i,
+					    mqprio->count[i],
+					    mqprio->offset[i]);
+
+		/* Always use supplied priority mappings */
+		for (i = 0; i <= TC_BITMASK; i++)
+			netdev_set_prio_tc_map(dev, i,
+					       mqprio->prio_tc_map[i]);
+	}
+
 	err = parse_taprio_schedule(q, tb, new_admin, extack);
 	if (err < 0)
 		goto free_sched;
@@ -1621,21 +1650,6 @@  static int taprio_change(struct Qdisc *sch, struct nlattr *opt,
 
 	taprio_set_picos_per_byte(dev, q);
 
-	if (mqprio) {
-		err = netdev_set_num_tc(dev, mqprio->num_tc);
-		if (err)
-			goto free_sched;
-		for (i = 0; i < mqprio->num_tc; i++)
-			netdev_set_tc_queue(dev, i,
-					    mqprio->count[i],
-					    mqprio->offset[i]);
-
-		/* Always use supplied priority mappings */
-		for (i = 0; i <= TC_BITMASK; i++)
-			netdev_set_prio_tc_map(dev, i,
-					       mqprio->prio_tc_map[i]);
-	}
-
 	if (FULL_OFFLOAD_IS_ENABLED(q->flags))
 		err = taprio_enable_offload(dev, q, new_admin, extack);
 	else