netdev - Re: [jkirsher/next-queue PATCH v4 0/6] tc-flower based cloud filters in i40e

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: <20171011205830.GD9297@nanopsycho>
Date:   Wed, 11 Oct 2017 22:58:30 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     David Miller <davem@...emloft.net>
Cc:     alexander.duyck@...il.com, amritha.nambiar@...el.com,
        intel-wired-lan@...ts.osuosl.org, jeffrey.t.kirsher@...el.com,
        alexander.h.duyck@...el.com, netdev@...r.kernel.org,
        jhs@...atatu.com, xiyou.wangcong@...il.com
Subject: Re: [jkirsher/next-queue PATCH v4 0/6] tc-flower based cloud filters
 in i40e

Wed, Oct 11, 2017 at 10:46:52PM CEST, davem@...emloft.net wrote:
>From: Jiri Pirko <jiri@...nulli.us>
>Date: Wed, 11 Oct 2017 22:38:32 +0200
>
>> Wed, Oct 11, 2017 at 07:46:27PM CEST, alexander.duyck@...il.com wrote:
>>>On Wed, Oct 11, 2017 at 5:56 AM, Jiri Pirko <jiri@...nulli.us> wrote:
>>>> Wed, Oct 11, 2017 at 02:24:12AM CEST, amritha.nambiar@...el.com wrote:
>>>>>This patch series enables configuring cloud filters in i40e
>>>>>using the tc-flower classifier. The classification function
>>>>>of the filter is to match a packet to a class. cls_flower is
>>>>>extended to offload classid to hardware. The offloaded classid
>>>>>is used direct matched packets to a traffic class on the device.
>>>>>The approach here is similar to the tc 'prio' qdisc which uses
>>>>>the classid for band selection. The ingress qdisc is called ffff:0,
>>>>>so traffic classes are ffff:1 to ffff:8 (i40e has max of 8 TCs).
>>>>
>>>>
>>>> NACK. This clearly looks like abuse of classid to something
>>>> else. Classid is here to identify qdisc instance. However, you use it
>>>> for hw tclass identification. This is mixing of apples and oranges.
>>>>
>>>> Why?
>>>>
>>>> Please don't try to abuse things! This is not nice.
>>>
>>>This isn't an abuse. This is reproducing in hardware what is already
>>>the behavior for software. Isn't that how offloads are supposed to
>>>work?
>> 
>> What is meaning of classid in HW? Classid is SW only identification of
>> qdisc instances. No relation to HW instances = abuse.
>
>Jiri I really don't see what the problem is.
>
>As long as the driver does the right thing when changes are made to the
>qdisc, it doesn't really matter what "key" they use to refer to it.
>
>It could have just as easily used the qdisc pointer and then internally
>use some IDR allocated ID to refer to it in the driver and hardware.
>
>But that's such a waste, we have a unique handle already so why can't
>the driver just use that?

Well if I see classid, I expect it should refer to qdisc instance. So
far, this has been always a case. But for some drivers, this would mean
something totally different and unrelated. So what should I think?
What's next? Classid could be abused to identify something else. I don't
understand why.

classid in kernel and tclass in hw are 2 completely unrelated things.
Why they should share the same userspace api? What am I missing that
indicates this is not an abuse?

There should be clean and well-defined userspace api:
1) classid to identify qdisc instances
2) something else to identify HW tclasses