| Message ID | 20240527153955.553333-1-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e634134180885574d1fe7aa162777ba41e7fcd5b |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,1/2] net/sched: taprio: make q->picos_per_byte available to fill_sched_entry() | expand |
On Mon, May 27, 2024 at 5:40 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > In commit b5b73b26b3ca ("taprio: Fix allowing too small intervals"), a > comparison of user input against length_to_duration(q, ETH_ZLEN) was > introduced, to avoid RCU stalls due to frequent hrtimers. > > The implementation of length_to_duration() depends on q->picos_per_byte > being set for the link speed. The blamed commit in the Fixes: tag has > moved this too late, so the checks introduced above are ineffective. > The q->picos_per_byte is zero at parse_taprio_schedule() -> > parse_sched_list() -> parse_sched_entry() -> fill_sched_entry() time. > > Move the taprio_set_picos_per_byte() call as one of the first things in > taprio_change(), before the bulk of the netlink attribute parsing is > done. That's because it is needed there. > > Add a selftest to make sure the issue doesn't get reintroduced. > > Fixes: 09dbdf28f9f9 ("net/sched: taprio: fix calculation of maximum gate durations") > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Eric Dumazet <edumazet@google.com> Thanks!
Hello: This series was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Mon, 27 May 2024 18:39:54 +0300 you wrote: > In commit b5b73b26b3ca ("taprio: Fix allowing too small intervals"), a > comparison of user input against length_to_duration(q, ETH_ZLEN) was > introduced, to avoid RCU stalls due to frequent hrtimers. > > The implementation of length_to_duration() depends on q->picos_per_byte > being set for the link speed. The blamed commit in the Fixes: tag has > moved this too late, so the checks introduced above are ineffective. > The q->picos_per_byte is zero at parse_taprio_schedule() -> > parse_sched_list() -> parse_sched_entry() -> fill_sched_entry() time. > > [...] Here is the summary with links: - [net,1/2] net/sched: taprio: make q->picos_per_byte available to fill_sched_entry() https://git.kernel.org/netdev/net/c/e63413418088 - [net,2/2] net/sched: taprio: extend minimum interval restriction to entire cycle too https://git.kernel.org/netdev/net/c/fb66df20a720 You are awesome, thank you!
diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c index 1ab17e8a7260..118915055360 100644 --- a/net/sched/sch_taprio.c +++ b/net/sched/sch_taprio.c @@ -1848,6 +1848,9 @@ static int taprio_change(struct Qdisc *sch, struct nlattr *opt, } q->flags = taprio_flags; + /* Needed for length_to_duration() during netlink attribute parsing */ + taprio_set_picos_per_byte(dev, q); + err = taprio_parse_mqprio_opt(dev, mqprio, extack, q->flags); if (err < 0) return err; @@ -1907,7 +1910,6 @@ static int taprio_change(struct Qdisc *sch, struct nlattr *opt, if (err < 0) goto free_sched; - taprio_set_picos_per_byte(dev, q); taprio_update_queue_max_sdu(q, new_admin, stab); if (FULL_OFFLOAD_IS_ENABLED(q->flags)) diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json index 12da0a939e3e..8f12f00a4f57 100644 --- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json +++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/taprio.json @@ -132,6 +132,28 @@ "echo \"1\" > /sys/bus/netdevsim/del_device" ] }, + { + "id": "6f62", + "name": "Add taprio Qdisc with too short interval", + "category": [ + "qdisc", + "taprio" + ], + "plugins": { + "requires": "nsPlugin" + }, + "setup": [ + "echo \"1 1 8\" > /sys/bus/netdevsim/new_device" + ], + "cmdUnderTest": "$TC qdisc add dev $ETH root handle 1: taprio num_tc 2 queues 1@0 1@1 sched-entry S 01 300 sched-entry S 02 1700 clockid CLOCK_TAI", + "expExitCode": "2", + "verifyCmd": "$TC qdisc show dev $ETH", + "matchPattern": "qdisc taprio 1: root refcnt", + "matchCount": "0", + "teardown": [ + "echo \"1\" > /sys/bus/netdevsim/del_device" + ] + }, { "id": "3e1e", "name": "Add taprio Qdisc with an invalid cycle-time",
In commit b5b73b26b3ca ("taprio: Fix allowing too small intervals"), a comparison of user input against length_to_duration(q, ETH_ZLEN) was introduced, to avoid RCU stalls due to frequent hrtimers. The implementation of length_to_duration() depends on q->picos_per_byte being set for the link speed. The blamed commit in the Fixes: tag has moved this too late, so the checks introduced above are ineffective. The q->picos_per_byte is zero at parse_taprio_schedule() -> parse_sched_list() -> parse_sched_entry() -> fill_sched_entry() time. Move the taprio_set_picos_per_byte() call as one of the first things in taprio_change(), before the bulk of the netlink attribute parsing is done. That's because it is needed there. Add a selftest to make sure the issue doesn't get reintroduced. Fixes: 09dbdf28f9f9 ("net/sched: taprio: fix calculation of maximum gate durations") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- net/sched/sch_taprio.c | 4 +++- .../tc-testing/tc-tests/qdiscs/taprio.json | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-)