[PATCH ipsec-next v4 0/2] Update offload configuration with SA

[PATCH ipsec-next v4 0/2] Update offload configuration with SA

[Chiachang Wang]

The current Security Association (SA) offload setting
cannot be modified without removing and re-adding the
SA with the new configuration. Although existing netlink
messages allow SA migration, the offload setting will
be removed after migration.

This patchset enhances SA migration to include updating
the offload setting. This is beneficial for devices that
support IPsec session management.

Chiachang Wang (2):
  xfrm: Migrate offload configuration
  xfrm: Refactor migration setup during the cloning process

 include/net/xfrm.h     |  8 ++++++--
 net/key/af_key.c       |  2 +-
 net/xfrm/xfrm_policy.c |  4 ++--
 net/xfrm/xfrm_state.c  | 25 +++++++++++++++++--------
 net/xfrm/xfrm_user.c   | 15 ++++++++++++---
 5 files changed, 38 insertions(+), 16 deletions(-)

---
v3 -> v4:
 - Change the target tree to ipsec-next
 - Rebase commit to adopt updated xfrm_init_state()
 - Remove redundant variable to rely on validiaty of pointer
 - Refactor xfrm_migrate copy into xfrm_state_clone and rename the
   method
v2 -> v3:
 - Update af_key.c to address kbuild error
v1 -> v2:
 - Revert "xfrm: Update offload configuration during SA update"
   change as the patch can be protentially handled in the
   hardware without the change.
 - Address review feedback to correct the logic in the
   xfrm_state_migrate in the migration offload configuration
   change.
 - Revise the commit message for "xfrm: Migrate offload configuration"
--
2.49.0.rc0.332.g42c0ae87b1-goog

[PATCH ipsec-next v4 1/2] xfrm: Migrate offload configuration

[Chiachang Wang]

Add hardware offload configuration to XFRM_MSG_MIGRATE
using an option netlink attribute XFRMA_OFFLOAD_DEV.

In the existing xfrm_state_migrate(), the xfrm_init_state()
is called assuming no hardware offload by default. Even the
original xfrm_state is configured with offload, the setting will
be reset. If the device is configured with hardware offload,
it's reasonable to allow the device to maintain its hardware
offload mode. But the device will end up with offload disabled
after receiving a migration event when the device migrates the
connection from one netdev to another one.

The devices that support migration may work with different
underlying networks, such as mobile devices. The hardware setting
should be forwarded to the different netdev based on the
migration configuration. This change provides the capability
for user space to migrate from one netdev to another.

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>
---
 v3 -> v4:
 - Change the target tree to ipsec-next
 - Rebase commit to adopt updated xfrm_init_state()
 - Remove redundant variable to rely on validiaty of pointer
 v2 -> v3:
 - Modify af_key to fix kbuild error
 v1 -> v2:
 - Address review feedback to correct the logic in the
   xfrm_state_migrate in the migration offload configuration
   change
 - Revise the commit message for "xfrm: Migrate offload configuration"
---
 include/net/xfrm.h     |  8 ++++++--
 net/key/af_key.c       |  2 +-
 net/xfrm/xfrm_policy.c |  4 ++--
 net/xfrm/xfrm_state.c  |  9 ++++++++-
 net/xfrm/xfrm_user.c   | 15 ++++++++++++---
 5 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 39365fd2ea17..f80cdef43ed5 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1893,12 +1893,16 @@ struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *n
 						u32 if_id);
 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);
 int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 		 struct xfrm_migrate *m, int num_bundles,
 		 struct xfrm_kmaddress *k, struct net *net,
 		 struct xfrm_encap_tmpl *encap, u32 if_id,
-		 struct netlink_ext_ack *extack);
+		 struct netlink_ext_ack *extack,
+		 struct xfrm_user_offload *xuo);
 #endif

 int km_new_mapping(struct xfrm_state *x, xfrm_address_t *ipaddr, __be16 sport);
diff --git a/net/key/af_key.c b/net/key/af_key.c
index c56bb4f451e6..efc2a91f4c48 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2630,7 +2630,7 @@ static int pfkey_migrate(struct sock *sk, struct sk_buff *skb,
 	}

 	return xfrm_migrate(&sel, dir, XFRM_POLICY_TYPE_MAIN, m, i,
-			    kma ? &k : NULL, net, NULL, 0, NULL);
+			    kma ? &k : NULL, net, NULL, 0, NULL, NULL);

  out:
 	return err;
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 6551e588fe52..82f755e39110 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -4630,7 +4630,7 @@ int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 		 struct xfrm_migrate *m, int num_migrate,
 		 struct xfrm_kmaddress *k, struct net *net,
 		 struct xfrm_encap_tmpl *encap, u32 if_id,
