> -----Original Message----- > From: Paolo Abeni <[email protected]> > Sent: Thursday, April 16, 2026 3:26 PM > To: Chia-Yu Chang (Nokia) <[email protected]>; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; Koen De Schepper (Nokia) > <[email protected]>; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected] > Subject: Re: [PATCH v1 net 1/1] net/sched: sch_dualpi2: fix limit/memlimit > enforcement when dequeueing L-queue > > > CAUTION: This is an external email. Please be very careful when clicking > links or opening attachments. See the URL nok.it/ext for additional > information. > > > > On 4/13/26 6:37 PM, [email protected] wrote: > > From: Chia-Yu Chang <[email protected]> > > > > Fix dualpi2_change() to correctly enforce updated limit and memlimit > > values after a configuration change of the dualpi2 qdisc. > > > > Before this patch, dualpi2_change() always attempted to dequeue > > packets via the root qdisc (C-queue) when reducing backlog or memory > > usage, and unconditionally assumed that a valid skb will be returned. > > When traffic classification results in packets being queued in the > > L-queue while the C-queue is empty, this leads to a NULL skb > > dereference during limit or memlimit enforcement. > > > > This is fixed by first dequeuing from the C-queue path if it is non-empty. > > Once the C-queue is empty, packets are dequeued directly from the L-queue. > > Return values from qdisc_dequeue_internal() are checked for both > > queues. When dequeuing from the L-queue, the parent qdisc qlen and > > backlog counters are updated explicitly to keep overall qdisc statistics > > consistent. > > > > Fixes: 320d031ad6e4 ("sched: Struct definition and parsing of dualpi2 > > qdisc") > > Signed-off-by: Chia-Yu Chang <[email protected]> > > --- > > net/sched/sch_dualpi2.c | 24 +++++++++++++++++++----- > > 1 file changed, 19 insertions(+), 5 deletions(-) > > > > diff --git a/net/sched/sch_dualpi2.c b/net/sched/sch_dualpi2.c index > > 6d7e6389758d..56d4422970b6 100644 > > --- a/net/sched/sch_dualpi2.c > > +++ b/net/sched/sch_dualpi2.c > > @@ -872,11 +872,25 @@ static int dualpi2_change(struct Qdisc *sch, struct > > nlattr *opt, > > old_backlog = sch->qstats.backlog; > > while (qdisc_qlen(sch) > sch->limit || > > q->memory_used > q->memory_limit) { > > - struct sk_buff *skb = qdisc_dequeue_internal(sch, true); > > - > > - q->memory_used -= skb->truesize; > > - qdisc_qstats_backlog_dec(sch, skb); > > - rtnl_qdisc_drop(skb, sch); > > + int c_len = qdisc_qlen(sch) - qdisc_qlen(q->l_queue); > > + struct sk_buff *skb = NULL; > > + > > + if (c_len) { > > + skb = qdisc_dequeue_internal(sch, true); > > + if (!skb) > > + break; > > + q->memory_used -= skb->truesize; > > + rtnl_qdisc_drop(skb, sch); > > + } else if (qdisc_qlen(q->l_queue)) { > > + skb = qdisc_dequeue_internal(q->l_queue, true); > > + if (!skb) > > + break; > > + q->memory_used -= skb->truesize; > > + rtnl_qdisc_drop(skb, q->l_queue); > > + /* Keep the overall qdisc stats consistent */ > > + --sch->q.qlen; > > + qdisc_qstats_backlog_dec(sch, skb); > > Sashiko says: > --- > The drop counter is incremented for the L-queue via rtnl_qdisc_drop(), but it > appears the drop counter for the parent qdisc (sch) is not updated. > Will this cause user-facing statistics for the overall dualpi2 qdisc to > underreport drops? > ---
Hi Paolo, You are right, this is my miss. I will add "qdisc_qstats_drop(sch)" for the L-queue dropping case. Thanks! Chia-Yu
