[PATCH] net/sched: taprio: enforce minimum software scheduling interval

[PATCH] net/sched: taprio: enforce minimum software scheduling interval

[syzbot]

From: Uladzislau Zhauniarovich <uladzislau.zhauniarovich@gmail.com>

When configuring taprio with a very small schedule interval (e.g., 129 ns),
the kernel validates the interval against the time it takes to transmit a
minimum-sized Ethernet frame (60 bytes). On high-speed links, this minimum
duration is extremely small (e.g., 48 ns at 10 Gbps). Since the requested
interval is larger than this, the validation passes. Virtual devices like
veth or bonding can defeat this link-speed minimum check because they
report inflated link speeds (e.g., veth reports 10 Gbps, and bonding sums
member speeds).

However, when hardware offload is not used, taprio falls back to software
scheduling and arms an hrtimer. The hrtimer is programmed to fire at the
configured interval. If this interval is too small, it cannot sustain the
timer service cost of one advance_sched() invocation, which includes lock
acquisition, budget recomputation, and TX softirq processing. As a result,
the timer constantly falls behind, and the CPU is livelocked in hardirq
context endlessly servicing the advance_sched() hrtimer. This starves the
RCU grace-period kthreads, leading to an RCU stall panic:

rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
rcu: 	1-...!: (1 GPs behind) idle=4854/0/0x1 softirq=136062/136068 fqs=0
rcu: 	(detected by 0, t=10506 jiffies, g=161469, q=1866 ncpus=2)
Sending NMI from CPU 0 to CPUs 1:
NMI backtrace for cpu 1
CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Not tainted
Call Trace:
 <IRQ>
 lock_is_held include/linux/lockdep.h:249 [inline]
 enqueue_hrtimer+0x79/0x2c0 kernel/time/hrtimer.c:1107
 __run_hrtimer kernel/time/hrtimer.c:1946 [inline]
 __hrtimer_run_queues+0x4ce/0xa10 kernel/time/hrtimer.c:1994
 hrtimer_interrupt+0x448/0x910 kernel/time/hrtimer.c:2113
 local_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1050 [inline]
 __sysvec_apic_timer_interrupt+0x102/0x430 arch/x86/kernel/apic/apic.c:1067
 instr_sysvec_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1061
 [inline]
 sysvec_apic_timer_interrupt+0xa1/0xc0 arch/x86/kernel/apic/apic.c:1061
 </IRQ>

To fix this, enforce a hard absolute minimum interval of 100 microseconds
(TAPRIO_MIN_SW_INTERVAL_NS) for software-based scheduling, which provides
enough margin over the timer service cost. Fully offloaded schedules are
unaffected since they do not rely on the CPU's hrtimer. Introduce a helper
taprio_min_interval() to consolidate the minimum interval logic for both
individual schedule entries and the overall cycle_time validation.

Fixes: b5b73b26b3ca ("taprio: Fix allowing too small intervals")
Assisted-by: Gemini:gemini-3.5-flash Gemini:gemini-3.1-pro-preview syzbot
Reported-by: syzbot+19d01f6082ec61dd45b2@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=19d01f6082ec61dd45b2
Link: https://syzkaller.appspot.com/ai_job?id=37f064ee-e021-43aa-8cfc-9f2a5a7c35d7
Signed-off-by: Uladzislau Zhauniarovich <uladzislau.zhauniarovich@gmail.com>

---
diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 45245157e..2bf4cae16 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -49,6 +49,13 @@ static struct static_key_false taprio_have_working_mqprio;
  */
 #define TAPRIO_PICOS_PER_BYTE_MIN 17
 
