netdev - Re: [PATCH net 1/1] net: openvswitch: Fix ct_state nat flags for conns arriving from tc

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] [day] [month] [year] [list]

Message-ID: <0d81227f-944c-47f2-9cba-2ae063f3a44c@mojatatu.com>
Date:   Thu, 6 Jan 2022 07:54:34 -0500
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Paul Blakey <paulb@...dia.com>,
        Daniel Borkmann <daniel@...earbox.net>
Cc:     dev@...nvswitch.org, netdev@...r.kernel.org,
        Cong Wang <xiyou.wangcong@...il.com>,
        Pravin B Shelar <pshelar@....org>, davem@...emloft.net,
        Jiri Pirko <jiri@...dia.com>, Jakub Kicinski <kuba@...nel.org>,
        Saeed Mahameed <saeedm@...dia.com>,
        Oz Shlomo <ozsh@...dia.com>, Vlad Buslov <vladbu@...dia.com>,
        Roi Dayan <roid@...dia.com>, john.fastabend@...il.com
Subject: Re: [PATCH net 1/1] net: openvswitch: Fix ct_state nat flags for
 conns arriving from tc

On 2022-01-05 11:18, Paul Blakey wrote:
> 
> 
> On Wed, 5 Jan 2022, Daniel Borkmann wrote:
> 

[..]

>> Full ack on the bloat for corner cases like ovs offload, especially
>> given distros just enable most stuff anyway and therefore no light
>> fast path as with !CONFIG_NET_TC_SKB_EXT. :(
>>
>> Could this somehow be hidden behind static key or such if offloads
>> are not used, so we can shrink it back to just calling into plain
>> __tcf_classify() for sw-only use cases (like BPF)?
>>
>>
> 
> It is used for both tc -> ovs and driver -> tc path.
> 
> I think I can do what you suggest adn will work on something like
> that,  but this specific patch  doesn't really change the ext
> allocation/derefences count (and probably  not the size as well).
> So can  we take  this (not yet posted v2 after fixing what already
> mentioned) and I'll do a patch of what you suggest in net-next?
> 

Sounds reasonable.

The main outstanding challenge is still going to be all these bit
declarations (and i am sure more to come) that are specific for
specific components in TC (act_ct in this case); if every component
started planting their flags(pun intended) then both backwards and
forwards compatibility are going to be needed for maintainance.

The _cb spaces are supposed to be opaque and meaning to whats in
that space is only sensible to the component that is storing and
retrieving. Something like chain id is ok in that namespaces
because it has global meaning to TC, the others are not. My
suggestion is going forward try to not add more component specific
variables.
For a proper solution:
I think we need some sort of "metadata bus" to resolve this in the
long run. Something which is a bigger surgery...

cheers,
jamal

cheers,
jamal