Re: [ RFC net-next v2 2/2] net: flow_offload: add action stats api

[Edward Cree <ecree.xilinx@gmail.com>]

On 04/10/2022 09:22, Oz Shlomo wrote:
>> @@ -588,6 +596,8 @@ struct flow_cls_offload {
>>       unsigned long cookie;
>>       struct flow_rule *rule;
>>       struct flow_stats stats;
>> +    struct flow_act_stat act_stats[FLOW_OFFLOAD_MAX_ACT_STATS];
> 
> Apparently this array can exceed the stack frame size for several archs (reported by the kernel test bot).

Seems to me like yet another sign this array shouldn't exist in the
 first place and you should be calling a driver offload function per
 action and not trying to squeeze all this through the existing
 flow-rule-centric API.
Why can't the action stats be stored in struct flow_action_entry,
 inside struct flow_rule?  Then, if you really want a single call
 for performance reasons, action-stats-aware drivers could just walk
 through flow_action_for_each(..., &flow_rule->action) and update
 each action's stats based on action->act_cookie.  That should still
 allow proper shared action handling, and seems far more elegant
 than this array.  (Any time you have two arrays and there's a close
 connection between a[i] and b[i], that's a good sign one's elements
 should be a struct member of the other's, rather than them being
 tied together only by index.)

-ed