+/* The software scheduler cannot sustain hrtimer intervals smaller than
+ * this without livelocking the CPU. The value must exceed the cost of
+ * one advance_sched() invocation (lock acquisition, budget recomputation,
+ * TX softirq) with margin.
+ */
+#define TAPRIO_MIN_SW_INTERVAL_NS (100 * NSEC_PER_USEC)
+
 struct sched_entry {
 	/* Durations between this GCL entry and the GCL entry where the
 	 * respective traffic class gate closes
@@ -259,6 +266,16 @@ static int length_to_duration(struct taprio_sched *q, int len)
 	return div_u64(len * atomic64_read(&q->picos_per_byte), PSEC_PER_NSEC);
 }
 
+static int taprio_min_interval(struct taprio_sched *q)
+{
+	int min_duration = length_to_duration(q, ETH_ZLEN);
+
+	if (!FULL_OFFLOAD_IS_ENABLED(q->flags))
+		min_duration = max_t(int, min_duration, TAPRIO_MIN_SW_INTERVAL_NS);
+
+	return min_duration;
+}
+
 static int duration_to_length(struct taprio_sched *q, u64 duration)
 {
 	return div_u64(duration * PSEC_PER_NSEC, atomic64_read(&q->picos_per_byte));
@@ -1038,7 +1055,7 @@ static int fill_sched_entry(struct taprio_sched *q, struct nlattr **tb,
 			    struct sched_entry *entry,
 			    struct netlink_ext_ack *extack)
 {
-	int min_duration = length_to_duration(q, ETH_ZLEN);
+	int min_duration = taprio_min_interval(q);
 	u32 interval = 0;
 
 	if (tb[TCA_TAPRIO_SCHED_ENTRY_CMD])
@@ -1054,7 +1071,8 @@ static int fill_sched_entry(struct taprio_sched *q, struct nlattr **tb,
 			tb[TCA_TAPRIO_SCHED_ENTRY_INTERVAL]);
 
 	/* The interval should allow at least the minimum ethernet
-	 * frame to go out.
+	 * frame to go out. When software scheduling is used, the
+	 * interval must also be at least the minimum hrtimer interval.
 	 */
 	if (interval < min_duration) {
 		NL_SET_ERR_MSG(extack, "Invalid interval for schedule entry");
@@ -1166,7 +1184,7 @@ static int parse_taprio_schedule(struct taprio_sched *q, struct nlattr **tb,
 		new->cycle_time = cycle;
 	}
 
-	if (new->cycle_time < new->num_entries * length_to_duration(q, ETH_ZLEN)) {
+	if (new->cycle_time < (s64)new->num_entries * taprio_min_interval(q)) {
 		NL_SET_ERR_MSG(extack, "'cycle_time' is too small");
 		return -EINVAL;
 	}


base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
-- 
See https://goo.gle/syzbot-ai-patches for information about AI-generated patches.
The person who has signed off on the patch is responsible for
addressing comments.
syzbot engineers can be reached at syzkaller@googlegroups.com.

Re: [PATCH] net/sched: taprio: enforce minimum software scheduling interval

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/#/patchset/afe041f6-ef7d-4434-b2d0-096be49b5bcb@mail.kernel.org?part=1
---
--- Patch [1]: [PATCH] net/sched: taprio: enforce minimum software scheduling interval ---
commit 984087f4873d7bad874b78ae4728e68b305dd5dc
Author: Uladzislau Zhauniarovich <uladzislau.zhauniarovich@gmail.com>
net/sched: taprio: enforce minimum software scheduling interval

This commit introduces a minimum interval of 100 microseconds for the
software scheduler in taprio to prevent CPU livelocking from excessive
hrtimer interrupts. It skips this check for fully offloaded schedules.

Link: https://syzkaller.appspot.com/bug?extid=19d01f6082ec61dd45b2
Link: https://syzkaller.appspot.com/ai_job?id=37f064ee-e021-43aa-8cfc-9f2a5a7c35d7

> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index 45245157e00a6..2bf4cae164bc2 100644
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c

[ ... ]

> @@ -259,6 +266,16 @@ static int length_to_duration(struct taprio_sched *q, int len)
>  	return div_u64(len * atomic64_read(&q->picos_per_byte), PSEC_PER_NSEC);
>  }
>  
> +static int taprio_min_interval(struct taprio_sched *q)
> +{
> +	int min_duration = length_to_duration(q, ETH_ZLEN);
> +
> +	if (!FULL_OFFLOAD_IS_ENABLED(q->flags))
> +		min_duration = max_t(int, min_duration, TAPRIO_MIN_SW_INTERVAL_NS);

[Severity: High]
Does this limit erroneously catch TXTIME_ASSIST offload configurations?

The code applies the software interval limit when FULL_OFFLOAD_IS_ENABLED
is false, which subjects TXTIME_ASSIST setups to the 100 microseconds limit.
However, TXTIME_ASSIST mode does not use the software advance_sched
hrtimer; it relies on Earliest Departure Time and NIC pacing instead.

Could this break valid hardware-assisted TSN schedules that require smaller
intervals by rejecting them during netlink configuration?

> +
> +	return min_duration;
> +}

[ ... ]


[Severity: High]
This isn't a bug introduced by this patch, but does the budget calculation
suffer from truncation on high-speed links?

When converting long gate durations to byte lengths, truncation from a 64-bit
unsigned integer to a 32-bit signed integer can occur:

net/sched/sch_taprio.c:taprio_set_budgets() {
    ...
    budget = div64_u64((u64)entry->gate_duration[tc] * PSEC_PER_NSEC,
                       atomic64_read(&q->picos_per_byte));
    atomic_set(&entry->budget[tc], budget);
    ...
}

A valid schedule with a gate duration long enough to exceed INT_MAX bytes at
high link speeds (e.g., 400 Gbps where picos_per_byte = 20) yields a budget
exceeding 2,147,483,647 bytes.

Because the budget is a signed 32-bit int, it overflows to a negative value.
Can this cause taprio_update_budgets() to return a negative value and
taprio_dequeue_from_txq() to perpetually stall and drop packets?


[Severity: High]
This is a pre-existing issue, but do failed configuration changes leave the
taprio qdisc in a partially modified state?

If a taprio_change() request passes early validation but fails later, it
appears to leave hardware and software out of sync:

net/sched/sch_taprio.c:taprio_change() {
    ...
    if (FULL_OFFLOAD_IS_ENABLED(q->flags))
        err = taprio_enable_offload(dev, q, new_admin, extack);
    ...
    if (tb[TCA_TAPRIO_ATTR_TXTIME_DELAY]) {
        if (!TXTIME_ASSIST_IS_ENABLED(q->flags)) {
            NL_SET_ERR_MSG_MOD(extack, "txtime-delay can only be set when txtime-assist mode is enabled");
            err = -EINVAL;
            goto unlock;
        }
    ...
}

Hardware offload is enabled with the new schedule, but a subsequent failure
(like the TXTIME_DELAY check) jumps to the unlock and free_sched labels.
Does this discard the software schedule without reverting the hardware
offload, causing the two to become permanently desynchronized?

Additionally, in taprio_parse_tc_entries():

net/sched/sch_taprio.c:taprio_parse_tc_entries() {
    for (tc = 0; tc < TC_QOPT_MAX_QUEUE; tc++) {
        q->max_sdu[tc] = max_sdu[tc];
        q->fp[tc] = fp[tc];
    ...
    if (have_preemption) {
        if (!FULL_OFFLOAD_IS_ENABLED(q->flags)) {
            ...
            return -EOPNOTSUPP;
        }
    ...
}

Internal state like q->max_sdu and q->fp are updated directly in the
software state before checking if preemption is actually supported. Does this
return an error without reverting them on failure?