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: <ZsS3Zh8bT-qc46s7@hog>
Date: Tue, 20 Aug 2024 17:33:58 +0200
From: Sabrina Dubroca <sd@...asysnail.net>
To: Hangbin Liu <liuhangbin@...il.com>,
	Steffen Klassert <steffen.klassert@...unet.com>
Cc: netdev@...r.kernel.org, Jay Vosburgh <j.vosburgh@...il.com>,
	"David S . Miller" <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Nikolay Aleksandrov <razor@...ckwall.org>,
	Tariq Toukan <tariqt@...dia.com>, Jianbo Liu <jianbol@...dia.com>,
	Simon Horman <horms@...nel.org>
Subject: Re: [PATCHv3 net-next 2/3] bonding: Add ESN support to IPSec HW
 offload

Hi Hangbin,

(adding Steffen since we're getting a bit into details of IPsec)

2024-08-20, 08:48:39 +0800, Hangbin Liu wrote:
> Currently, users can see that bonding supports IPSec HW offload via ethtool.
> However, this functionality does not work with NICs like Mellanox cards when
> ESN (Extended Sequence Numbers) is enabled, as ESN functions are not yet
> supported. This patch adds ESN support to the bonding IPSec device offload,
> ensuring proper functionality with NICs that support ESN.
> 
> Reviewed-by: Nikolay Aleksandrov <razor@...ckwall.org>
> Signed-off-by: Hangbin Liu <liuhangbin@...il.com>
> ---
>  drivers/net/bonding/bond_main.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 560e3416f6f5..24747fceef66 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -651,10 +651,35 @@ static bool bond_ipsec_offload_ok(struct sk_buff *skb, struct xfrm_state *xs)
>  	return err;
>  }
>  
> +/**
> + * bond_advance_esn_state - ESN support for IPSec HW offload
> + * @xs: pointer to transformer state struct
> + **/
> +static void bond_advance_esn_state(struct xfrm_state *xs)
> +{
> +	struct net_device *real_dev;
> +
> +	rcu_read_lock();
> +	real_dev = bond_ipsec_dev(xs);
> +	if (!real_dev)
> +		goto out;
> +
> +	if (!real_dev->xfrmdev_ops ||
> +	    !real_dev->xfrmdev_ops->xdo_dev_state_advance_esn) {
> +		pr_warn("%s: %s doesn't support xdo_dev_state_advance_esn\n", __func__, real_dev->name);

xdo_dev_state_advance_esn is called on the receive path for every
packet when ESN is enabled (xfrm_input -> xfrm_replay_advance ->
xfrm_replay_advance_esn -> xfrm_dev_state_advance_esn), this needs to
be ratelimited.


But this CB is required to make offload with ESN work. If it's not
implemented on a lower device, I expect things will break. I wonder
what the best behavior would be:

 - just warn (this patch) -- but things will break for users

 - don't allow mixing devices that support ESN with devices that don't
   in the same bond (really bad if the user doesn't care about ESN)

 - don't allow enslaving devices that don't support ESN if an xfrm
   state using ESN has been added to the bond, and don't allow
   creating ESN states if the bond has one non-ESN slave

 - disable re-offloading the state if we have to migrate it from an
   ESN-enabled to a non-ESN device (when changing the active slave)
   -- and fall back to the (slow) SW implementation

 - other options that I'm not thinking about?


Sorry I'm only noticing this at v3 :(

-- 
Sabrina


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