Re: [PATCH ipsec-next v4 2/2] xfrm: Refactor migration setup during the cloning process

[Leon Romanovsky <leon@kernel.org>]

On Mon, Mar 10, 2025 at 09:16:20AM +0000, Chiachang Wang wrote:
> Previously, migration related setup, such as updating family,
> destination address, and source address, was performed after
> the clone was created in `xfrm_state_migrate`. This change
> moves this setup into the cloning function itself, improving
> code locality and reducing redundancy.
> 
> The `xfrm_state_clone_and_setup` function now conditionally
> applies the migration parameters from struct xfrm_migrate
> if it is provided. This allows the function to be used both
> for simple cloning and for cloning with migration setup.
> 
> Test: Tested with kernel test in the Android tree located
>       in https://android.googlesource.com/kernel/tests/
>       The xfrm_tunnel_test.py under the tests folder in
>       particular.
> Signed-off-by: Chiachang Wang <chiachangwang@google.com>
> ---
>  net/xfrm/xfrm_state.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index 9cd707362767..0365daedea32 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -1958,8 +1958,9 @@ static inline int clone_security(struct xfrm_state *x, struct xfrm_sec_ctx *secu
>  	return 0;
>  }
> 
> -static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig,
> -					   struct xfrm_encap_tmpl *encap)
> +static struct xfrm_state *xfrm_state_clone_and_setup(struct xfrm_state *orig,
> +					   struct xfrm_encap_tmpl *encap,
> +					   struct xfrm_migrate *m)
>  {
>  	struct net *net = xs_net(orig);
>  	struct xfrm_state *x = xfrm_state_alloc(net);
> @@ -2058,6 +2059,12 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig,
>  			goto error;
>  	}
> 
> +	if (m) {

Why do you need this "if (m)"? "m" should be valid at this stage.

Thanks

> +		x->props.family = m->new_family;
> +		memcpy(&x->id.daddr, &m->new_daddr, sizeof(x->id.daddr));
> +		memcpy(&x->props.saddr, &m->new_saddr, sizeof(x->props.saddr));
> +	}
> +
>  	return x;