netdev - Re: [PATCH net-next] net/sched: cls_flower: Add user specified data

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: <586B29A4.2060408@gmail.com>
Date:   Mon, 2 Jan 2017 20:33:40 -0800
From:   John Fastabend <john.fastabend@...il.com>
To:     Jamal Hadi Salim <jhs@...atatu.com>,
        Paul Blakey <paulb@...lanox.com>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Cc:     Jiri Pirko <jiri@...lanox.com>,
        Hadar Hen Zion <hadarh@...lanox.com>,
        Or Gerlitz <ogerlitz@...lanox.com>,
        Roi Dayan <roid@...lanox.com>, Roman Mashak <mrv@...atatu.com>,
        Simon Horman <simon.horman@...ronome.com>
Subject: Re: [PATCH net-next] net/sched: cls_flower: Add user specified data

On 17-01-02 05:22 PM, Jamal Hadi Salim wrote:
> On 17-01-02 05:58 PM, John Fastabend wrote:
>> On 17-01-02 02:21 PM, Jamal Hadi Salim wrote:
> 
> 
>> Well having the length value avoids ending up with cookie1, cookie2, ...
>> values as folks push more and more data into the cookie.
>>
> 
> Unless there is good reason I dont see why it shouldnt be a fixed length
> value. u64/128 should be plenty.
> 
>> I don't see any use in the kernel interpreting it. It has no use
>> for it as far as I can see. It doesn't appear to be metadata which
>> we use skb->mark for at the moment.
>>
> 
> Like all cookie semantics it is for storing state. The receiver (kernel)
> is not just store it and not intepret it. The user when reading it back
> simplifies what they have to do for their processing.
> 
>>
>> The tuple <ifindex:qdisc:prio:handle> really should be unique why
>> not use this for system wide mappings?
>>
> 
> I think on a single machine should be enough, however:
> typically the user wants to define the value in a manner that
> in a distributed system it is unique. It would be trickier to
> do so with well defined values such as above.
> 

Just extend the tuple <hostname:ifindex:qdisc:prio:handle> that
should be unique in the domain of hostname's, or use some other domain
wide machine identifier.

> 
>> The only thing I can think to do with this that I can't do with
>> the above tuple and a simple userspace lookup is stick hardware specific
>> "hints" in the cookie for the firmware to consume. Which would be
>> very helpful for what its worth.
>>
> 
> Ok, very different from our use case with actions.
> We just use those values to map to stats info without needing to
> know what flow or action (graph) it is associated with.
> 

Sure.

>> Its a bit strange to push it as an action when its not really an
>> action in the traditional datapath.
>>
> 
> The action is part of a graph pointed to by a filter.

Although actions can be shared so the cookie can be shared across
filters. Maybe its useful but it doesn't uniquely identify a filter
in the shared case but the user would have to specify that case
so maybe its not important.

> 
>> I suspect the OVS usage is a simple 1:1 lookup from OVS id to TC id to
>> avoid a userspace map lookup.
> 
> Not that i care about OVS but it sounds like a good use case (even for
> tc), no?

I'm not opposed to it. Just pushing on the use case a bit to understand.

> 
> cheers,
> jamal