netdev - Re: [PATCH net] net: add net.core.qdisc_max

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iKNGtyFvWHAB_MnBgF4rKW10OhAGeptkoXWf97LtArGSA@mail.gmail.com>
Date: Fri, 9 Jan 2026 15:42:21 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Cong Wang <xiyou.wangcong@...il.com>
Cc: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, netdev@...r.kernel.org, 
	eric.dumazet@...il.com, Neal Cardwell <ncardwell@...gle.com>
Subject: Re: [PATCH net] net: add net.core.qdisc_max_burst

On Thu, Jan 8, 2026 at 12:25 AM Cong Wang <xiyou.wangcong@...il.com> wrote:
>
> On Wed, Jan 07, 2026 at 10:41:59AM +0000, Eric Dumazet wrote:
> > In blamed commit, I added a check against the temporary queue
> > built in __dev_xmit_skb(). Idea was to drop packets early,
> > before any spinlock was acquired.
> >
> > if (unlikely(defer_count > READ_ONCE(q->limit))) {
> >       kfree_skb_reason(skb, SKB_DROP_REASON_QDISC_DROP);
> >       return NET_XMIT_DROP;
> > }
> >
> > It turned out that HTB Qdisc has a zero q->limit.
> > HTB limits packets on a per-class basis.
> > Some of our tests became flaky.
>
> Hm, if q->limit is the problem here, why not introduce a new Qdisc
> option for this?

My first patch was using a plain macro.

I think the global switch makes more sense : I do not expect anyone
would need to
tune it, but better be safe than sorry.

A per-qdisc parameter would need iproute2 changes

 A per qdisc attribute would force all iproute2 users to update their scripts
in case of an emergency.

>
> >
> > Add a new sysctl : net.core.qdisc_max_burst to control
> > how many packets can be stored in the temporary lockless queue.
>
> This becomes global instead of per-Qdisc. If this is intended, you might
> want to document it explicitly in the documentation.

My patch did update Documentation/admin-guide/sysctl/net.rst

Are you thinking of something else ?

An alternative would be to set q->limit to 1000 in HTB only.
And eventually add an TCA_HTB_LIMIT new attribute to tune it.

diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index b5e40c51655a731e7c1879e4eb4932b9c1687ea5..7f2402dfb0bfb4e8ee8a07f6d18eab0e8555c6d0
100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1059,6 +1059,7 @@ static int htb_init(struct Qdisc *sch, struct nlattr *opt,
        bool offload;
        int err;

+       sch->limit = 1000;
        qdisc_watchdog_init(&q->watchdog, sch);
        INIT_WORK(&q->work, htb_work_func);