[PATCH net 1/2] net/sched: codel: bound the dropping loop per dequeue call

[PATCH net 1/2] net/sched: codel: bound the dropping loop per dequeue call

[Jamal Hadi Salim]

The CoDel control law schedules the next drop one interval/sqrt(count)
after the previous drop, using the configured interval
(codel_params.interval). For very small intervals the scheduled step
rounds down to zero, so the dropping loop in codel_dequeue() never
advances and drains the entire backlog under the qdisc lock in one
call - an unprivileged user can trigger a soft lockup this way.

Fix in the shared codel code used by both codel and fq_codel:

1. Make the control-law step at least 1 tick so the dropping loop
   always moves forward.

2. Cap the dropping loop at CODEL_MAX_DROPS_PER_DEQUEUE (256) drops
   per codel_dequeue() call, resyncing drop_next to now when the cap
   is hit: the catch-up owed to the loop grows with the idle gap and
   the backlog, which no interval threshold can bound. This is a
   deliberate behaviour change after long idle gaps.

The cap applies to fq_codel (4b549a2ef4be) and the mac80211 TXQ path
(fixed interval, cap only).

The target sojourn delay (codel_params.target) is not validated: it
does not feed the control law, so a sub-tick value is aggressive
rather than deadlock-prone.

Conditions to recreate the bug (repro from vega@nebusec.ai):
  - tc qdisc add dev lo root handle 1: tbf rate 1kbit burst 2kb limit 1000000
  - tc qdisc add dev lo parent 1:1 handle 10: codel interval 2us target 1ms noecn limit 1000000 (same for fq_codel)
  - unpatched kernel: tc accepts it; a UDP flood under the 1kbit tbf
    soft-lockups (watchdog: BUG: soft lockup) while one
    codel_dequeue() call drops the backlog under the qdisc lock
  - patched kernel: same setup, at most 256 drops per dequeue call,
    no soft lockup

Fixes: 76e3cc126bb2 ("codel: Controlled Delay AQM")
Reported-by: Vega <vega@nebusec.ai>
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 include/net/codel.h      |  5 +++++
 include/net/codel_impl.h | 16 +++++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/include/net/codel.h b/include/net/codel.h
