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

From: Sabrina Dubroca <[email-masked]>
Date: Tue, 20 Aug 2024 16:38:16 +0200

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 <[email-masked]>
> Signed-off-by: Hangbin Liu <[email-masked]>
> ---
>  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