netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steffen Klassert <steffen.klassert@secunet.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Feng Wang <wangfe@google.com>, Leon Romanovsky <leon@kernel.org>,
	<netdev@vger.kernel.org>, <antony.antony@secunet.com>,
	<pabeni@redhat.com>
Subject: Re: [PATCH v7] xfrm: add SA information to the offloaded packet when if_id is set
Date: Wed, 11 Dec 2024 11:57:03 +0100	[thread overview]
Message-ID: <Z1lv/+VBldgUYTGw@gauss3.secunet.de> (raw)
In-Reply-To: <20241210192048.386d518a@kernel.org>

On Tue, Dec 10, 2024 at 07:20:48PM -0800, Jakub Kicinski wrote:
> On Tue, 10 Dec 2024 10:38:34 +0100 Steffen Klassert wrote:
> > > This patch was done based on our previous discussion.  I did the
> > > changes we agreed on.  
> > 
> > there is still no real packet offload support for netdev sim.
> > And as said, this is at most the second best option.
> > 
> > You need to prove that this works. I want a complete API,
> > but I also want a working one.
> > 
> > The easiest way to prove that this is implemented correctly
> > is to upstream your driver. Everyting else is controversial
> > and complicated.
> 
> Yes, I don't have full context but FWIW offload changes accompanied 
> by just netdevsim modifications raise a red flag:

Actually, we have the mlx5 driver that supports packet offload.
When that was implemented, xfrm interfaces were no usecase.
Because of that, we forgot to care for the xfrm interface ID as
a lookup key. The problem is that users can still configure
policies/states for xfrm interfaces and packet offload.
The driver then just don't know if a packet was routed via
a xfrm interface, and if so, via which one. So it might happen
that a wrong policy/state is applied to a packet.

My idea was to fix that by supporting xfrm interfaces
with packet offload in the stack. But after looking
a bit closer to the code yesterday, I noticed that we
might not need any stack changes to get this right.

I think the driver should do the following:

If xfrm interfaces are not supported:

- Reject policies and states that have xfrm interfaces
  configured.

If xfrm interfaces are supported:

- Accept policies and states that have xfrm interfaces
  configured.

- On TX: Use the drivers xdo_dev_offload_ok function
  to check if the packet came via the correct xfrm
  interface and/or amend that info by adding the
  xfrm state to the packets secpath.

- On RX: Driver traveses through the list of registered
  xfrm interfaces and match these against the used SAs
  xfrm interface ID.


By doing that, no stack changes should be required.

  reply	other threads:[~2024-12-11 10:57 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-09 20:28 [PATCH v7] xfrm: add SA information to the offloaded packet when if_id is set Feng Wang
2024-12-09 21:53 ` Leon Romanovsky
2024-12-09 23:44   ` Feng Wang
2024-12-10  9:38     ` Steffen Klassert
2024-12-11  3:20       ` Jakub Kicinski
2024-12-11 10:57         ` Steffen Klassert [this message]
2024-12-10  9:28 ` Steffen Klassert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z1lv/+VBldgUYTGw@gauss3.secunet.de \
    --to=steffen.klassert@secunet.com \
    --cc=antony.antony@secunet.com \
    --cc=kuba@kernel.org \
    --cc=leon@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=wangfe@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).