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:   Wed, 9 Aug 2017 08:40:30 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Cong Wang <xiyou.wangcong@...il.com>,
        John Fastabend <john.fastabend@...il.com>
Cc:     Jiri Pirko <jiri@...nulli.us>, David Miller <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        mlxsw@...lanox.com
Subject: Re: Qdisc->u32_node - licence to kill

On 17-08-07 07:21 PM, Cong Wang wrote:
> On Mon, Aug 7, 2017 at 12:54 PM, John Fastabend
> <john.fastabend@...il.com> wrote:
>> On 08/07/2017 12:06 PM, Jiri Pirko wrote:
>>> Mon, Aug 07, 2017 at 07:47:14PM CEST, john.fastabend@...il.com wrote:
>>>> On 08/07/2017 09:41 AM, Jiri Pirko wrote:
>>>>> Hi Jamal/Cong/David/all.
>>>>>
=
>>>
>>> Not correct. prio/pref is one level up priority, independent on specific
>>> cls implementation. You can have cls_u32 instance on prio 10 and
>>> cls_flower instance on prio 20. Both work.
>>
>> ah right, lets make sure I got this right then (its been awhile since I've
>> read this code). So the tcf_ctl_tfilter hook walks classifiers, inserting the
>> classifier by prio. Then tcf_classify walks the list of classifiers looking
>> for any matches, specifically any return codes it recognizes or a return code
>> greater than zero. u32 though has this link notion that allows users to jump
>> to other u32 classifiers that are in this list, because it has a global hash
>> table list. So the per prio classifier isolation is not true in u32 case.
> 
> u32 filter supports multiple hash tables within a qdisc, struct
> tc_u_common is supposed to link them together. This has to be
> per qdisc because all of these hash tables belong to one qdisc
> and their ID's are unique within the qdisc.
> 

I think historically this used to sit within the u32 code as a
static linked list; i cant recall why it got attached to the
qdisc.
Indeed, the idea is that hash tables can be added independently
without linking and then linked afterwards. They have to be held
somewhere in transient. And they have priorities (at least the
prios are used in the dump)

> I dislike it too, and I actually tried to improve it in the past,
> unfortunately didn't make any real progress. I think we can
> definitely make it less ugly, but I don't think we can totally
> get rid of it because of the design of u32.
> 
> Similar for tp->data.
> 

tp->q maybe harder to deal with. I agree with getting rid of
this dependency. Could this info be stored in the block instead?

cheers,
jamal

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