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. > > >>> 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. Makes sense. > >>> >>>> 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). 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. > >>> 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. > > cheers, > jamal