-		 struct netlink_ext_ack *extack)
+		 struct netlink_ext_ack *extack, struct xfrm_user_offload *xuo)
 {
 	int i, err, nx_cur = 0, nx_new = 0;
 	struct xfrm_policy *pol = NULL;
@@ -4663,7 +4663,7 @@ int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 		if ((x = xfrm_migrate_state_find(mp, net, if_id))) {
 			x_cur[nx_cur] = x;
 			nx_cur++;
-			xc = xfrm_state_migrate(x, mp, encap);
+			xc = xfrm_state_migrate(x, mp, encap, net, xuo, extack);
 			if (xc) {
 				x_new[nx_new] = xc;
 				nx_new++;
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 7b1028671144..9cd707362767 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2120,7 +2120,10 @@ 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;

@@ -2136,6 +2139,10 @@ struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x,
 	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 (xuo && xfrm_dev_state_add(net, xc, xuo, extack))
+		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
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 784a2d124749..85383d15d003 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -3069,6 +3069,7 @@ static int xfrm_do_migrate(struct sk_buff *skb, struct nlmsghdr *nlh,
 	int n = 0;
 	struct net *net = sock_net(skb->sk);
 	struct xfrm_encap_tmpl  *encap = NULL;
+	struct xfrm_user_offload *xuo = NULL;
 	u32 if_id = 0;

 	if (!attrs[XFRMA_MIGRATE]) {
@@ -3099,11 +3100,19 @@ static int xfrm_do_migrate(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (attrs[XFRMA_IF_ID])
 		if_id = nla_get_u32(attrs[XFRMA_IF_ID]);

+	if (attrs[XFRMA_OFFLOAD_DEV]) {
+		xuo = kmemdup(nla_data(attrs[XFRMA_OFFLOAD_DEV]),
+			      sizeof(*xuo), GFP_KERNEL);
+		if (!xuo) {
+			err = -ENOMEM;
+			goto error;
+		}
+	}
 	err = xfrm_migrate(&pi->sel, pi->dir, type, m, n, kmp, net, encap,
-			   if_id, extack);
-
+			   if_id, extack, xuo);
+error:
 	kfree(encap);
-
+	kfree(xuo);
 	return err;
 }
 #else
--
2.49.0.rc0.332.g42c0ae87b1-goog

Re: [PATCH ipsec-next v4 1/2] xfrm: Migrate offload configuration

[Leon Romanovsky]

On Mon, Mar 10, 2025 at 09:16:19AM +0000, Chiachang Wang wrote:
> Add hardware offload configuration to XFRM_MSG_MIGRATE
> using an option netlink attribute XFRMA_OFFLOAD_DEV.
> 
> In the existing xfrm_state_migrate(), the xfrm_init_state()
> is called assuming no hardware offload by default. Even the
> original xfrm_state is configured with offload, the setting will
> be reset. If the device is configured with hardware offload,
> it's reasonable to allow the device to maintain its hardware
> offload mode. But the device will end up with offload disabled
> after receiving a migration event when the device migrates the
> connection from one netdev to another one.
> 
> The devices that support migration may work with different
> underlying networks, such as mobile devices. The hardware setting
> should be forwarded to the different netdev based on the
> migration configuration. This change provides the capability
> for user space to migrate from one netdev to another.
> 
> 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>
> ---
>  v3 -> v4:
>  - Change the target tree to ipsec-next
>  - Rebase commit to adopt updated xfrm_init_state()
>  - Remove redundant variable to rely on validiaty of pointer
>  v2 -> v3:
>  - Modify af_key to fix kbuild error
>  v1 -> v2:
>  - Address review feedback to correct the logic in the
>    xfrm_state_migrate in the migration offload configuration
>    change
>  - Revise the commit message for "xfrm: Migrate offload configuration"
> ---
>  include/net/xfrm.h     |  8 ++++++--
>  net/key/af_key.c       |  2 +-
>  net/xfrm/xfrm_policy.c |  4 ++--
>  net/xfrm/xfrm_state.c  |  9 ++++++++-
>  net/xfrm/xfrm_user.c   | 15 ++++++++++++---
>  5 files changed, 29 insertions(+), 9 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

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

[Chiachang Wang]

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) {
+		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;

  error:
@@ -2127,18 +2134,13 @@ struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x,
 {
 	struct xfrm_state *xc;

-	xc = xfrm_state_clone(x, encap);
+	xc = xfrm_state_clone_and_setup(x, encap, m);
 	if (!xc)
 		return NULL;

-	xc->props.family = m->new_family;
-
 	if (xfrm_init_state(xc) < 0)
 		goto error;

-	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 (xuo && xfrm_dev_state_add(net, xc, xuo, extack))
 		goto error;
--
2.49.0.rc0.332.g42c0ae87b1-goog

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

[Leon Romanovsky]

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;

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

[Chiachang Wang]

While the xfrm_state_migrate() is the only caller for this method
currently, this check can be removed indeed.
I add this for the feasibility of other callers without performing the
validation. If you have a strong opinion on this. I can update to
remove this.
Please let me know if you prefer to do so.

Thank you!

Leon Romanovsky <leon@kernel.org> 於 2025年3月10日 週一 下午7:52寫道：
>
> 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;

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

[Leon Romanovsky]

On Mon, Mar 10, 2025 at 08:20:39PM +0800, Chiachang Wang wrote:
> While the xfrm_state_migrate() is the only caller for this method
> currently, this check can be removed indeed.
> I add this for the feasibility of other callers without performing the
> validation. If you have a strong opinion on this. I can update to
> remove this.
> Please let me know if you prefer to do so.

Sure, please remove. We are adding code when it is actually needed.

Thanks

> 
> Thank you!
> 
> Leon Romanovsky <leon@kernel.org> 於 2025年3月10日 週一 下午7:52寫道：
> >
> > 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;
>