index aa80f744826c..183d43c2bd43 100644
--- a/include/net/codel.h
+++ b/include/net/codel.h
@@ -140,6 +140,11 @@ struct codel_vars {
 /* needed shift to get a Q0.32 number from rec_inv_sqrt */
 #define REC_INV_SQRT_SHIFT (32 - REC_INV_SQRT_BITS)
 
+/* Cap on drops per codel_dequeue() call: the loop's work depends on the
+ * idle gap and backlog, both outside our control; resync when exceeded.
+ */
+#define CODEL_MAX_DROPS_PER_DEQUEUE 256
+
 /**
  * struct codel_stats - contains codel shared variables and stats
  * @maxpacket:	largest packet we've seen so far
diff --git a/include/net/codel_impl.h b/include/net/codel_impl.h
index 2c1f0ec309e9..9e779cd4d6de 100644
--- a/include/net/codel_impl.h
+++ b/include/net/codel_impl.h
@@ -93,12 +93,17 @@ static void codel_Newton_step(struct codel_vars *vars)
  * CoDel control_law is t + interval/sqrt(count)
  * We maintain in rec_inv_sqrt the reciprocal value of sqrt(count) to avoid
  * both sqrt() and divide operation.
+ *
+ * Clamp the increment to at least 1 tick: a very small interval (or a
+ * large count) can truncate it to zero, stalling the dropping loop.
  */
 static codel_time_t codel_control_law(codel_time_t t,
 				      codel_time_t interval,
 				      u32 rec_inv_sqrt)
 {
-	return t + reciprocal_scale(interval, rec_inv_sqrt << REC_INV_SQRT_SHIFT);
+	return t + max_t(u32, 1,
+			 reciprocal_scale(interval,
+					  rec_inv_sqrt << REC_INV_SQRT_SHIFT));
 }
 
 static bool codel_should_drop(const struct sk_buff *skb,
@@ -156,6 +161,7 @@ static struct sk_buff *codel_dequeue(void *ctx,
 	struct sk_buff *skb = dequeue_func(vars, ctx);
 	codel_time_t now;
 	bool drop;
+	unsigned int drops = 0;
 
 	if (!skb) {
 		vars->first_above_time = 0;
@@ -180,6 +186,14 @@ static struct sk_buff *codel_dequeue(void *ctx,
 			 */
 			while (vars->dropping &&
 			       codel_time_after_eq(now, vars->drop_next)) {
+				if (++drops > CODEL_MAX_DROPS_PER_DEQUEUE) {
+					/* fell far behind the schedule */
+					WRITE_ONCE(vars->drop_next,
+						   codel_control_law(now,
+								     params->interval,
+								     vars->rec_inv_sqrt));
+					break;
+				}
 				/* dont care of possible wrap
 				 * since there is no more divide.
 				 */
-- 
2.43.0

Re: [PATCH net 1/2] net/sched: codel: bound the dropping loop per dequeue call

[Eric Dumazet]

On Thu, Sep 3, 2026 at 10:56 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> The CoDel control law schedules the next drop one interval/sqrt(count)
> after the previous drop, using the configured interval
> (codel_params.interval). For very small intervals the scheduled step
> rounds down to zero, so the dropping loop in codel_dequeue() never
> advances and drains the entire backlog under the qdisc lock in one
> call - an unprivileged user can trigger a soft lockup this way.
>

...

>  static bool codel_should_drop(const struct sk_buff *skb,
> @@ -156,6 +161,7 @@ static struct sk_buff *codel_dequeue(void *ctx,
>         struct sk_buff *skb = dequeue_func(vars, ctx);
>         codel_time_t now;
>         bool drop;
> +       unsigned int drops = 0;

SGTM, note that this line breaks RCT.

Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks.

Re: [PATCH net 1/2] net/sched: codel: bound the dropping loop per dequeue call

[Jamal Hadi Salim]

On Thu, Sep 3, 2026 at 8:35 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Sep 3, 2026 at 10:56 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> >
> > The CoDel control law schedules the next drop one interval/sqrt(count)
> > after the previous drop, using the configured interval
> > (codel_params.interval). For very small intervals the scheduled step
> > rounds down to zero, so the dropping loop in codel_dequeue() never
> > advances and drains the entire backlog under the qdisc lock in one
> > call - an unprivileged user can trigger a soft lockup this way.
> >
>
> ...
>
> >  static bool codel_should_drop(const struct sk_buff *skb,
> > @@ -156,6 +161,7 @@ static struct sk_buff *codel_dequeue(void *ctx,
> >         struct sk_buff *skb = dequeue_func(vars, ctx);
> >         codel_time_t now;
> >         bool drop;
> > +       unsigned int drops = 0;
>
> SGTM, note that this line breaks RCT.
>

Grrr.. it was in my brain to fix it, but my fingers didnt get the command ;->
If needed, I will send a v2.

> Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks for the review - struggled a bit with this one and wasnt 100%
sure it was the right approach.

cheers,
jamal

Re: [PATCH net 1/2] net/sched: codel: bound the dropping loop per dequeue call

[Jamal Hadi Salim]

On Fri, Sep 4, 2026 at 4:31 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On Thu, Sep 3, 2026 at 8:35 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Thu, Sep 3, 2026 at 10:56 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > >
> > > The CoDel control law schedules the next drop one interval/sqrt(count)
> > > after the previous drop, using the configured interval
> > > (codel_params.interval). For very small intervals the scheduled step
> > > rounds down to zero, so the dropping loop in codel_dequeue() never
> > > advances and drains the entire backlog under the qdisc lock in one
> > > call - an unprivileged user can trigger a soft lockup this way.
> > >
> >
> > ...
> >
> > >  static bool codel_should_drop(const struct sk_buff *skb,
> > > @@ -156,6 +161,7 @@ static struct sk_buff *codel_dequeue(void *ctx,
> > >         struct sk_buff *skb = dequeue_func(vars, ctx);
> > >         codel_time_t now;
> > >         bool drop;
> > > +       unsigned int drops = 0;
> >
> > SGTM, note that this line breaks RCT.
> >
>
> Grrr.. it was in my brain to fix it, but my fingers didnt get the command ;->
> If needed, I will send a v2.

Should i resend this? sashiko-nipa never reviewed and other one found no issues.

cheers,
jamal
> > Reviewed-by: Eric Dumazet <edumazet@google.com>
>
> Thanks for the review - struggled a bit with this one and wasnt 100%
> sure it was the right approach.
> cheers,
> jamal

Re: [PATCH net 1/2] net/sched: codel: bound the dropping loop per dequeue call

[Jakub Kicinski]

On Fri, 11 Sep 2026 19:12:28 -0400 Jamal Hadi Salim wrote:
> > > SGTM, note that this line breaks RCT.
> > >  
> >
> > Grrr.. it was in my brain to fix it, but my fingers didnt get the command ;->
> > If needed, I will send a v2.  
> 
> Should i resend this? sashiko-nipa never reviewed and other one found no issues.

Yes, let's get a repost, I'm losing track of all the outages..
This is getting a bit old for SUIE to track tho, so repost is better.