[PATCHv2 net-next 0/2] Bonding: support new xfrm state offload functions

[PATCHv2 net-next 0/2] Bonding: support new xfrm state offload functions

[Hangbin Liu]

Add 2 new xfrm state offload functions xdo_dev_state_advance_esn and
xdo_dev_state_update_stats for bonding. The xdo_dev_state_free will be
added by Jianbo's patchset [1]. I will add the bonding xfrm policy offload
in future.

I planned to add the new XFRM state offload functions after Jianbo's
patchset [1], but it seems that may take some time. Therefore, I am
posting these two patches to net-next now, as our users are waiting for
this functionality. If Jianbo's patch is applied first, I can update these
patches accordingly.

[1] https://lore.kernel.org/netdev/20240815142103.2253886-2-tariqt@nvidia.com

v2: Add a function to process the common device checking (Nikolay Aleksandrov)
    Remove unused variable (Simon Horman)
v1: lore.kernel.org/netdev/20240816035518.203704-1-liuhangbin@gmail.com

Hangbin Liu (3):
  bonding: add common function to check ipsec device
  bonding: Add ESN support to IPSec HW offload
  bonding: support xfrm state update

 drivers/net/bonding/bond_main.c | 93 ++++++++++++++++++++++++++++-----
 1 file changed, 80 insertions(+), 13 deletions(-)

-- 
2.45.0

[PATCHv2 net-next 1/3] bonding: add common function to check ipsec device

[Hangbin Liu]

This patch adds a common function to check the status of IPSec devices.
This function will be useful for future implementations, such as IPSec ESN
and state offload callbacks.

Suggested-by: Nikolay Aleksandrov <razor@blackwall.org>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 drivers/net/bonding/bond_main.c | 43 +++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index f9633a6f8571..250a2717b4e9 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -418,6 +418,34 @@ static int bond_vlan_rx_kill_vid(struct net_device *bond_dev,
 /*---------------------------------- XFRM -----------------------------------*/
 
 #ifdef CONFIG_XFRM_OFFLOAD
+/**
+ * bond_ipsec_dev - return the device for ipsec offload, or NULL if not exist
+ *                  caller must hold rcu_read_lock.
+ * @xs: pointer to transformer state struct
+ **/
+static struct net_device bond_ipsec_dev(struct xfrm_state *xs)
+{
+	struct net_device *bond_dev = xs->xso.dev;
+	struct net_device *real_dev;
+	struct bonding *bond;
+	struct slave *slave;
+
+	if (!bond_dev)
+		return NULL;
+
+	bond = netdev_priv(bond_dev);
+	slave = rcu_dereference(bond->curr_active_slave);
+	real_dev = slave ? slave->dev : NULL;
+
+	if ((BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) ||
+	    !slave || !real_dev || !xs->xso.real_dev)
+		return NULL;
+
+	WARN_ON(xs->xso.real_dev != slave->dev);
+
+	return real_dev;
+}
+
 /**
  * bond_ipsec_add_sa - program device with a security association
  * @xs: pointer to transformer state struct
@@ -595,23 +623,12 @@ static void bond_ipsec_del_sa_all(struct bonding *bond)
  **/
 static bool bond_ipsec_offload_ok(struct sk_buff *skb, struct xfrm_state *xs)
 {
-	struct net_device *bond_dev = xs->xso.dev;
 	struct net_device *real_dev;
-	struct slave *curr_active;
-	struct bonding *bond;
 	int err;
 
-	bond = netdev_priv(bond_dev);
 	rcu_read_lock();
-	curr_active = rcu_dereference(bond->curr_active_slave);
-	real_dev = curr_active->dev;
-
-	if (BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) {
-		err = false;
-		goto out;
-	}
-
-	if (!xs->xso.real_dev) {
+	real_dev = bond_ipsec_dev(xs);
+	if (!real_dev) {
 		err = false;
 		goto out;
 	}
-- 
2.45.0

[PATCHv2 net-next 2/3] bonding: Add ESN support to IPSec HW offload

[Hangbin Liu]

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.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.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 250a2717b4e9..3c04bdba17d4 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -646,10 +646,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);
+		goto out;
+	}
+
+	rhel_dev->xfrmdev_ops->xdo_dev_state_advance_esn(xs);
+out:
+	rcu_read_unlock();
+}
+
 static const struct xfrmdev_ops bond_xfrmdev_ops = {
 	.xdo_dev_state_add = bond_ipsec_add_sa,
 	.xdo_dev_state_delete = bond_ipsec_del_sa,
 	.xdo_dev_offload_ok = bond_ipsec_offload_ok,
+	.xdo_dev_state_advance_esn = bond_advance_esn_state,
 };
 #endif /* CONFIG_XFRM_OFFLOAD */
 
-- 
2.45.0

[PATCHv2 net-next 3/3] bonding: support xfrm state update

