Message-ID: <6b671aaf-d35d-70a5-65e0-40308baeb471@mojatatu.com>
Date:   Mon, 2 Jan 2017 17:21:41 -0500
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     John Fastabend <john.fastabend@...il.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 01:23 PM, John Fastabend wrote:

>
> Additionally I would like to point out this is an arbitrary length binary
> blob (for undefined use, without even a specified encoding) that gets pushed
> between user space and hardware ;) This seemed to get folks fairly excited in
> the past.
>

The binary blob size is a little strange - but i think there is value
in storing some "cookie" field. The challenge is whether the kernel
gets to intepret it; in which case encoding must be specified. Or
whether we should leave it up to user space - in which something
like tc could standardize its own encodings.

> Some questions, exactly what do you mean by "port mappings" above? In
> general the 'tc' API uses the netdev the netlink msg is processed on as
> the port mapping. If you mean OVS port to netdev port I think this is
> a OVS problem and nothing to do with 'tc'. For what its worth there is an
> existing problem with 'tc' where rules only apply to a single ingress or
> egress port which is limiting on hardware.
>

In our case the desire is to be able to correlate for a system wide
mostly identity/key mapping.

> The UFID in my ovs code base is defined as best I can tell here,
>
>         [OVS_FLOW_ATTR_UFID] = { .type = NL_A_UNSPEC, .optional = true,
>                                  .min_len = sizeof(ovs_u128) },
>
> So you need 128 bits if you want a 1:1 mapping onto 'tc'. So rather
> than an arbitrary blob why not make the case that 'tc' ids need to be
> 128 bits long? Even if its just initially done in flower call it
> flower_flow_id and define it so its not opaque and at least at the code
> level it isn't an arbitrary blob of data.
>

I dont know what this UFID is, but do note:
The idea is not new - the FIB for example has some such cookie
(albeit a tiny one) which will typically be populated to tell
you who/what installed the entry.
I could see f.e use for this cookie to simplify and pretty print in
a human language for the u32 classifier (i.e user space tc sets
some fields in the cookie when updating kernel and when user space
invokes get/dump it uses the cookie to intepret how to pretty print).

I have attached a compile tested version of the cookies on actions
(flat 64 bit; now that we have experienced the use when we have a
large number of counters - I would not mind a 128 bit field).


cheers,
jamal

> And what are the "next" uses of this besides OVS. It would be really
> valuable to see how this generalizes to other usage models. To avoid
> embedding OVS syntax into 'tc'.
>
> Finally if you want to see an example of binary data encodings look at
> how drivers/hardware/users are currently using the user defined bits in
> ethtools ntuple API. Also track down out of tree drivers to see other
> interesting uses. And that was capped at 64bits :/
>
> Thanks,
> John
>
>
>
>
>


View attachment "kernel-new-net-next" of type "text/plain" (2178 bytes)

View attachment "iprt-ck-patch" of type "text/plain" (2426 bytes)

Powered by blists - more mailing lists