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: <CADsK2K8KqJThB3pkz7oAZT_4yXgy8v89TK83W50KaR-VSSKjOg@mail.gmail.com>
Date: Thu, 29 Aug 2024 14:19:25 -0700
From: Feng Wang <wangfe@...gle.com>
To: Leon Romanovsky <leon@...nel.org>
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

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. However, the crucial point is
that including this information empowers certain drivers to leverage
it without affecting those that don't need it.

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.

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