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:   Fri, 18 Nov 2022 11:49:07 +0100
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Leon Romanovsky <leon@...nel.org>
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 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.

> > 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.