Thread (4 messages) 4 messages, 2 authors, 2026-06-18

Re: [PATCH] net/sched: dualpi2: fix GSO backlog accounting

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2026-06-17 10:24:04
Also in: stable

On Tue, Jun 16, 2026 at 6:03 PM Xingquan Liu [off-list ref] wrote:
quoted hunk ↗ jump to hunk
When DualPI2 splits a GSO skb into N segments, it propagates N
additional packets to its parent before returning NET_XMIT_SUCCESS.
The parent then accounts for the original skb once more, leaving its
qlen one larger than the number of packets actually queued.

With QFQ as the parent, after all real packets are dequeued, QFQ still
has a non-zero qlen while its in-service aggregate has no active
classes. qfq_choose_next_agg() returns NULL and qfq_dequeue() passes
the result to qfq_peek_skb(), causing a NULL pointer dereference.

Count only successfully queued segments and propagate the difference
between the original skb and those segments. Return success whenever
at least one segment was queued.

Fixes: 8f9516daedd6 ("sched: Add enqueue/dequeue of dualpi2 qdisc")
Cc: stable@vger.kernel.org
Signed-off-by: Xingquan Liu <redacted>
---
 net/sched/sch_dualpi2.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/net/sched/sch_dualpi2.c b/net/sched/sch_dualpi2.c
index dfec3c99eb45..37d6a8960310 100644
--- a/net/sched/sch_dualpi2.c
+++ b/net/sched/sch_dualpi2.c
@@ -461,7 +461,7 @@ static int dualpi2_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
                if (IS_ERR_OR_NULL(nskb))
                        return qdisc_drop(skb, sch, to_free);

-               cnt = 1;
+               cnt = 0;
                byte_len = 0;
                orig_len = qdisc_pkt_len(skb);
                skb_list_walk_safe(nskb, nskb, next) {
@@ -488,16 +488,15 @@ static int dualpi2_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
                                byte_len += nskb->len;
                        }
                }
-               if (cnt > 1) {
+               if (cnt > 0) {
                        /* The caller will add the original skb stats to its
                         * backlog, compensate this if any nskb is enqueued.
                         */
-                       --cnt;
-                       byte_len -= orig_len;
+                       qdisc_tree_reduce_backlog(sch, 1 - cnt,
+                                                 orig_len - byte_len);
                }
-               qdisc_tree_reduce_backlog(sch, -cnt, -byte_len);
                consume_skb(skb);
-               return err;
+               return cnt > 0 ? NET_XMIT_SUCCESS : err;
        }
This looks like a behavior change?
Ex: If the last segment failed you will return XMIT_SUCCESS whereas
before it could be with __NET_XMIT_BYPASS, NET_XMIT_CN,  etc.
I am not sure what the best answer is and maybe it doesnt matter. Did
you look at what other qdiscs do? I dont have time right now but will
later - or you can before i get to it.
Also, you didnt add the owner of this qdisc on your to:  - maybe he
has some thoughts..

cheers,
jamal

        return dualpi2_enqueue_skb(skb, sch, to_free);
 }

base-commit: fbc6a80cb5d3fd4ac4b56e8c9d791dd17be890c4
--
Xingquan Liu
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help