lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <c3868497-a299-3796-fe79-439dfb0653d2@gmail.com> Date: Thu, 6 Oct 2022 10:53:08 +0100 From: Edward Cree <ecree.xilinx@...il.com> To: Oz Shlomo <ozsh@...dia.com>, netdev@...r.kernel.org Cc: Jiri Pirko <jiri@...dia.com>, Jamal Hadi Salim <jhs@...atatu.com>, Simon Horman <simon.horman@...igine.com>, Baowen Zheng <baowen.zheng@...igine.com>, Vlad Buslov <vladbu@...dia.com>, Ido Schimmel <idosch@...dia.com>, Roi Dayan <roid@...dia.com> Subject: Re: [ RFC net-next v2 2/2] net: flow_offload: add action stats api 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
Powered by blists - more mailing lists