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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240911104040.GG4026@unreal>
Date: Wed, 11 Sep 2024 13:40:40 +0300
From: Leon Romanovsky <leon@...nel.org>
To: Steffen Klassert <steffen.klassert@...unet.com>
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 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. It is just not implemented. XFRM
interfaces are optional field, which is not really popular in the
field.

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

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

> 
> > IMHO, attempt to enrich core code without showing users of this new flow
> > is comparable to premature optimization.
> > 
> > And Feng more than once said that this code is for some out-of-tree
> > driver.
> 
> It is an API, so everyone can use it.

Of course, as long as in-kernel user exists.

Thanks