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: <20240830143051.GA4000@unreal>
Date: Fri, 30 Aug 2024 17:30:51 +0300
From: Leon Romanovsky <leon@...nel.org>
To: Feng Wang <wangfe@...gle.com>
Cc: Steffen Klassert <steffen.klassert@...unet.com>, netdev@...r.kernel.org,
	antony.antony@...unet.com
Subject: Re: [PATCH] xfrm: add SA information to the offloaded packet

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
>