Message-ID: <CADsK2K9aHkouKK4btdEMmPGkwOEZTNmd7OPHvYQErd+3NViDnQ@mail.gmail.com>
Date: Tue, 24 Sep 2024 10:57:12 -0700
From: Feng Wang <wangfe@...gle.com>
To: Steffen Klassert <steffen.klassert@...unet.com>
Cc: Leon Romanovsky <leon@...nel.org>, netdev@...r.kernel.org, antony.antony@...unet.com
Subject: Re: [PATCH] xfrm: add SA information to the offloaded packet

Hi Steffen,

The easiest thing would be to upstream your driver, that is the
prefered way and would just end this discussion.

I will try to upstream the xfrm interface id handling code to
netdevsim, thus it will have an in-driver implementation.

Thanks,

Feng

On Tue, Sep 24, 2024 at 3:34 AM Steffen Klassert
<steffen.klassert@...unet.com> wrote:
>
> 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.
>

Powered by blists - more mailing lists