[Hangbin Liu]

The patch add xfrm statistics update for bonding IPsec offload.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.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 3c04bdba17d4..9e41e34e9039 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -670,11 +670,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);
+		goto out;
+	}
+
+	real_dev->xfrmdev_ops->xdo_dev_state_update_stats(xs);
+out:
+	rcu_read_unlock();
+}
+
 static const struct xfrmdev_ops bond_xfrmdev_ops = {
 	.xdo_dev_state_add = bond_ipsec_add_sa,
 	.xdo_dev_state_delete = bond_ipsec_del_sa,
 	.xdo_dev_offload_ok = bond_ipsec_offload_ok,
 	.xdo_dev_state_advance_esn = bond_advance_esn_state,
+	.xdo_dev_state_update_stats = bond_xfrm_update_stats,
 };
 #endif /* CONFIG_XFRM_OFFLOAD */
 
-- 
2.45.0

Re: [PATCHv2 net-next 1/3] bonding: add common function to check ipsec device

[Nikolay Aleksandrov]

On 19/08/2024 10:53, Hangbin Liu wrote:
> This patch adds a common function to check the status of IPSec devices.
> This function will be useful for future implementations, such as IPSec ESN
> and state offload callbacks.
> 
> Suggested-by: Nikolay Aleksandrov <razor@blackwall.org>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  drivers/net/bonding/bond_main.c | 43 +++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index f9633a6f8571..250a2717b4e9 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -418,6 +418,34 @@ static int bond_vlan_rx_kill_vid(struct net_device *bond_dev,
>  /*---------------------------------- XFRM -----------------------------------*/
>  
>  #ifdef CONFIG_XFRM_OFFLOAD
> +/**
> + * bond_ipsec_dev - return the device for ipsec offload, or NULL if not exist
> + *                  caller must hold rcu_read_lock.
> + * @xs: pointer to transformer state struct
> + **/
> +static struct net_device bond_ipsec_dev(struct xfrm_state *xs)
> +{
> +	struct net_device *bond_dev = xs->xso.dev;
> +	struct net_device *real_dev;
> +	struct bonding *bond;
> +	struct slave *slave;
> +
> +	if (!bond_dev)
> +		return NULL;
> +
> +	bond = netdev_priv(bond_dev);
> +	slave = rcu_dereference(bond->curr_active_slave);
> +	real_dev = slave ? slave->dev : NULL;
> +
> +	if ((BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) ||
> +	    !slave || !real_dev || !xs->xso.real_dev)
> +		return NULL;

No need to check !slave again here.  !real_dev implies !slave and
vice-versa, if it is set then we must have had a slave.
I prefer the more obvious way - check slave after deref and
bail out, similar to my fix, I think it is easier to follow the
code and more obvious. Although I don't feel strong about that
it's just a preference. :)

> +
> +	WARN_ON(xs->xso.real_dev != slave->dev);
> +
> +	return real_dev;
> +}
> +
>  /**
>   * bond_ipsec_add_sa - program device with a security association
>   * @xs: pointer to transformer state struct
> @@ -595,23 +623,12 @@ static void bond_ipsec_del_sa_all(struct bonding *bond)
>   **/
>  static bool bond_ipsec_offload_ok(struct sk_buff *skb, struct xfrm_state *xs)
>  {
> -	struct net_device *bond_dev = xs->xso.dev;
>  	struct net_device *real_dev;
> -	struct slave *curr_active;
> -	struct bonding *bond;
>  	int err;
>  
> -	bond = netdev_priv(bond_dev);
>  	rcu_read_lock();
> -	curr_active = rcu_dereference(bond->curr_active_slave);
> -	real_dev = curr_active->dev;
> -
> -	if (BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) {
> -		err = false;
> -		goto out;
> -	}
> -
> -	if (!xs->xso.real_dev) {
> +	real_dev = bond_ipsec_dev(xs);
> +	if (!real_dev) {
>  		err = false;
>  		goto out;
>  	}

Re: [PATCHv2 net-next 2/3] bonding: Add ESN support to IPSec HW offload

[Nikolay Aleksandrov]

On 19/08/2024 10:53, 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.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.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 250a2717b4e9..3c04bdba17d4 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -646,10 +646,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);
> +		goto out;
> +	}
> +
> +	rhel_dev->xfrmdev_ops->xdo_dev_state_advance_esn(xs);
> +out:
> +	rcu_read_unlock();
> +}
> +
>  static const struct xfrmdev_ops bond_xfrmdev_ops = {
>  	.xdo_dev_state_add = bond_ipsec_add_sa,
>  	.xdo_dev_state_delete = bond_ipsec_del_sa,
>  	.xdo_dev_offload_ok = bond_ipsec_offload_ok,
> +	.xdo_dev_state_advance_esn = bond_advance_esn_state,
>  };
>  #endif /* CONFIG_XFRM_OFFLOAD */
>  

Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>

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

