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

Message-ID: <5599f6-e02e-9ad5-3585-db252e946f43@nvidia.com>
Date:   Wed, 5 Jan 2022 18:18:06 +0200
From:   Paul Blakey <paulb@...dia.com>
To:     Daniel Borkmann <daniel@...earbox.net>
CC:     Jamal Hadi Salim <jhs@...atatu.com>, <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 Wed, 5 Jan 2022, Daniel Borkmann wrote:

> On 1/5/22 3:57 PM, Jamal Hadi Salim wrote:
> > On 2022-01-04 03:28, Paul Blakey wrote:
> > [..]
> >> --- a/include/linux/skbuff.h
> >> +++ b/include/linux/skbuff.h
> >> @@ -287,7 +287,9 @@ struct tc_skb_ext {
> >>       __u32 chain;
> >>       __u16 mru;
> >>       __u16 zone;
> >> -    bool post_ct;
> >> +    bool post_ct:1;
> >> +    bool post_ct_snat:1;
> >> +    bool post_ct_dnat:1;
> >>   };
> > 
> > is skb_ext intended only for ovs? If yes, why does it belong
> > in the core code? Ex: Looking at tcf_classify() which is such
> > a core function in the fast path any packet going via tc, it
> > is now encumbered with with checking presence of skb_ext.
> > I know passing around metadata is a paramount requirement
> > for programmability but this is getting messier with speacial
> > use cases for ovs and/or offload...
> 
> 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?