netdev - Re: [PATCH net-next 0/3] net/sched: act_ct: Add support for specifying tuple offload policy

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9f68872f-fe3f-e86a-4c74-8b33cd9ee433@solarflare.com>
Date:   Thu, 14 May 2020 16:28:14 +0100
From:   Edward Cree <ecree@...arflare.com>
To:     Jiri Pirko <jiri@...nulli.us>
CC:     Paul Blakey <paulb@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Oz Shlomo <ozsh@...lanox.com>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Vlad Buslov <vladbu@...lanox.com>,
        "David Miller" <davem@...emloft.net>, <netdev@...r.kernel.org>,
        Jiri Pirko <jiri@...lanox.com>, Roi Dayan <roid@...lanox.com>
Subject: Re: [PATCH net-next 0/3] net/sched: act_ct: Add support for
 specifying tuple offload policy

On 14/05/2020 15:49, Jiri Pirko wrote:
> Thu, May 14, 2020 at 04:04:02PM CEST, ecree@...arflare.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