netdev - Re: [PATCH net-next 2/2] net: sched: Lockless Token Bucket (LTB) Qdisc

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]

Date:   Thu, 9 Jul 2020 23:09:49 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     "YU, Xiangning" <xiangning.yu@...baba-inc.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 2/2] net: sched: Lockless Token Bucket (LTB) Qdisc

On Thu, Jul 9, 2020 at 10:49 PM YU, Xiangning
<xiangning.yu@...baba-inc.com> wrote:
>
> Well, we do ask packets from a flow to be classified to a single class, not multiple ones. It doesn't have to be socket priority, it could be five tuple hash, or even container classid.

I don't see how it is so in your code, without skb priority your code
simply falls back to default class:

+       /* Allow to select a class by setting skb->priority */
+       if (likely(skb->priority != 0)) {
+               cl = ltb_find_class(sch, skb->priority);
+               if (cl)
+                       return cl;
+       }
+       return rcu_dereference_bh(ltb->default_cls);

Mind to be more specific here?

BTW, your qdisc does not even support TC filters, does it?
At least I don't see that tcf_classify() is called.


>
> I think it's ok to have this requirement, even if we use htb, I would suggest the same. Why do you think this is a problem?

Because HTB does not have a per-cpu queue for each class,
yours does, cl->aggr_queues[cpu], if your point here is why we
don't blame HTB.

Thanks.