[Nikolay Aleksandrov]

On 19/08/2024 10:53, Hangbin Liu wrote:
> The patch add xfrm statistics update for bonding IPsec offload.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.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 3c04bdba17d4..9e41e34e9039 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -670,11 +670,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);
> +		goto out;
> +	}
> +
> +	real_dev->xfrmdev_ops->xdo_dev_state_update_stats(xs);
> +out:
> +	rcu_read_unlock();
> +}
> +
>  static const struct xfrmdev_ops bond_xfrmdev_ops = {
>  	.xdo_dev_state_add = bond_ipsec_add_sa,
>  	.xdo_dev_state_delete = bond_ipsec_del_sa,
>  	.xdo_dev_offload_ok = bond_ipsec_offload_ok,
>  	.xdo_dev_state_advance_esn = bond_advance_esn_state,
> +	.xdo_dev_state_update_stats = bond_xfrm_update_stats,
>  };
>  #endif /* CONFIG_XFRM_OFFLOAD */
>  


Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>

Re: [PATCHv2 net-next 0/2] Bonding: support new xfrm state offload functions

[Nikolay Aleksandrov]

On 19/08/2024 10:53, Hangbin Liu wrote:
> Add 2 new xfrm state offload functions xdo_dev_state_advance_esn and
> xdo_dev_state_update_stats for bonding. The xdo_dev_state_free will be
> added by Jianbo's patchset [1]. I will add the bonding xfrm policy offload
> in future.
> 
> I planned to add the new XFRM state offload functions after Jianbo's
> patchset [1], but it seems that may take some time. Therefore, I am
> posting these two patches to net-next now, as our users are waiting for
> this functionality. If Jianbo's patch is applied first, I can update these
> patches accordingly.
> 
> [1] https://lore.kernel.org/netdev/20240815142103.2253886-2-tariqt@nvidia.com
> 
> v2: Add a function to process the common device checking (Nikolay Aleksandrov)
>     Remove unused variable (Simon Horman)
> v1: lore.kernel.org/netdev/20240816035518.203704-1-liuhangbin@gmail.com
> 
> Hangbin Liu (3):
>   bonding: add common function to check ipsec device
>   bonding: Add ESN support to IPSec HW offload
>   bonding: support xfrm state update
> 
>  drivers/net/bonding/bond_main.c | 93 ++++++++++++++++++++++++++++-----
>  1 file changed, 80 insertions(+), 13 deletions(-)
> 


By the way just noticed $SUBJ says 0/2, but the patches are 3? :)

Re: [PATCHv2 net-next 1/3] bonding: add common function to check ipsec device

[Hangbin Liu]

