netdev - Re: [PATCH xfrm-next v7 6/8] xfrm: speed-up lookup of HW policies

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]

Date:   Sun, 20 Nov 2022 21:17:02 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Steffen Klassert <steffen.klassert@...unet.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH xfrm-next v7 6/8] xfrm: speed-up lookup of HW policies

On Fri, Nov 18, 2022 at 11:49:07AM +0100, Steffen Klassert wrote:
> On Thu, Nov 17, 2022 at 02:51:33PM +0200, Leon Romanovsky wrote:
> > On Thu, Nov 17, 2022 at 01:12:43PM +0100, Steffen Klassert wrote:
> > > On Wed, Nov 09, 2022 at 02:54:34PM +0200, Leon Romanovsky wrote:
> > > > From: Leon Romanovsky <leonro@...dia.com>
> > > 
> > > This does not work. A larval state will never have a x->xso.type set.
> > 
> > x->xso.type always exists. Default is 0, which is XFRM_DEV_OFFLOAD_UNSPECIFIED.
> > It means this XFRM_STATE_INSERT() will behave exactly as hlist_add_head_rcu() before.
> 
> Sure it exists, and is always 0 here.
> 
> > 
> > > So this raises the question how to handle acquires with this packet
> > > offload. 
> > 
> > We handle acquires as SW policies and don't offload them.
> 
> We trigger acquires with states, not policies. The thing is,
> we might match a HW policy but create a SW acquire state.
> This will not match anymore as soon as the lookup is
> implemented correctly.

For now, all such packets will be dropped as we have offlaoded
policy but not SA.

Like we said in one of our IPsec coffee hours, this flow will be
supported later. Right now, it is not important.

> 
> > > You could place the type and offload device to the template,
> > > but we also have to make sure not to mess too much with the non
> > > offloaded codepath.
> > > 
> > > This is yet another corner case where the concept of doing policy and
> > > state lookup in software for a HW offload does not work so well. I
> > > fear this is not the last corner case that comes up once you put this
> > > into a real network.
> > > 
> > 
> > It is not different from any other kernel code, bugs will be fixed.
> 
> The thing that is different here is, that the concept is already
> broken. We can't split the datapath to be partially handled in
> SW and HW in any sane way, this becomes clearer and clearer.
> 
> The full protocol offload simply does not fit well into HW,
> but we try to make it fit with a hammer. This is the problem
> why I do not really like this, and is also the reason why this
> is still not merged. We might be much better of by doing a
> HW frindly redesign of the protocol and offload this then.
> But, yes that takes time and will have issues too.

When you say "protocol", what do you mean? Many users, who have
deployed IPsec solutions, just want to have same look and feel
but much faster.

I truly believe that this packet offload fits SW model and the
(small) amount of changes supports it. There are almost no changes
to the stack to natively support this offload.

As long as HW involved, you will never have solution without issues,
and like you said even redesign "will have issues".

Plus, we called it packet offload, better and redesigned version will
be called something else. My patches which enhance XFRM to support more
than type of offload are exactly for that.

Thanks