netdev - Re: [PATCHv3 net-next 3/3] bonding: support xfrm state update

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: <ZsSqWO-zbgYSQIdY@hog>
Date: Tue, 20 Aug 2024 16:38:16 +0200
From: Sabrina Dubroca <sd@...asysnail.net>
To: Hangbin Liu <liuhangbin@...il.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 3/3] bonding: support xfrm state update

Hi Hangbin,

2024-08-20, 08:48:40 +0800, Hangbin Liu wrote:
> The patch add xfrm statistics update for bonding IPsec offload.
> 
> 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 24747fceef66..4a4a1d9c8cca 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -675,11 +675,36 @@ static void bond_advance_esn_state(struct xfrm_state *xs)
>  	rcu_read_unlock();
>  }
>  
> +/**
> + * bond_xfrm_update_stats - Update xfrm state
> + * @xs: pointer to transformer state struct
> + **/
> +static void bond_xfrm_update_stats(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_update_stats) {
> +		pr_warn("%s: %s doesn't support xdo_dev_state_update_stats\n", __func__, real_dev->name);

I'm not convinced we should warn here. Most drivers don't implement
xdo_dev_state_update_stats, so if we're using one of those drivers
(for example netdevsim), we'll get one line in dmesg for every "ip
xfrm state" command run by the user. At most it should be ratelimited,
but since it's an optional callback, I think no warning would be fine.

-- 
Sabrina