On Mon, Aug 19, 2024 at 11:02:14AM +0300, Nikolay Aleksandrov wrote:
> > +static struct net_device bond_ipsec_dev(struct xfrm_state *xs)
> > +{
> > +	struct net_device *bond_dev = xs->xso.dev;
> > +	struct net_device *real_dev;
> > +	struct bonding *bond;
> > +	struct slave *slave;
> > +
> > +	if (!bond_dev)
> > +		return NULL;
> > +
> > +	bond = netdev_priv(bond_dev);
> > +	slave = rcu_dereference(bond->curr_active_slave);
> > +	real_dev = slave ? slave->dev : NULL;
> > +
> > +	if ((BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) ||
> > +	    !slave || !real_dev || !xs->xso.real_dev)
> > +		return NULL;
> 
> No need to check !slave again here.  !real_dev implies !slave and
> vice-versa, if it is set then we must have had a slave.

Ah yes, I missed this.

> I prefer the more obvious way - check slave after deref and
> bail out, similar to my fix, I think it is easier to follow the
> code and more obvious. Although I don't feel strong about that
> it's just a preference. :)

I don't have a inclination, I just want to check all the error and fail out.
If we check each one separately, do you think if I should do like

	if (!bond_dev)
		return NULL;

	bond = netdev_priv(bond_dev);
	if (BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP)
		return NULL;

	slave = rcu_dereference(bond->curr_active_slave);
	if (!slave)
		return NULL;

> > +	WARN_ON(xs->xso.real_dev != slave->dev);

Here as you said, the WARN_ON would be triggered easily, do you think if I
should change to pr_warn or salve_warn?

Thanks
Hangbin

Re: [PATCHv2 net-next 1/3] bonding: add common function to check ipsec device

[Nikolay Aleksandrov]

On 19/08/2024 11:43, Hangbin Liu wrote:
> On Mon, Aug 19, 2024 at 11:02:14AM +0300, Nikolay Aleksandrov wrote:
>>> +static struct net_device bond_ipsec_dev(struct xfrm_state *xs)
>>> +{
>>> +	struct net_device *bond_dev = xs->xso.dev;
>>> +	struct net_device *real_dev;
>>> +	struct bonding *bond;
>>> +	struct slave *slave;
>>> +
>>> +	if (!bond_dev)
>>> +		return NULL;
>>> +
>>> +	bond = netdev_priv(bond_dev);
>>> +	slave = rcu_dereference(bond->curr_active_slave);
>>> +	real_dev = slave ? slave->dev : NULL;
>>> +
>>> +	if ((BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) ||
>>> +	    !slave || !real_dev || !xs->xso.real_dev)
>>> +		return NULL;
>>
>> No need to check !slave again here.  !real_dev implies !slave and
>> vice-versa, if it is set then we must have had a slave.
> 
> Ah yes, I missed this.
> 

This is exactly my point about it being easier to follow if it's not all
combined in this way.

>> I prefer the more obvious way - check slave after deref and
>> bail out, similar to my fix, I think it is easier to follow the
>> code and more obvious. Although I don't feel strong about that
>> it's just a preference. :)
> 
> I don't have a inclination, I just want to check all the error and fail out.
> If we check each one separately, do you think if I should do like
> 
> 	if (!bond_dev)
> 		return NULL;
> 
> 	bond = netdev_priv(bond_dev);
> 	if (BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP)
> 		return NULL;
> 
> 	slave = rcu_dereference(bond->curr_active_slave);
> 	if (!slave)
> 		return NULL;
> 

I like this, even though it's more verbose, it's also easier to follow.
The alternative I have to track all code above and then verify the
combined check below. Depending on how complex it is, might be ok.
As I said it's a preference, if you prefer the other way - I don't mind.

>>> +	WARN_ON(xs->xso.real_dev != slave->dev);
> 
> Here as you said, the WARN_ON would be triggered easily, do you think if I
> should change to pr_warn or salve_warn?
> 

I haven't verified it, but yeah. Given how these are synced, it should be something
that's limited (we're talking about packets here, we shouldn't flood the logs) and
not a WARN_ON(). We can see any mid-state while changing.

> Thanks
> Hangbin

Cheers,
 Nik

Re: [PATCHv2 net-next 1/3] bonding: add common function to check ipsec device

[Simon Horman]

On Mon, Aug 19, 2024 at 03:53:31PM +0800, Hangbin Liu wrote:
> This patch adds a common function to check the status of IPSec devices.
> This function will be useful for future implementations, such as IPSec ESN
> and state offload callbacks.
> 
> Suggested-by: Nikolay Aleksandrov <razor@blackwall.org>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  drivers/net/bonding/bond_main.c | 43 +++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index f9633a6f8571..250a2717b4e9 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -418,6 +418,34 @@ static int bond_vlan_rx_kill_vid(struct net_device *bond_dev,
>  /*---------------------------------- XFRM -----------------------------------*/
>  
>  #ifdef CONFIG_XFRM_OFFLOAD
> +/**
> + * bond_ipsec_dev - return the device for ipsec offload, or NULL if not exist
> + *                  caller must hold rcu_read_lock.
> + * @xs: pointer to transformer state struct
> + **/
> +static struct net_device bond_ipsec_dev(struct xfrm_state *xs)

Hi Hangbin,

Based on the implementation of the function, it looks like it should return
'struct net_device *' rather than 'struct net_device' (a '*' is missing).

> +{
> +	struct net_device *bond_dev = xs->xso.dev;
> +	struct net_device *real_dev;
> +	struct bonding *bond;
> +	struct slave *slave;
> +
> +	if (!bond_dev)
> +		return NULL;
> +
> +	bond = netdev_priv(bond_dev);
> +	slave = rcu_dereference(bond->curr_active_slave);
> +	real_dev = slave ? slave->dev : NULL;
> +
> +	if ((BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) ||
> +	    !slave || !real_dev || !xs->xso.real_dev)
> +		return NULL;
> +
> +	WARN_ON(xs->xso.real_dev != slave->dev);
> +
> +	return real_dev;
> +}
> +
>  /**
>   * bond_ipsec_add_sa - program device with a security association
>   * @xs: pointer to transformer state struct

-- 
pw-bot: cr

Re: [PATCHv2 net-next 1/3] bonding: add common function to check ipsec device

[kernel test robot]

Hi Hangbin,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Hangbin-Liu/bonding-add-common-function-to-check-ipsec-device/20240819-195504
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240819075334.236334-2-liuhangbin%40gmail.com
patch subject: [PATCHv2 net-next 1/3] bonding: add common function to check ipsec device
config: x86_64-buildonly-randconfig-001-20240820 (https://download.01.org/0day-ci/archive/20240820/202408200327.ab8Ea0y8-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240820/202408200327.ab8Ea0y8-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408200327.ab8Ea0y8-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/net/bonding/bond_main.c:434:10: error: returning 'void *' from a function with incompatible result type 'struct net_device'
     434 |                 return NULL;
         |                        ^~~~
   include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
       8 | #define NULL ((void *)0)
         |              ^~~~~~~~~~~
   drivers/net/bonding/bond_main.c:442:10: error: returning 'void *' from a function with incompatible result type 'struct net_device'
     442 |                 return NULL;
         |                        ^~~~
   include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
       8 | #define NULL ((void *)0)
         |              ^~~~~~~~~~~
>> drivers/net/bonding/bond_main.c:446:9: error: returning 'struct net_device *' from a function with incompatible result type 'struct net_device'; dereference with *
     446 |         return real_dev;
         |                ^~~~~~~~
         |                *
>> drivers/net/bonding/bond_main.c:630:11: error: assigning to 'struct net_device *' from incompatible type 'struct net_device'
     630 |         real_dev = bond_ipsec_dev(xs);
         |                  ^ ~~~~~~~~~~~~~~~~~~
   4 errors generated.


vim +434 drivers/net/bonding/bond_main.c

   419	
   420	#ifdef CONFIG_XFRM_OFFLOAD
   421	/**
   422	 * bond_ipsec_dev - return the device for ipsec offload, or NULL if not exist
   423	 *                  caller must hold rcu_read_lock.
   424	 * @xs: pointer to transformer state struct
   425	 **/
   426	static struct net_device bond_ipsec_dev(struct xfrm_state *xs)
   427	{
   428		struct net_device *bond_dev = xs->xso.dev;
   429		struct net_device *real_dev;
   430		struct bonding *bond;
   431		struct slave *slave;
   432	
   433		if (!bond_dev)
 > 434			return NULL;
   435	
   436		bond = netdev_priv(bond_dev);
   437		slave = rcu_dereference(bond->curr_active_slave);
   438		real_dev = slave ? slave->dev : NULL;
   439	
   440		if ((BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) ||
   441		    !slave || !real_dev || !xs->xso.real_dev)
   442			return NULL;
   443	
   444		WARN_ON(xs->xso.real_dev != slave->dev);
   445	
 > 446		return real_dev;
   447	}
   448	
   449	/**
   450	 * bond_ipsec_add_sa - program device with a security association
   451	 * @xs: pointer to transformer state struct
   452	 * @extack: extack point to fill failure reason
   453	 **/
   454	static int bond_ipsec_add_sa(struct xfrm_state *xs,
   455				     struct netlink_ext_ack *extack)
   456	{
   457		struct net_device *bond_dev = xs->xso.dev;
   458		struct bond_ipsec *ipsec;
   459		struct bonding *bond;
   460		struct slave *slave;
   461		int err;
   462	
   463		if (!bond_dev)
   464			return -EINVAL;
   465	
   466		rcu_read_lock();
   467		bond = netdev_priv(bond_dev);
   468		slave = rcu_dereference(bond->curr_active_slave);
   469		if (!slave) {
   470			rcu_read_unlock();
   471			return -ENODEV;
   472		}
   473	
   474		if (!slave->dev->xfrmdev_ops ||
   475		    !slave->dev->xfrmdev_ops->xdo_dev_state_add ||
   476		    netif_is_bond_master(slave->dev)) {
   477			NL_SET_ERR_MSG_MOD(extack, "Slave does not support ipsec offload");
   478			rcu_read_unlock();
   479			return -EINVAL;
   480		}
   481	
   482		ipsec = kmalloc(sizeof(*ipsec), GFP_ATOMIC);
   483		if (!ipsec) {
   484			rcu_read_unlock();
   485			return -ENOMEM;
   486		}
   487		xs->xso.real_dev = slave->dev;
   488	
   489		err = slave->dev->xfrmdev_ops->xdo_dev_state_add(xs, extack);
   490		if (!err) {
   491			ipsec->xs = xs;
   492			INIT_LIST_HEAD(&ipsec->list);
   493			spin_lock_bh(&bond->ipsec_lock);
   494			list_add(&ipsec->list, &bond->ipsec_list);
   495			spin_unlock_bh(&bond->ipsec_lock);
   496		} else {
   497			kfree(ipsec);
   498		}
   499		rcu_read_unlock();
   500		return err;
   501	}
   502	
   503	static void bond_ipsec_add_sa_all(struct bonding *bond)
   504	{
   505		struct net_device *bond_dev = bond->dev;
   506		struct bond_ipsec *ipsec;
   507		struct slave *slave;
   508	
   509		rcu_read_lock();
   510		slave = rcu_dereference(bond->curr_active_slave);
   511		if (!slave)
   512			goto out;
   513	
   514		if (!slave->dev->xfrmdev_ops ||
   515		    !slave->dev->xfrmdev_ops->xdo_dev_state_add ||
   516		    netif_is_bond_master(slave->dev)) {
   517			spin_lock_bh(&bond->ipsec_lock);
   518			if (!list_empty(&bond->ipsec_list))
   519				slave_warn(bond_dev, slave->dev,
   520					   "%s: no slave xdo_dev_state_add\n",
   521					   __func__);
   522			spin_unlock_bh(&bond->ipsec_lock);
   523			goto out;
   524		}
   525	
   526		spin_lock_bh(&bond->ipsec_lock);
   527		list_for_each_entry(ipsec, &bond->ipsec_list, list) {
   528			ipsec->xs->xso.real_dev = slave->dev;
   529			if (slave->dev->xfrmdev_ops->xdo_dev_state_add(ipsec->xs, NULL)) {
   530				slave_warn(bond_dev, slave->dev, "%s: failed to add SA\n", __func__);
   531				ipsec->xs->xso.real_dev = NULL;
   532			}
   533		}
   534		spin_unlock_bh(&bond->ipsec_lock);
   535	out:
   536		rcu_read_unlock();
   537	}
   538	
   539	/**
   540	 * bond_ipsec_del_sa - clear out this specific SA
   541	 * @xs: pointer to transformer state struct
   542	 **/
   543	static void bond_ipsec_del_sa(struct xfrm_state *xs)
   544	{
   545		struct net_device *bond_dev = xs->xso.dev;
   546		struct bond_ipsec *ipsec;
   547		struct bonding *bond;
   548		struct slave *slave;
   549	
   550		if (!bond_dev)
   551			return;
   552	
   553		rcu_read_lock();
   554		bond = netdev_priv(bond_dev);
   555		slave = rcu_dereference(bond->curr_active_slave);
   556	
   557		if (!slave)
   558			goto out;
   559	
   560		if (!xs->xso.real_dev)
   561			goto out;
   562	
   563		WARN_ON(xs->xso.real_dev != slave->dev);
   564	
   565		if (!slave->dev->xfrmdev_ops ||
   566		    !slave->dev->xfrmdev_ops->xdo_dev_state_delete ||
   567		    netif_is_bond_master(slave->dev)) {
   568			slave_warn(bond_dev, slave->dev, "%s: no slave xdo_dev_state_delete\n", __func__);
   569			goto out;
   570		}
   571	
   572		slave->dev->xfrmdev_ops->xdo_dev_state_delete(xs);
   573	out:
   574		spin_lock_bh(&bond->ipsec_lock);
   575		list_for_each_entry(ipsec, &bond->ipsec_list, list) {
   576			if (ipsec->xs == xs) {
   577				list_del(&ipsec->list);
   578				kfree(ipsec);
   579				break;
   580			}
   581		}
   582		spin_unlock_bh(&bond->ipsec_lock);
   583		rcu_read_unlock();
   584	}
   585	
   586	static void bond_ipsec_del_sa_all(struct bonding *bond)
   587	{
   588		struct net_device *bond_dev = bond->dev;
   589		struct bond_ipsec *ipsec;
   590		struct slave *slave;
   591	
   592		rcu_read_lock();
   593		slave = rcu_dereference(bond->curr_active_slave);
   594		if (!slave) {
   595			rcu_read_unlock();
   596			return;
   597		}
   598	
   599		spin_lock_bh(&bond->ipsec_lock);
   600		list_for_each_entry(ipsec, &bond->ipsec_list, list) {
   601			if (!ipsec->xs->xso.real_dev)
   602				continue;
   603	
   604			if (!slave->dev->xfrmdev_ops ||
   605			    !slave->dev->xfrmdev_ops->xdo_dev_state_delete ||
   606			    netif_is_bond_master(slave->dev)) {
   607				slave_warn(bond_dev, slave->dev,
   608					   "%s: no slave xdo_dev_state_delete\n",
   609					   __func__);
   610			} else {
   611				slave->dev->xfrmdev_ops->xdo_dev_state_delete(ipsec->xs);
   612			}
   613			ipsec->xs->xso.real_dev = NULL;
   614		}
   615		spin_unlock_bh(&bond->ipsec_lock);
   616		rcu_read_unlock();
   617	}
   618	
   619	/**
   620	 * bond_ipsec_offload_ok - can this packet use the xfrm hw offload
   621	 * @skb: current data packet
   622	 * @xs: pointer to transformer state struct
   623	 **/
   624	static bool bond_ipsec_offload_ok(struct sk_buff *skb, struct xfrm_state *xs)
   625	{
   626		struct net_device *real_dev;
   627		int err;
   628	
   629		rcu_read_lock();
 > 630		real_dev = bond_ipsec_dev(xs);
   631		if (!real_dev) {
   632			err = false;
   633			goto out;
   634		}
   635	
   636		if (!real_dev->xfrmdev_ops ||
   637		    !real_dev->xfrmdev_ops->xdo_dev_offload_ok ||
   638		    netif_is_bond_master(real_dev)) {
   639			err = false;
   640			goto out;
   641		}
   642	
   643		err = real_dev->xfrmdev_ops->xdo_dev_offload_ok(skb, xs);
   644	out:
   645		rcu_read_unlock();
   646		return err;
   647	}
   648	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCHv2 net-next 1/3] bonding: add common function to check ipsec device

[kernel test robot]

Hi Hangbin,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Hangbin-Liu/bonding-add-common-function-to-check-ipsec-device/20240819-195504
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240819075334.236334-2-liuhangbin%40gmail.com
patch subject: [PATCHv2 net-next 1/3] bonding: add common function to check ipsec device
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20240820/202408200345.a4DQ8ecT-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240820/202408200345.a4DQ8ecT-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408200345.a4DQ8ecT-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/uapi/linux/posix_types.h:5,
                    from include/uapi/linux/types.h:14,
                    from include/linux/types.h:6,
                    from include/linux/kasan-checks.h:5,
                    from include/asm-generic/rwonce.h:26,
                    from ./arch/m68k/include/generated/asm/rwonce.h:1,
                    from include/linux/compiler.h:314,
                    from include/linux/array_size.h:5,
                    from include/linux/kernel.h:16,
                    from drivers/net/bonding/bond_main.c:35:
   drivers/net/bonding/bond_main.c: In function 'bond_ipsec_dev':
>> include/linux/stddef.h:8:14: error: incompatible types when returning type 'void *' but 'struct net_device' was expected
       8 | #define NULL ((void *)0)
         |              ^
   drivers/net/bonding/bond_main.c:434:24: note: in expansion of macro 'NULL'
     434 |                 return NULL;
         |                        ^~~~
>> include/linux/stddef.h:8:14: error: incompatible types when returning type 'void *' but 'struct net_device' was expected
       8 | #define NULL ((void *)0)
         |              ^
   drivers/net/bonding/bond_main.c:442:24: note: in expansion of macro 'NULL'
     442 |                 return NULL;
         |                        ^~~~
>> drivers/net/bonding/bond_main.c:446:16: error: incompatible types when returning type 'struct net_device *' but 'struct net_device' was expected
     446 |         return real_dev;
         |                ^~~~~~~~
   drivers/net/bonding/bond_main.c: In function 'bond_ipsec_offload_ok':
>> drivers/net/bonding/bond_main.c:630:20: error: incompatible types when assigning to type 'struct net_device *' from type 'struct net_device'
     630 |         real_dev = bond_ipsec_dev(xs);
         |                    ^~~~~~~~~~~~~~


vim +8 include/linux/stddef.h

^1da177e4c3f41 Linus Torvalds   2005-04-16  6  
^1da177e4c3f41 Linus Torvalds   2005-04-16  7  #undef NULL
^1da177e4c3f41 Linus Torvalds   2005-04-16 @8  #define NULL ((void *)0)
6e218287432472 Richard Knutsson 2006-09-30  9  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCHv2 net-next 2/3] bonding: Add ESN support to IPSec HW offload

[kernel test robot]

Hi Hangbin,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Hangbin-Liu/bonding-add-common-function-to-check-ipsec-device/20240819-195504
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240819075334.236334-3-liuhangbin%40gmail.com
patch subject: [PATCHv2 net-next 2/3] bonding: Add ESN support to IPSec HW offload
config: x86_64-buildonly-randconfig-001-20240820 (https://download.01.org/0day-ci/archive/20240820/202408200431.wjjkEZ2m-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240820/202408200431.wjjkEZ2m-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408200431.wjjkEZ2m-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/net/bonding/bond_main.c:434:10: error: returning 'void *' from a function with incompatible result type 'struct net_device'
     434 |                 return NULL;
         |                        ^~~~
   include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
       8 | #define NULL ((void *)0)
         |              ^~~~~~~~~~~
   drivers/net/bonding/bond_main.c:442:10: error: returning 'void *' from a function with incompatible result type 'struct net_device'
     442 |                 return NULL;
         |                        ^~~~
   include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
       8 | #define NULL ((void *)0)
         |              ^~~~~~~~~~~
   drivers/net/bonding/bond_main.c:446:9: error: returning 'struct net_device *' from a function with incompatible result type 'struct net_device'; dereference with *
     446 |         return real_dev;
         |                ^~~~~~~~
         |                *
   drivers/net/bonding/bond_main.c:630:11: error: assigning to 'struct net_device *' from incompatible type 'struct net_device'
     630 |         real_dev = bond_ipsec_dev(xs);
         |                  ^ ~~~~~~~~~~~~~~~~~~
   drivers/net/bonding/bond_main.c:658:11: error: assigning to 'struct net_device *' from incompatible type 'struct net_device'
     658 |         real_dev = bond_ipsec_dev(xs);
         |                  ^ ~~~~~~~~~~~~~~~~~~
>> drivers/net/bonding/bond_main.c:668:2: error: use of undeclared identifier 'rhel_dev'; did you mean 'real_dev'?
     668 |         rhel_dev->xfrmdev_ops->xdo_dev_state_advance_esn(xs);
         |         ^~~~~~~~
         |         real_dev
   drivers/net/bonding/bond_main.c:655:21: note: 'real_dev' declared here
     655 |         struct net_device *real_dev;
         |                            ^
   6 errors generated.


vim +668 drivers/net/bonding/bond_main.c

   648	
   649	/**
   650	 * bond_advance_esn_state - ESN support for IPSec HW offload
   651	 * @xs: pointer to transformer state struct
   652	 **/
   653	static void bond_advance_esn_state(struct xfrm_state *xs)
   654	{
   655		struct net_device *real_dev;
   656	
   657		rcu_read_lock();
   658		real_dev = bond_ipsec_dev(xs);
   659		if (!real_dev)
   660			goto out;
   661	
   662		if (!real_dev->xfrmdev_ops ||
   663		    !real_dev->xfrmdev_ops->xdo_dev_state_advance_esn) {
   664			pr_warn("%s: %s doesn't support xdo_dev_state_advance_esn\n", __func__, real_dev->name);
   665			goto out;
   666		}
   667	
 > 668		rhel_dev->xfrmdev_ops->xdo_dev_state_advance_esn(xs);
   669	out:
   670		rcu_read_unlock();
   671	}
   672	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCHv2 net-next 2/3] bonding: Add ESN support to IPSec HW offload

[Hangbin Liu]

On Tue, Aug 20, 2024 at 05:17:52AM +0800, kernel test robot wrote:
> Hi Hangbin,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on net-next/main]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Hangbin-Liu/bonding-add-common-function-to-check-ipsec-device/20240819-195504
> base:   net-next/main
> patch link:    https://lore.kernel.org/r/20240819075334.236334-3-liuhangbin%40gmail.com
> patch subject: [PATCHv2 net-next 2/3] bonding: Add ESN support to IPSec HW offload
> config: x86_64-buildonly-randconfig-001-20240820 (https://download.01.org/0day-ci/archive/20240820/202408200431.wjjkEZ2m-lkp@intel.com/config)
> compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240820/202408200431.wjjkEZ2m-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202408200431.wjjkEZ2m-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/net/bonding/bond_main.c:434:10: error: returning 'void *' from a function with incompatible result type 'struct net_device'
>      434 |                 return NULL;
>          |                        ^~~~
>    include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
>        8 | #define NULL ((void *)0)
>          |              ^~~~~~~~~~~
>    drivers/net/bonding/bond_main.c:442:10: error: returning 'void *' from a function with incompatible result type 'struct net_device'
>      442 |                 return NULL;
>          |                        ^~~~
>    include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
>        8 | #define NULL ((void *)0)
>          |              ^~~~~~~~~~~
>    drivers/net/bonding/bond_main.c:446:9: error: returning 'struct net_device *' from a function with incompatible result type 'struct net_device'; dereference with *
>      446 |         return real_dev;
>          |                ^~~~~~~~
>          |                *
>    drivers/net/bonding/bond_main.c:630:11: error: assigning to 'struct net_device *' from incompatible type 'struct net_device'
>      630 |         real_dev = bond_ipsec_dev(xs);
>          |                  ^ ~~~~~~~~~~~~~~~~~~
>    drivers/net/bonding/bond_main.c:658:11: error: assigning to 'struct net_device *' from incompatible type 'struct net_device'
>      658 |         real_dev = bond_ipsec_dev(xs);
>          |                  ^ ~~~~~~~~~~~~~~~~~~
> >> drivers/net/bonding/bond_main.c:668:2: error: use of undeclared identifier 'rhel_dev'; did you mean 'real_dev'?

Hmm, weird... I tried to build the patch via `vng` before post, with
CONFIG_XFRM_OFFLOAD=y in tools/testing/selftests/drivers/net/bonding/config.
But looks it's not build in. Next time I need to double check..

Thanks
Hangbin

Re: [PATCHv2 net-next 1/3] bonding: add common function to check ipsec device

[Hangbin Liu]

On Mon, Aug 19, 2024 at 03:37:53PM +0100, Simon Horman wrote:
> > +static struct net_device bond_ipsec_dev(struct xfrm_state *xs)
> 
> Hi Hangbin,
> 
> Based on the implementation of the function, it looks like it should return
> 'struct net_device *' rather than 'struct net_device' (a '*' is missing).

Sigh, I noticed this but forgot the update the patch.. The build also passed
due to missing CONFIG_XFRM_OFFLOAD=y, even I added CONFIG_XFRM_OFFLOAD=y to
config file. I need to check the reason.

Hangbin