Netdev List
 help / color / mirror / Atom feed
* [netdev] htb_qlen_notify warning triggered after qdisc_tree_reduce_backlog change
@ 2025-08-06  8:29 He, Guocai (CN)
  2025-08-08 20:29 ` Jakub Kicinski
  0 siblings, 1 reply; 4+ messages in thread
From: He, Guocai (CN) @ 2025-08-06  8:29 UTC (permalink / raw)
  To: Lion Ackermann, netdev@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org, ovs-discuss@openvswitch.org

Dear netdev maintainers and community,

I have encountered a kernel warning in the HTB scheduler (`htb_qlen_notify` at `net/sched/sch_htb.c:609`) when using Open vSwitch (OVS) with a linux-htb QoS configuration. The issue appears related to a recent change in `qdisc_tree_reduce_backlog`.

### Environment
- Kernel version: 5.15.189-rt76-yocto-preempt-rt
- Open vSwitch version: 2.17.9
- Configuration:
  - Created a veth pair (`veth0` and `veth1`), added `veth0` to an OVS bridge (`br-test`).
  - Applied QoS with linux-htb type, total max-rate=2Mbps, two queues (queue 0: max-rate=1Mbps, queue 1: max-rate=0.5Mbps).
  - Command sequence:
    ```bash
    ip link add veth0 type veth peer name veth1
    ip link set veth0 up
    ip link set veth1 up
    ovs-vsctl add-br br-test
    ovs-vsctl add-port br-test veth0
    ip addr add 10.0.0.1/24 dev veth1
    ovs-vsctl set port veth0 qos=@newqos \
    -- --id=@newqos create qos type=linux-htb other-config:max-rate=2000000 queues=0=@q0,1=@q1 \
    -- --id=@q0 create queue other-config:min-rate=800000 other-config:max-rate=1000000 \
    -- --id=@q1 create queue other-config:min-rate=400000 other-config:max-rate=500000
    
    
### Issue
After applying the QoS configuration, the following warning appears in dmesg:
[73591.168117] WARNING: CPU: 6 PID: 61296 at net/sched/sch_htb.c:609 htb_qlen_notify+0x3a/0x40 [sch_htb]

Suspected Cause
The warning seems related to a change in qdisc_tree_reduce_backlog (/net/sched/sch_api.c)
the commit is  e269f29e9395527bc00c213c6b15da04ebb35070 (5.15)

when I revert this commit, the warning disappeared.

I dont know if it is a known issue or have fixing ?

git show e269f29e9395527bc00c213c6b15da04ebb35070
commit e269f29e9395527bc00c213c6b15da04ebb35070
Author: Lion Ackermann <nnamrec@gmail.com>
Date:   Mon Jun 30 15:27:30 2025 +0200
    net/sched: Always pass notifications when child class becomes empty
    [ Upstream commit 103406b38c600fec1fe375a77b27d87e314aea09 ]
    Certain classful qdiscs may invoke their classes' dequeue handler on an
    enqueue operation. This may unexpectedly empty the child qdisc and thus
    make an in-flight class passive via qlen_notify(). Most qdiscs do not
    expect such behaviour at this point in time and may re-activate the
    class eventually anyways which will lead to a use-after-free.
..............
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index d9ce273ba43d..222921b4751f 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -768,15 +768,12 @@ static u32 qdisc_alloc_handle(struct net_device *dev)

 void qdisc_tree_reduce_backlog(struct Qdisc *sch, int n, int len)
 {
-       bool qdisc_is_offloaded = sch->flags & TCQ_F_OFFLOADED;
        const struct Qdisc_class_ops *cops;
        unsigned long cl;
        u32 parentid;
        bool notify;
        int drops;

-       if (n == 0 && len == 0)
-               return;
        drops = max_t(int, n, 0);
        rcu_read_lock();
        while ((parentid = sch->parent)) {
@@ -785,17 +782,8 @@ void qdisc_tree_reduce_backlog(struct Qdisc *sch, int n, int len)

                if (sch->flags & TCQ_F_NOPARENT)
                        break;
-               /* Notify parent qdisc only if child qdisc becomes empty.
-                *
-                * If child was empty even before update then backlog
-                * counter is screwed and we skip notification because
-                * parent class is already passive.
-                *
-                * If the original child was offloaded then it is allowed
-                * to be seem as empty, so the parent is notified anyway.
-                */
-               notify = !sch->q.qlen && !WARN_ON_ONCE(!n &&
-                                                      !qdisc_is_offloaded);
+               /* Notify parent qdisc only if child qdisc becomes empty. */
+               notify = !sch->q.qlen;
                /* TODO: perform the search on a per txq basis */
                sch = qdisc_lookup(qdisc_dev(sch), TC_H_MAJ(parentid));
                if (sch == NULL) {
@@ -804,6 +792,9 @@ void qdisc_tree_reduce_backlog(struct Qdisc *sch, int n, int len)
                }
                cops = sch->ops->cl_ops;
                if (notify && cops->qlen_notify) {
+                       /* Note that qlen_notify must be idempotent as it may get called
+                        * multiple times.
+                        */
                        cl = cops->find(sch, parentid);
                        cops->qlen_notify(sch, cl);
                }


Thanks
Guocai

^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-08-11  1:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-06  8:29 [netdev] htb_qlen_notify warning triggered after qdisc_tree_reduce_backlog change He, Guocai (CN)
2025-08-08 20:29 ` Jakub Kicinski
2025-08-08 20:40   ` Ilya Maximets
2025-08-11  1:29   ` He, Guocai (CN)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox