Thread (30 messages) 30 messages, 4 authors, 2026-04-07

Re: [devel-ipsec] Re: [PATCH ipsec-next v5 8/8] xfrm: add XFRM_MSG_MIGRATE_STATE for single SA migration

From: Antony Antony <hidden>
Date: 2026-03-10 16:52:08
Also in: lkml, selinux

On Tue, Mar 10, 2026 at 12:09:27PM +0100, Sabrina Dubroca wrote:
2026-03-08, 15:42:58 +0100, Antony Antony wrote:
quoted
On Fri, Feb 27, 2026 at 03:14:21PM -0800, Yan Yan via Devel wrote:
quoted
quoted
Anything that we leave as implicit copy will have to be "forever"
implicitly copied with this new MIGRATE_STATE op -- unless we find a
way to pass a new "clear these properties" flag (probably via a list
of XFRMA_* attribute names)
that is a limitation we should avoid. It would be nice to extend it
over time. We have been there before and it is a pain point. So it is
worth investigating alternatives if there is momentum here, otherwise
I would keep it simple:)
Well, because it's a known pain point, we shouldn't just jump into an
implementation.

(FWIW, as a kernel-only developer, ie not involved at all in the *swan
code, I don't have much of an opinion on which property should behave
which way, just that we know what we're getting into)
Understandable. Partly *swan world, Android, IETF RFC guidelines, NIST et 
al. The *swan world usually wakes late after the time the kernel
reaches distributions.This is too late to fix kernel without breaking ABI.
standards world has its own path. Kernel often implements that what
makes practical sense in the moment and hard depreciate/remove ABI:)

I feel these days there are more interactions such as this, which is a good 
sign!
quoted
quoted
That is true. I also have the concern that if all updatable attributes
follow the "omit-to-clear" pattern, we lose the extensibility. Thus
ideally we should switch to an "omit-to-inherit" model for some, if
not all, attributes to ensure that adding new SA properties doesn't
break backward compatibility.
Implicit copy ("omit-to-inherit") gets us in the situation we have
with the old MIGRATE code, we don't have a way to _not inherit_
properties. Well, apparently we do, based on what Antony wrote below.


quoted
Here is my proposal. I extended the code and am testing it now; I hope
to send out v6 soon.
I think it would have been nice to postpone v6 a little bit so that
others had time to answer here (and avoid a respin if there's some
disagreement on what the behavior should be).
quoted
How would omit-to-inherit look in practice? Specify almost all XFRMA
attributes supported in XFRM_MSG_NEWSA, minus some immutable ones.
The immutable attributes that come to mind are:

    - XFRMA_ALG_*      : crypto must not change during the life of an SA;
                         also *swan userspace does not keep this in memory
                         after the SA is installed, which is correct
                         behaviour.
    - XFRMA_SA_DIR     : direction is fixed at SA creation.
    - XFRMA_SEC_CTX    : security context is immutable.
So we should be rejecting an attempt to pass those attributes in a
MIGRATE_STATE request.
yes. I added it o v6.It reject all unused attributes.  
NL_SET_ERR_MSG_ATTR(extack, attrs[i], "Unsupported attribute in 
XFRM_MSG_MIGRATE_STATE");
quoted
currently supported attributes, using omit-to-inherit semantics:

    sentinel value to clear, omit to inherit:
    - XFRMA_ENCAP              : encap_type=0 to clear
That would be a new special case value for that attribute, ok.
quoted
    - XFRMA_OFFLOAD_DEV        : ifindex=0 to clear
OFFLOAD_DEV with xuo.ifindex=0 is a valid attribute to request offload
and let the kernel figure out which device to use. We can't use that
to clear/disable offload of an SA.
Good catch, thanks.

Two options to fix this:

Option 1: add XFRM_OFFLOAD_CLEAR to xfrm_user_offload flags in uapi xfrm.h:

#define XFRM_OFFLOAD_CLEAR  (1 << 7)
When set in XFRMA_OFFLOAD_DEV, it means remove offload rather than configure it.

Option 2: add a __u32 flags field to xfrm_user_migrate_state in uapi xfrm.h.
There is a __u16 reserved currently used for alignment, but 16 bits feels
too small if we want to cover clearing other attributes in the future.
A __u32 at the end of the struct avoids that constraint.

I am leaning toward option 2. Any preference? 

For the rest, it seems that passing 0 is equivalent to omitting the
attribute in the current code. Except XFRMA_SA_PCPU, where we consider
UINT_MAX as "disable" (default value for x->pcpu_num), but we reject
userspace passing us that value, and XFRMA_SA_PCPU containing 0 is a
valid value.
yes XFRMA_SA_PCPU that would be UINT_MAX.

-antony
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help