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

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

[Xingquan Liu]

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 <b1n@b1n.io>
---
 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;
 	}
 	return dualpi2_enqueue_skb(skb, sch, to_free);
 }

base-commit: fbc6a80cb5d3fd4ac4b56e8c9d791dd17be890c4
--
Xingquan Liu

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

[Jamal Hadi Salim]

On Tue, Jun 16, 2026 at 6:03 PM Xingquan Liu <b1n@b1n.io> wrote:
>
> 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 <b1n@b1n.io>
> ---
>  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
>

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

[Jamal Hadi Salim]

On Wed, Jun 17, 2026 at 6:23 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On Tue, Jun 16, 2026 at 6:03 PM Xingquan Liu <b1n@b1n.io> wrote:
> >
> > 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 <b1n@b1n.io>
> > ---
> >  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..
>

After looking at what other qdiscs do, your patch is fine. But please
fixup the commit to something like:

---
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.

Follow the same pattern used by tbf_segment() and taprio: count only
successfully queued segments, propagate the difference between the
original skb and those segments, and return NET_XMIT_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 <b1n@b1n.io>
-----

Do you know how to create a tdc test that will recreate this? If not
either Victor or myself can help you create one.

cheers,
jamal

> cheers,
> jamal
>
>
> >         return dualpi2_enqueue_skb(skb, sch, to_free);
> >  }
> >
> > base-commit: fbc6a80cb5d3fd4ac4b56e8c9d791dd17be890c4
> > --
> > Xingquan Liu
> >

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

[Xingquan Liu]

[-- Attachment #1: Type: text/plain, Size: 245 bytes --]

On Wed, Jun 17, 2026 at 10:23:42AM -0400, Jamal Hadi Salim wrote:
> Do you know how to create a tdc test that will recreate this? If not
> either Victor or myself can help you create one.

Okay, I will try to create a tdc test.

--
Xingquan Liu

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 403 bytes --]