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

[Leon Romanovsky <leon@kernel.org>]

On Thu, Aug 29, 2024 at 02:19:25PM -0700, Feng Wang wrote:
> Hi Leon,
> 
> Thank you again for your thoughtful questions and comments. I'd like
> to provide further clarification and address your points:
> 
> SA Information Usage:
> 
> There are several instances in the kernel code where it's used, such
> as in esp4(6)_offload.c and xfrm.c. This clearly demonstrates how SA
> information is used. Moreover, passing this information to the driver
> shouldn't negatively impact those drivers that don't require it.
> Regarding a driver example, the function mlx5e_ipsec_feature_check
> caught my attention.
> https://elixir.bootlin.com/linux/v6.10/source/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h#L89)
> As you're more familiar with this codebase, I defer to your expertise
> on whether it's an appropriate sample. 

This function is not involved when packet is going to be offloaded for "IPsec packet offload".

> However, the crucial point is that including this information empowers certain drivers to leverage
> it without affecting those that don't need it.

Can you please provide a list of drivers that will benefit from this change?
Can you give a complete flow (including driver) which didn't work before
and will work after this change?

> 
> validate_xmit_xfrm Function:
> My primary goal in discussing the validate_xmit_xfrm function is to
> assure you that my patch maintains the existing packet offload code
> flow, avoiding any unintended disruption.

The whole idea of packet offload is to skip everything in XFRM stack and
present packet as plain text.

> 
> State Release:
> I've noticed that secpath_reset() is called before xfrm_output(). The
> sequence seems to be: xfrmi_xmit2 -> xfrmi_scrub_packet ->
> secpath_reset(), followed by xfrmi_xmit2 calling dst_output, which is
> essentially xfrm_output().
> I'm also open to moving the xfrm_state_hold(x) after the if (!xo)
> check block. This would ensure the state is held only when everything
> is ok. I'll gladly make this adjustment if you believe it's the better
> option.
> 
> Thank you once again for your valuable insights and collaboration.
> Your feedback is greatly appreciated!
> 
> Feng
>