On Sat 24 Oct 2020 at 20:40, Jamal Hadi Salim <j...@mojatatu.com> wrote: > On 2020-10-23 8:48 a.m., Vlad Buslov wrote: >> On Thu 22 Oct 2020 at 17:05, Jamal Hadi Salim <j...@mojatatu.com> wrote: >>> On 2020-10-21 4:19 a.m., Vlad Buslov wrote: >>>> >>>> On Tue 20 Oct 2020 at 15:29, Jamal Hadi Salim <j...@mojatatu.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? >> >> AFAIK on only one action per flow. >> > > To each their own i guess. Sounds like it is being used as flowid > entity. > We pack a lot of metaencoding into those cookies. And to us > a "service" is essentially a filter match folowed by a graph > of actions (which could cyclic). > > >>>> 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). >> >> There was several reasons for me to include it: >> >> - As I wrote in previous email its TLV is only included in dump if user >> set the cookie. Users who don't use cookies don't lose any performance >> of terse dump. >> >> - Including it didn't require any changes to tc_action_ops->dump() (like >> passing 'terse' flag or introducing dedicated terse_dump() callback) >> because it is processed in tcf_action_dump_1(). >> >> - OVS was the main use-case for us because it relies on filter dump for >> flow revalidation and uses cookie to identify the flows. >> > > Which is fine - but it is a very ovs specific need. >>> >>>>> 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. >> >> Yes, that makes sense. I guess introducing something like 'tc action -br >> ls ..' mode implemented by means of existing terse flag + new 'also >> output action index' flag would achieve that goal. >> > > Right. There should be no interest in the cookie here at all. Maybe > it could be optional with a flag indication. > Have time to cook a patch? I'll taste/test it.
Patch to make cookie in filter terse dump optional? That would break existing terse dump users that rely on it (OVS). > > cheers, > jamal
