netdev - Re: [BUG] net/sched : qlen can not really be per cpu ?

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: <e312625a-7946-42a4-88f0-95536768d5c8@gmail.com>
Date:   Wed, 27 Feb 2019 08:59:15 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     John Fastabend <john.fastabend@...il.com>,
        Networking <netdev@...r.kernel.org>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Jiri Pirko <jiri@...nulli.us>
Subject: Re: [BUG] net/sched : qlen can not really be per cpu ?



On 02/26/2019 04:56 PM, Eric Dumazet wrote:
> 
> 
> On 02/26/2019 03:51 PM, Cong Wang wrote:
>> On Tue, Feb 26, 2019 at 3:19 PM Eric Dumazet <eric.dumazet@...il.com> wrote:
>>>
>>>
>>>
>>> On 02/25/2019 10:42 PM, Eric Dumazet wrote:
>>>> HTB + pfifo_fast as a leaf qdisc hits badly the following warning in htb_activate() :
>>>>
>>>> WARN_ON(cl->level || !cl->leaf.q || !cl->leaf.q->q.qlen);
>>>>
>>>> This is because pfifo_fast does not update sch->q.qlen, but per cpu counters.
>>>> So cl->leaf.q->q.qlen is zero.
>>>>
>>>> HFSC, CBQ, DRR, QFQ  have the same problem.
>>>>
>>>> Any ideas how we can fix this ?
>>>
>>> What about something simple for stable ?
>>> ( I yet have to boot/test this )
>>
>> Is merely updating qlen sufficient for fixing it?
>>
>> I thought it is because of the lack of qdisc_tree_reduce_backlog()
>> in pfifo_fast.
> 
> It does not seem to be the qdisc_tree_reduce_backlog() thing.
> 
> HTB, HFSC, CBQ, DRR, QFQ only peek at their children 'qlen' to decide if there
> is at least one packet in them.
> 
> The backlog is only reported for dumps, but the actual backlog value is not used in data path.
> 
> 

Another way to fix this would be to have a shadow version of pfifo_fast, which
basically would be the old version of it, that would be automatically selected
when used as a child of another qdisc (except mq/mqprio of course)

This seems not a stable candidate though.