On 06/05/2019 13:41, Jamal Hadi Salim wrote: > On 2019-05-04 2:27 a.m., Jakub Kicinski wrote: >> On Fri, 3 May 2019 16:06:55 +0100, Edward Cree wrote: >>> Introduce a new offload command TC_CLSFLOWER_STATS_BYINDEX, similar to >>> the existing TC_CLSFLOWER_STATS but specifying an action_index (the >>> tcfa_index of the action), which is called for each stats-having action >>> on the rule. Drivers should implement either, but not both, of these >>> commands. > > [..] >> >> It feels a little strange to me to call the new stats updates from >> cls_flower, if we really want to support action sharing correctly. >> >> Can RTM_GETACTION not be used to dump actions without dumping the >> classifiers? If we dump from the classifiers wouldn't that lead to >> stale stats being returned? > > Not sure about the staleness factor, but: > For efficiency reasons we certainly need the RTM_GETACTION approach > (as you stated above we dont need to dump all that classifier info if > all we want are stats). This becomes a big deal if you have a lot > of stats/rules. I don't know much of anything about RTM_GETACTION, but it doesn't appear to be part of the current "tc offload" world, which AIUI is very much centred around cls_flower. I'm just trying to make counters in cls_flower offload do 'the right thing' (whatever that may be), anything else is out of scope.
> Most H/W i have seen has a global indexed stats table which is > shared by different action types (droppers, accept, mirror etc). > The specific actions may also have their own tables which also > then refer to the 32 bit index used in the stats table[1]. > So for this to work well, the action will need at minimal to have > two indices one that is used in hardware stats table > and another that is kernel mapped to identify the attributes. Of > course we'll need to have a skip_sw flag etc. I'm not sure I'm parsing this correctly, but are you saying that the index namespace is per-action type? I.e. a mirred and a drop action could have the same index yet expect to have separate counters? My approach here has assumed that in such a case they would share their counters.
