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: >> >> On Mon 19 Oct 2020 at 16:48, Jamal Hadi Salim <j...@mojatatu.com> wrote: >>> On 2020-10-18 8:16 a.m., Vlad Buslov wrote: > > [..] > >>> That could be a good thing, no? you get to see the action name with the >>> error. Its really not a big deal if you decide to do a->terse_print() >>> instead. >> >> Maybe. Just saying that this change would also change user-visible >> iproute2 behavior. >> > > You are right(for the non-terse output). tbh, not sure if it is a big > deal given it happens only for the error case (where scripts look > for exit codes typically); having said that: > a ->terse_print() would be ok > >> It is not a trivial change. To get this data we need to call >> tc_action_ops->dump() which puts bunch of other unrelated info in >> TCA_OPTIONS nested attr. This hurts both dump size and runtime >> performance. Even if we add another argument to dump "terse dump, print >> only index", index is still part of larger options structure which >> includes at least following fields: >> >> #define tc_gen \ >> __u32 index; \ >> __u32 capab; \ >> int action; \ >> int refcnt; \ >> int bindcnt >> > > > index is the _only_ important field for analytics purposes in that list. > i.e if i know the index i can correlate stats with one or more > filters (whether shared or not). > 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. > 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. > >> 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. > 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? > > cheers, > jamal
