netdev - Re: [PATCH iproute2-next v3 2/2] tc: implement support for terse dump

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, 22 Oct 2020 10:05:30 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Vlad Buslov <vladbu@...dia.com>
Cc:     Vlad Buslov <vlad@...lov.dev>, dsahern@...il.com,
        stephen@...workplumber.org, netdev@...r.kernel.org,
        davem@...emloft.net, xiyou.wangcong@...il.com, jiri@...nulli.us,
        ivecera@...hat.com, Vlad Buslov <vladbu@...lanox.com>
Subject: Re: [PATCH iproute2-next v3 2/2] tc: implement support for terse dump

On 2020-10-21 4:19 a.m., Vlad Buslov wrote:
> 
> On Tue 20 Oct 2020 at 15:29, Jamal Hadi Salim <jhs@...atatu.com> wrote:
>> On 2020-10-19 11:18 a.m., Vlad Buslov wrote:
>> My worry is you have a very specific use case for your hardware or
>> maybe it is ovs - where counters are uniquely tied to filters and
>> there is no sharing. And possibly maybe only one counter can be tied
>> to a filter (was not sure if you could handle more than one action
>> in the terse from looking at the code).
> 
> OVS uses cookie to uniquely identify the flow and it does support
> multiple actions per flow.

ok, so they use it like a flowid/classid to identify the flow.
In our use case the cookie stores all kinds of other state that
the controller can avoid to lookup after a query.
index otoh is universal i.e two different users can intepret it
per action tying it specific stats.
IOW: I dont think it replaces the index.
Do they set cookies on all actions in a flow?

>> Our assumptions so far had no such constraints.
>> Maybe a new TERSE_OPTIONS TLV, and then add an extra flag
>> to indicate interest in the tlv? Peharps store the stats in it as well.
> 
> Maybe, but wouldn't that require making it a new dump mode? Current
> terse dump is already in released kernel and this seems like a
> backward-incompatible change.
> 

I meant you would set a new flag(in addition to TERSE) in a request to
the kernel to ask for the index to be made available on the response.
Response comes back in a TLV with just index in it for now.

>>
>>> This wouldn't be much of a terse dump anymore. What prevents user that
>>> needs all action info from calling regular dump? It is not like terse
>>> dump substitutes it or somehow makes it harder to use.
>>
>> Both scaling and correctness are important. You have the cookie
>> in the terse dump, thats a lot of data.
> 
> Cookie only consumes space in resulting netlink packet if used set the
> cookie during action init. Otherwise, the cookie attribute is omitted.

True, but: I am wondering why it is even considered in when terseness
was a requirement (and index was left out).

>> In our case we totally bypass filters to reduce the amount of data
>> crossing to user space (tc action ls). Theres still a lot of data
>> crossing which we could trim with a terse dump. All we are interested
>> in are stats. Another alternative is perhaps to introduce the index for
>> the direct dump.
> 
> What is the direct dump?

tc action ls ...
Like i said in our use cases to get the stats we just dumped the actions
we wanted. It is a lot less data than having the filter + actions.
And with your idea of terseness we can trim down further how much
data by removing all the action attributes coming back if we set TERSE
flag in such a request. But the index has to be there to make sense.

cheers,
jamal