On 14/05/2020 15:49, Jiri Pirko wrote: > Thu, May 14, 2020 at 04:04:02PM CEST, ec...@solarflare.com wrote: >> Either way, the need to repeat the policy on every tc command suggests >> that there really ought to instead be a separate API for configuring >> conntrack offload policy, either per zone or per (zone, device) pair, >> as appropriate. > You don't have to repeat. You specify it in the first one, in the rest > you omit it. Ok, well (a) the commit message needs changing to make that clear, and (b) that kind of implicit action-at-a-distance slightly bothers me. If you don't repeat, then the order of tc commands matters, and any program (e.g. OvS) generating these commands will need to either keep track of which zones it's configured policy for already, or just repeat on every command just in case. It really doesn't feel like an orthogonal, Unixy API to me.
<offtopic rambliness="very"> TBH I think that when a tc rule with a ct action is offloaded, drivers should get three callbacks in order: 1) a CT zone callback (if the CT zone is 'new') 2) an action callback, including a pointer to the CT zone info (in case the driver chose to ignore the CT zone callback because it had no offloading work to do at that point) (if the action is 'new') 3) a rule callback, including a pointer to the action info (in case the driver ignored the action creation). And each of these should be drivable directly from userspace as well as being called automatically by the level below it. Currently we have (2) as a distinct entity in TC, but no-one offloads it (and as I've ranted before, that makes a mess of stats) and AIUI it's not called when user creates a rule, only when using 'tc action' command directly). And (1) doesn't exist at all; drivers just have to notice the first time a tc ct action they're offloading mentions a given zone, and call into nf_flow_table_offload to register for tracked conns in that zone. I feel that this hierarchical 'offload dependencies first' model would make drivers simpler and more robust, as well as helping to ensure different drivers share a consistent interpretation of the API. RFC on the above? Obviously I'm not likely to start implementing it until after we have a TC-supporting sfc driver upstream (it's coming Real Soon Now™, I swear!) but I thought it worthwhile to throw the design up for discussion earlier rather than later. </offtopic> -ed
