[PATCH net v2 1/2] net/sched: Make cake_enqueue return NET_XMIT_CN when past buffer_limit

[PATCH net v2 1/2] net/sched: Make cake_enqueue return NET_XMIT_CN when past buffer_limit

[William Liu]

The following setup can trigger a WARNING in htb_activate due to
the condition: !cl->leaf.q->q.qlen

tc qdisc del dev lo root
tc qdisc add dev lo root handle 1: htb default 1
tc class add dev lo parent 1: classid 1:1 \
       htb rate 64bit
tc qdisc add dev lo parent 1:1 handle f: \
       cake memlimit 1b
ping -I lo -f -c1 -s64 -W0.001 127.0.0.1

This is because the low memlimit leads to a low buffer_limit, which
causes packet dropping. However, cake_enqueue still returns
NET_XMIT_SUCCESS, causing htb_enqueue to call htb_activate with an
empty child qdisc. We should return NET_XMIT_CN when packets are
dropped from the same tin and flow.

I do not believe return value of NET_XMIT_CN is necessary for packet
drops in the case of ack filtering, as that is meant to optimize
performance, not to signal congestion.

Fixes: 046f6fd5daef ("sched: Add Common Applications Kept Enhanced (cake) qdisc")
Signed-off-by: William Liu <will@willsroot.io>
Reviewed-by: Savino Dicanosa <savy@syst3mfailure.io>
---
v1 -> v2: only return NET_XMIT_CN when dropping from the same tin and flow
---
 net/sched/sch_cake.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index dbcfb948c867..32bacfc314c2 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -1750,7 +1750,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	ktime_t now = ktime_get();
 	struct cake_tin_data *b;
 	struct cake_flow *flow;
-	u32 idx;
+	u32 idx, tin;
 
 	/* choose flow to insert into */
 	idx = cake_classify(sch, &b, skb, q->flow_mode, &ret);
@@ -1760,6 +1760,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		__qdisc_drop(skb, to_free);
 		return ret;
 	}
+	tin = (u32)(b - q->tins);
 	idx--;
 	flow = &b->flows[idx];
 
@@ -1927,13 +1928,22 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		q->buffer_max_used = q->buffer_used;
 
 	if (q->buffer_used > q->buffer_limit) {
+		bool same_flow = false;
 		u32 dropped = 0;
+		u32 drop_id;
 
 		while (q->buffer_used > q->buffer_limit) {
 			dropped++;
-			cake_drop(sch, to_free);
+			drop_id = cake_drop(sch, to_free);
+
+			if ((drop_id >> 16) == tin &&
+			    (drop_id & 0xFFFF) == idx)
+				same_flow = true;
 		}
 		b->drop_overlimit += dropped;
+
+		if (same_flow)
+			return NET_XMIT_CN;
 	}
 	return NET_XMIT_SUCCESS;
 }
-- 
2.43.0

[PATCH net v2 2/2] net/sched: Remove unnecessary WARNING condition for empty child qdisc in htb_activate

[William Liu]

