Re: [PATCH ipsec-next v8 12/14] xfrm: add XFRM_MSG_MIGRATE_STATE for single SA migration

[Steffen Klassert <steffen.klassert@secunet.com>]

On Tue, May 05, 2026 at 06:34:29AM +0200, Antony Antony wrote:
> Add a new netlink method to migrate a single xfrm_state.
> Unlike the existing migration mechanism (SA + policy), this
> supports migrating only the SA and allows changing the reqid.
> 
> The SA is looked up via xfrm_usersa_id, which uniquely
> identifies it, so old_saddr is not needed. old_daddr is carried in
> xfrm_usersa_id.daddr.
> 
> The reqid is invariant in the old migration.
> 
> Signed-off-by: Antony Antony <antony.antony@secunet.com>
> 
> ---
> v7->v8: - removed the unknown-flags validation block
> v6->v7: - add flags field to xfrm_user_migrate_state (based on Sabrina's feedback)
>   - add XFRM_MIGRATE_STATE_NO_OFFLOAD (bit 0): suppresses offload
>   - omit-to-inherit; mutually exclusive with XFRMA_OFFLOAD_DEV
>   - zero-initialize struct xfrm_migrate m[XFRM_MAX_DEPTH]
>   - add struct xfrm_selector new_sel to xfrm_user_migrate_state
>   - add XFRM_MIGRATE_STATE_UPDATE_SEL: derive new selector
>     from SA addresses when old selector is a single-host match
> v5->v6: - (Feedback from Sabrina's review)
>   - reqid change: use xfrm_state_add, not xfrm_state_insert
>   - encap and xuo: use nla_data() directly, no kmemdup needed
>   - notification failure is non-fatal: set extack warning, return 0
>   - drop state direction, x->dir, check, not required
>   - reverse xmas tree local variable ordering
>   - use NL_SET_ERR_MSG_WEAK for clone failure message
>   - fix implicit padding in xfrm_user_migrate_state uapi struct
>   - support XFRMA_SET_MARK/XFRMA_SET_MARK_MASK in XFRM_MSG_MIGRATE_STATE
> v4->v5: - set portid, seq in XFRM_MSG_MIGRATE_STATE netlink notification
>   - rename error label to out for clarity
>   - add locking and synchronize after cloning
>   - change some if(x) to if(!x) for clarity
>   - call __xfrm_state_delete() inside the lock
>   - return error from xfrm_send_migrate_state() instead of always returning 0
> v3->v4: preserve reqid invariant for each state migrated
> v2->v3: free the skb on the error path
> v1->v2: merged next patch here to fix use uninitialized value
>   - removed unnecessary inline
>   - added const when possible
> ---
>  include/net/xfrm.h          |  16 ++-
>  include/uapi/linux/xfrm.h   |  21 ++++
>  net/xfrm/xfrm_device.c      |   2 +-
>  net/xfrm/xfrm_policy.c      |  19 +++
>  net/xfrm/xfrm_state.c       |  29 +++--
>  net/xfrm/xfrm_user.c        | 281 +++++++++++++++++++++++++++++++++++++++++++-
>  security/selinux/nlmsgtab.c |   3 +-
>  7 files changed, 357 insertions(+), 14 deletions(-)

...

> +static unsigned int xfrm_migrate_state_msgsize(const struct xfrm_migrate *m,
> +					       u8 dir)
> +{
> +	return NLMSG_ALIGN(sizeof(struct xfrm_user_migrate_state)) +
> +		(m->encap ? nla_total_size(sizeof(struct xfrm_encap_tmpl)) : 0) +
> +		(m->xuo ? nla_total_size(sizeof(struct xfrm_user_offload)) : 0) +
> +		(m->new_mark ? nla_total_size(sizeof(struct xfrm_mark)) : 0) +
> +		(m->smark.v ? nla_total_size(sizeof(u32)) * 2 : 0) + /* SET_MARK + SET_MARK_MASK */

xfrm_smark_put() checks (m->v | m->m), maybe you should
do (m->smark.v | m->smark.m) here.

> +		(m->mapping_maxage ? nla_total_size(sizeof(u32)) : 0) +
> +		(m->nat_keepalive_interval ? nla_total_size(sizeof(u32)) : 0) +
> +		(dir ? nla_total_size(sizeof(u8)) : 0); /* XFRMA_SA_DIR */
> +}

Also, the function is not really readable.

> +
> +static int xfrm_send_migrate_state(const struct xfrm_user_migrate_state *um,
> +				   const struct xfrm_migrate *m,
> +				   u8 dir, u32 portid, u32 seq)
> +{
> +	int err;
> +	struct sk_buff *skb;
> +	struct net *net = &init_net;

This is wrong. I know we had this in the tree for ages, but I now have
a fix in ipsec/testing for it. We need to make this namespace aware.