Re: [PATCH ipsec v1 2/2] xfrm: Migrate offload configuration

[Simon Horman <horms@kernel.org>]

On Wed, Jan 22, 2025 at 12:09:41PM +0000, Chiachang Wang wrote:
> If the SA contains offload configuration, the migration
> path should update the SA as well.
> 
> This change supports SA migration with the offload attribute
> configured. This allows the device to migrate with offload
> configuration.
> 
> Test: Endable both in/out IPSec crypto offload, and verify
>       with Android device on both WiFi/cellular network,
>       including:
>       1. WiFi + offload -> Cellular + offload
>       2. WiFi + offload -> Cellular + no offload
>       3. WiFi + no offload -> Cellular + offload
>       4. Wifi + no offload -> Cellular + no offload
>       5. Cellular + offload -> WiFi + offload
>       6. Cellular + no offload -> WiFi + offload
>       7. Cellular + offload -> WiFi + no offload
>       8. Cell + no offload -> WiFi + no offload
> Signed-off-by: Chiachang Wang <chiachangwang@google.com>

...

> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index 46d75980eb2e..2fdb4ea97844 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -2007,22 +2007,30 @@ EXPORT_SYMBOL(xfrm_migrate_state_find);
>  
>  struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x,
>  				      struct xfrm_migrate *m,
> -				      struct xfrm_encap_tmpl *encap)
> +				      struct xfrm_encap_tmpl *encap,
> +				      struct net *net,
> +				      struct xfrm_user_offload *xuo,
> +				      struct netlink_ext_ack *extack)
>  {
>  	struct xfrm_state *xc;
> -
> +	bool offload = (xuo);
>  	xc = xfrm_state_clone(x, encap);
>  	if (!xc)
>  		return NULL;
>  
>  	xc->props.family = m->new_family;
>  
> -	if (xfrm_init_state(xc) < 0)
> +	if (__xfrm_init_state(xc, true, offload, NULL) < 0)
>  		goto error;
>  
> +	x->km.state = XFRM_STATE_VALID;
>  	memcpy(&xc->id.daddr, &m->new_daddr, sizeof(xc->id.daddr));
>  	memcpy(&xc->props.saddr, &m->new_saddr, sizeof(xc->props.saddr));
>  
> +	/* configure the hardware if offload is requested */
> +	if (offload & xfrm_dev_state_add(net, xc, xuo, extack))

Hi Chiachang Wang,

This looks like it is intended to be a logical and (&&)
rather than a bitwise and (&).

Flagged by Smatch.

> +		goto error;
> +
>  	/* add state */
>  	if (xfrm_addr_equal(&x->id.daddr, &m->new_daddr, m->new_family)) {
>  		/* a care is needed when the destination address of the