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