From: John Fastabend <john.fastabend@gmail.com>
To: Jamal Hadi Salim <jhs@mojatatu.com>,
Paul Blakey <paulb@mellanox.com>,
"David S. Miller" <davem@davemloft.net>,
netdev@vger.kernel.org
Cc: Jiri Pirko <jiri@mellanox.com>,
Hadar Hen Zion <hadarh@mellanox.com>,
Or Gerlitz <ogerlitz@mellanox.com>, Roi Dayan <roid@mellanox.com>,
Roman Mashak <mrv@mojatatu.com>,
Simon Horman <simon.horman@netronome.com>
Subject: Re: [PATCH net-next] net/sched: cls_flower: Add user specified data
Date: Mon, 2 Jan 2017 20:33:40 -0800 [thread overview]
Message-ID: <586B29A4.2060408@gmail.com> (raw)
In-Reply-To: <13dfd148-6a5b-cf62-b2b9-1c13e6d31155@mojatatu.com>
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
next prev parent reply other threads:[~2017-01-03 4:35 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-02 13:13 [PATCH net-next] net/sched: cls_flower: Add user specified data Paul Blakey
2017-01-02 14:59 ` Jamal Hadi Salim
2017-01-02 18:23 ` John Fastabend
2017-01-02 22:21 ` Jamal Hadi Salim
2017-01-02 22:58 ` John Fastabend
2017-01-03 1:22 ` Jamal Hadi Salim
2017-01-03 4:33 ` John Fastabend [this message]
2017-01-03 11:44 ` Jamal Hadi Salim
2017-01-03 12:22 ` Paul Blakey
2017-01-04 10:14 ` Simon Horman
2017-01-04 11:45 ` Paul Blakey
2017-01-05 8:03 ` Simon Horman
2017-01-14 13:06 ` Jamal Hadi Salim
2017-01-08 17:19 ` Jiri Pirko
2017-01-09 18:23 ` John Fastabend
2017-01-14 12:56 ` Jamal Hadi Salim
2017-01-14 14:48 ` Jiri Pirko
2017-01-14 15:03 ` Jamal Hadi Salim
2017-01-14 15:29 ` Jiri Pirko
2017-01-14 17:46 ` Jamal Hadi Salim
2017-01-08 17:15 ` Jiri Pirko
2017-01-08 17:12 ` Jiri Pirko
2017-01-15 17:36 ` Paul Blakey
2017-01-15 19:08 ` John Fastabend
2017-01-16 7:54 ` Paul Blakey
2017-01-16 9:51 ` Jiri Pirko
2017-01-17 11:23 ` Jamal Hadi Salim
2017-01-17 11:53 ` Paul Blakey
2017-01-18 11:06 ` Jamal Hadi Salim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=586B29A4.2060408@gmail.com \
--to=john.fastabend@gmail.com \
--cc=davem@davemloft.net \
--cc=hadarh@mellanox.com \
--cc=jhs@mojatatu.com \
--cc=jiri@mellanox.com \
--cc=mrv@mojatatu.com \
--cc=netdev@vger.kernel.org \
--cc=ogerlitz@mellanox.com \
--cc=paulb@mellanox.com \
--cc=roid@mellanox.com \
--cc=simon.horman@netronome.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.