netdev - Re: [PATCH] xfrm: add SA information to the offloaded packet

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: <ZvKVuBTkh2dts8Qy@gauss3.secunet.de>
Date: Tue, 24 Sep 2024 12:34:32 +0200
From: Steffen Klassert <steffen.klassert@...unet.com>
To: Leon Romanovsky <leon@...nel.org>
CC: Feng Wang <wangfe@...gle.com>, <netdev@...r.kernel.org>,
	<antony.antony@...unet.com>
Subject: Re: [PATCH] xfrm: add SA information to the offloaded packet

On Wed, Sep 11, 2024 at 01:40:40PM +0300, Leon Romanovsky wrote:
> On Mon, Sep 09, 2024 at 11:09:05AM +0200, Steffen Klassert wrote:
> > On Mon, Sep 02, 2024 at 12:44:52PM +0300, Leon Romanovsky wrote:
> > > On Mon, Sep 02, 2024 at 09:44:24AM +0200, Steffen Klassert wrote:
> > > > > 
> > > > > Steffen,
> > > > > 
> > > > > What is your position on this patch?
> > > > > It is the same patch (logically) as the one that was rejected before?
> > > > > https://lore.kernel.org/all/ZfpnCIv+8eYd7CpO@gauss3.secunet.de/
> > > > 
> > > > This is an infrastructure patch to support routing based IPsec
> > > > with xfrm interfaces. I just did not notice it because it was not
> > > > mentioned in the commit message of the first patchset. This should have
> > > > been included into the packet offload API patchset, but I overlooked
> > > > that xfrm interfaces can't work with packet offload mode. The stack
> > > > infrastructure should be complete, so that drivers can implement
> > > > that without the need to fix the stack before.
> > > 
> > > Core implementation that is not used by any upstream code is rarely
> > > right thing to do. It is not tested, complicates the code and mostly
> > > overlooked when patches are reviewed. The better way will be to extend
> > > the stack when this feature will be actually used and needed.
> > 
> > This is our tradeoff, an API should be fully designed from the
> > beginning, everything else is bad design and will likely result
> > in band aids (as it happens here). The API can be connected to
> > netdevsim to test it.
> > 
> > Currently the combination of xfrm interfaces and packet offload
> > is just broken. 
> 
> I don't think that it is broken.

I don't see anything that prevents you from offloading a SA
with an xfrm interface ID. The binding to the interface is
just ignored in that case.

> It is just not implemented. XFRM
> interfaces are optional field, which is not really popular in the
> field.

It is very popular, I know of more than a billion devices that
are using xfrm interfaces.

> 
> > Unfortunalely this patch does not fix it.
> > 
> > I think we need to do three things:
> > 
> > - Fix xfrm interfaces + packet offload combination
> > 
> > - Extend netdevsim to support packet offload
> > 
> > - Extend the API for xfrm interfaces (and everything
> >   else we forgot).
> 
> This is the most challenging part. It is not clear what should
> we extend if customers are not asking for it and they are extremely
> happy with the current IPsec packet offload state.

We just need to push the information down to the driver,
and reject the offload if not supported.

> 
> BTW, I'm aware of one gap, which is not clear how to handle, and
> it is combination of policy sockets and offload.

Socket policies are a bit special as they are configured by
the application that uses the socket. I don't think that
we can even configure offload for a socket policy.