Message ID | 20230130173145.475943-15-vladimir.oltean@nxp.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | ENETC mqprio/taprio cleanup | expand |
Context | Check | Description |
---|---|---|
netdev/tree_selection | success | Clearly marked for net-next, async |
netdev/fixes_present | success | Fixes tag not required for -next series |
netdev/subject_prefix | success | Link |
netdev/cover_letter | success | Series has a cover letter |
netdev/patch_count | success | Link |
netdev/header_inline | success | No static functions without inline keyword in header files |
netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
netdev/cc_maintainers | success | CCed 9 of 9 maintainers |
netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
netdev/module_param | success | Was 0 now: 0 |
netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
netdev/check_selftest | success | No net selftest shell script |
netdev/verify_fixes | success | No Fixes tag |
netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 72 lines checked |
netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
netdev/source_inline | success | Was 0 now: 0 |
On Mon, Jan 30, 2023 at 07:31:44PM +0200, Vladimir Oltean wrote: > "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 <vladimir.oltean@nxp.com> > Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Reviewed-by: Simon Horman <simon.horman@corigine.com>
diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c index f40016275384..a9873056ea97 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