On Mon, Apr 29, 2019 at 05:25:10PM +0100, Edward Cree wrote: > On 29/04/2019 16:21, Pablo Neira Ayuso wrote: > > On Mon, Apr 29, 2019 at 03:11:06PM +0100, Edward Cree wrote: > >> This is a bit of a mess; the best idea I've got is for the > >> TC_CLSFLOWER_STATS call to include a tcfa_index. Then the driver > >> returns counter stats for that index, and tcf_exts_stats_update() > >> only updates those actions whose index matches. But then > >> fl_hw_update_stats() would have to iterate over all the indices in > >> f->exts. What do you think? > > You could extend struct flow_stats to pass an array of stats to the > > driver, including one stats per action and the counter index. Then, > > tcf_exts_stats_update() uses this array of stats to update per-action > > stats. > Yes, but that means allocating the flow_stats.stats array each time;
We use the stack to attach a reasonable size array, eg. 16 actions, otherwise fall back to kmalloc(). I haven't seen any driver in the tree supporting more than that so far. > I'd rather avoid memory allocation unless it's necessary. As long as > we can move the preempt_disable() inside the loop that's currently in > tcf_exts_stats_update() (i.e. only disable pre-emption across each > individual call to tcf_action_stats_update()) I think we can. > I think I prefer my approach (ask for one tcfa_index at a time); but > unmodified drivers that don't look at the passed index would return > zeroes for actions after the first, so we'll need some way to handle > those drivers separately (e.g. one tc_setup_cb_call with "answer > this one if you don't do indices" and a bunch more with specified > index values). I think that requires much less change to the > existing drivers than putting an array back in the API, and keeps as > much of the work as possible in the core where it won't have to be > replicated in every driver. That's all right. This chunk update will not be particularly large, so we can change it anytime in the future. > I'll put an RFC patch together soonish if no objections. Sure, thanks.