The WARN_ON trigger based on !cl->leaf.q->q.qlen is unnecessary in
htb_activate. htb_dequeue_tree already accounts for that scenario.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: William Liu <will@willsroot.io>
Reviewed-by: Savino Dicanosa <savy@syst3mfailure.io>
---
 net/sched/sch_htb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index c968ea763774..b5e40c51655a 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -592,7 +592,7 @@ htb_change_class_mode(struct htb_sched *q, struct htb_class *cl, s64 *diff)
  */
 static inline void htb_activate(struct htb_sched *q, struct htb_class *cl)
 {
-	WARN_ON(cl->level || !cl->leaf.q || !cl->leaf.q->q.qlen);
+	WARN_ON(cl->level || !cl->leaf.q);
 
 	if (!cl->prio_activity) {
 		cl->prio_activity = 1 << cl->prio;
-- 
2.43.0

Re: [PATCH net v2 1/2] net/sched: Make cake_enqueue return NET_XMIT_CN when past buffer_limit

[Toke Høiland-Jørgensen]

William Liu <will@willsroot.io> writes:

> The following setup can trigger a WARNING in htb_activate due to
> the condition: !cl->leaf.q->q.qlen
>
> tc qdisc del dev lo root
> tc qdisc add dev lo root handle 1: htb default 1
> tc class add dev lo parent 1: classid 1:1 \
>        htb rate 64bit
> tc qdisc add dev lo parent 1:1 handle f: \
>        cake memlimit 1b
> ping -I lo -f -c1 -s64 -W0.001 127.0.0.1
>
> This is because the low memlimit leads to a low buffer_limit, which
> causes packet dropping. However, cake_enqueue still returns
> NET_XMIT_SUCCESS, causing htb_enqueue to call htb_activate with an
> empty child qdisc. We should return NET_XMIT_CN when packets are
> dropped from the same tin and flow.
>
> I do not believe return value of NET_XMIT_CN is necessary for packet
> drops in the case of ack filtering, as that is meant to optimize
> performance, not to signal congestion.
>
> Fixes: 046f6fd5daef ("sched: Add Common Applications Kept Enhanced (cake) qdisc")
> Signed-off-by: William Liu <will@willsroot.io>
> Reviewed-by: Savino Dicanosa <savy@syst3mfailure.io>

Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>

Re: [PATCH net v2 1/2] net/sched: Make cake_enqueue return NET_XMIT_CN when past buffer_limit

[Jamal Hadi Salim]

On Tue, Aug 19, 2025 at 4:51 AM Toke Høiland-Jørgensen <toke@toke.dk> wrote:
>
> William Liu <will@willsroot.io> writes:
>
> > The following setup can trigger a WARNING in htb_activate due to
> > the condition: !cl->leaf.q->q.qlen
> >
> > tc qdisc del dev lo root
> > tc qdisc add dev lo root handle 1: htb default 1
> > tc class add dev lo parent 1: classid 1:1 \
> >        htb rate 64bit
> > tc qdisc add dev lo parent 1:1 handle f: \
> >        cake memlimit 1b
> > ping -I lo -f -c1 -s64 -W0.001 127.0.0.1
> >
> > This is because the low memlimit leads to a low buffer_limit, which
> > causes packet dropping. However, cake_enqueue still returns
> > NET_XMIT_SUCCESS, causing htb_enqueue to call htb_activate with an
> > empty child qdisc. We should return NET_XMIT_CN when packets are
> > dropped from the same tin and flow.
> >
> > I do not believe return value of NET_XMIT_CN is necessary for packet
> > drops in the case of ack filtering, as that is meant to optimize
> > performance, not to signal congestion.
> >
> > Fixes: 046f6fd5daef ("sched: Add Common Applications Kept Enhanced (cake) qdisc")
> > Signed-off-by: William Liu <will@willsroot.io>
> > Reviewed-by: Savino Dicanosa <savy@syst3mfailure.io>
>
> Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>

Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal
>

Re: [PATCH net v2 1/2] net/sched: Make cake_enqueue return NET_XMIT_CN when past buffer_limit

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 19 Aug 2025 03:36:28 +0000 you wrote:
> The following setup can trigger a WARNING in htb_activate due to
> the condition: !cl->leaf.q->q.qlen
> 
> tc qdisc del dev lo root
> tc qdisc add dev lo root handle 1: htb default 1
> tc class add dev lo parent 1: classid 1:1 \
>        htb rate 64bit
> tc qdisc add dev lo parent 1:1 handle f: \
>        cake memlimit 1b
> ping -I lo -f -c1 -s64 -W0.001 127.0.0.1
> 
> [...]

Here is the summary with links:
  - [net,v2,1/2] net/sched: Make cake_enqueue return NET_XMIT_CN when past buffer_limit
    https://git.kernel.org/netdev/net/c/15de71d06a40
  - [net,v2,2/2] net/sched: Remove unnecessary WARNING condition for empty child qdisc in htb_activate
    https://git.kernel.org/netdev/net/c/2c2192e5f9c7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html