[PATCH ipsec 0/3] xfrm: some fixes for GSO with SW crypto

[PATCH ipsec 0/3] xfrm: some fixes for GSO with SW crypto

[Sabrina Dubroca]

This series fixes a few issues with GSO. Some recent patches made the
incorrect assumption that GSO is only used by offload. The first two
patches in this series restores the old behavior.

The final patch is in the UDP GSO code, but fixes an issue with IPsec
that is currently masked by the lack of GSO for SW crypto. With GSO,
VXLAN over IPsec doesn't get checksummed.

Sabrina Dubroca (3):
  xfrm: restore GSO for SW crypto
  Revert "xfrm: Remove unneeded device check from validate_xmit_xfrm"
  udp: also consider secpath when evaluating ipsec use for checksumming

 net/ipv4/udp_offload.c |  2 +-
 net/xfrm/xfrm_device.c | 16 +++++++++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

-- 
2.50.0

[PATCH ipsec 1/3] xfrm: restore GSO for SW crypto

[Sabrina Dubroca]

Commit 49431af6c4ef incorrectly assumes that the GSO path is only used
by HW offload, but it's also useful for SW crypto.

This patch re-enables GSO for SW crypto. It's not an exact revert to
preserve the other changes made to xfrm_dev_offload_ok afterwards, but
it reverts all of its effects.

Fixes: 49431af6c4ef ("xfrm: rely on XFRM offload")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/xfrm/xfrm_device.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index d2819baea414..1f88472aaac0 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -415,10 +415,12 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
 	struct net_device *dev = x->xso.dev;
 	bool check_tunnel_size;
 
-	if (x->xso.type == XFRM_DEV_OFFLOAD_UNSPECIFIED)
+	if (!x->type_offload ||
+	    (x->xso.type == XFRM_DEV_OFFLOAD_UNSPECIFIED && x->encap))
 		return false;
 
-	if ((dev == xfrm_dst_path(dst)->dev) && !xdst->child->xfrm) {
+	if ((!dev || dev == xfrm_dst_path(dst)->dev) &&
+	    !xdst->child->xfrm) {
 		mtu = xfrm_state_mtu(x, xdst->child_mtu_cached);
 		if (skb->len <= mtu)
 			goto ok;
@@ -430,6 +432,9 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
 	return false;
 
 ok:
+	if (!dev)
+		return true;
+
 	check_tunnel_size = x->xso.type == XFRM_DEV_OFFLOAD_PACKET &&
 			    x->props.mode == XFRM_MODE_TUNNEL;
 	switch (x->props.family) {
-- 
2.50.0

Re: [PATCH ipsec 1/3] xfrm: restore GSO for SW crypto

[Leon Romanovsky]

On Mon, Jul 28, 2025 at 05:17:18PM +0200, Sabrina Dubroca wrote:
> Commit 49431af6c4ef incorrectly assumes that the GSO path is only used
> by HW offload, but it's also useful for SW crypto.
> 
> This patch re-enables GSO for SW crypto. It's not an exact revert to
> preserve the other changes made to xfrm_dev_offload_ok afterwards, but
> it reverts all of its effects.
> 
> Fixes: 49431af6c4ef ("xfrm: rely on XFRM offload")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
>  net/xfrm/xfrm_device.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 

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

Re: [PATCH ipsec 1/3] xfrm: restore GSO for SW crypto

[Zhu Yanjun]

在 2025/7/28 8:17, Sabrina Dubroca 写道:
> Commit 49431af6c4ef incorrectly assumes that the GSO path is only used
> by HW offload, but it's also useful for SW crypto.
>
> This patch re-enables GSO for SW crypto. It's not an exact revert to
> preserve the other changes made to xfrm_dev_offload_ok afterwards, but
> it reverts all of its effects.
>
> Fixes: 49431af6c4ef ("xfrm: rely on XFRM offload")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>

Thanks a lot.

Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev>

Zhu Yanjun

> ---
>   net/xfrm/xfrm_device.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
> index d2819baea414..1f88472aaac0 100644
> --- a/net/xfrm/xfrm_device.c
> +++ b/net/xfrm/xfrm_device.c
> @@ -415,10 +415,12 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
>   	struct net_device *dev = x->xso.dev;
>   	bool check_tunnel_size;
>   
> -	if (x->xso.type == XFRM_DEV_OFFLOAD_UNSPECIFIED)
> +	if (!x->type_offload ||
> +	    (x->xso.type == XFRM_DEV_OFFLOAD_UNSPECIFIED && x->encap))
>   		return false;
>   
> -	if ((dev == xfrm_dst_path(dst)->dev) && !xdst->child->xfrm) {
> +	if ((!dev || dev == xfrm_dst_path(dst)->dev) &&
> +	    !xdst->child->xfrm) {
>   		mtu = xfrm_state_mtu(x, xdst->child_mtu_cached);
>   		if (skb->len <= mtu)
>   			goto ok;
> @@ -430,6 +432,9 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
>   	return false;
>   
>   ok:
> +	if (!dev)
> +		return true;
> +
>   	check_tunnel_size = x->xso.type == XFRM_DEV_OFFLOAD_PACKET &&
>   			    x->props.mode == XFRM_MODE_TUNNEL;
>   	switch (x->props.family) {

-- 
Best Regards,
Yanjun.Zhu

[PATCH ipsec 2/3] Revert "xfrm: Remove unneeded device check from validate_xmit_xfrm"

[Sabrina Dubroca]

This reverts commit d53dda291bbd993a29b84d358d282076e3d01506.

This change causes traffic using GSO with SW crypto running through a
NIC capable of HW offload to no longer get segmented during
validate_xmit_xfrm.

Fixes: d53dda291bbd ("xfrm: Remove unneeded device check from validate_xmit_xfrm")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/xfrm/xfrm_device.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index 1f88472aaac0..14d39ba9a362 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -145,6 +145,10 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur
 		return NULL;
 	}
 
+	/* This skb was already validated on the upper/virtual dev */
+	if ((x->xso.dev != dev) && (x->xso.real_dev == dev))
+		return skb;
+
 	local_irq_save(flags);
 	sd = this_cpu_ptr(&softnet_data);
 	err = !skb_queue_empty(&sd->xfrm_backlog);
@@ -155,7 +159,8 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur
 		return skb;
 	}
 
-	if (skb_is_gso(skb) && unlikely(xmit_xfrm_check_overflow(skb))) {
+	if (skb_is_gso(skb) && (unlikely(x->xso.dev != dev) ||
+				unlikely(xmit_xfrm_check_overflow(skb)))) {
 		struct sk_buff *segs;
 
 		/* Packet got rerouted, fixup features and segment it. */
-- 
2.50.0

Re: [PATCH ipsec 2/3] Revert "xfrm: Remove unneeded device check from validate_xmit_xfrm"

[Leon Romanovsky]

On Mon, Jul 28, 2025 at 05:17:19PM +0200, Sabrina Dubroca wrote:
> This reverts commit d53dda291bbd993a29b84d358d282076e3d01506.
> 
> This change causes traffic using GSO with SW crypto running through a
> NIC capable of HW offload to no longer get segmented during
> validate_xmit_xfrm.
> 
> Fixes: d53dda291bbd ("xfrm: Remove unneeded device check from validate_xmit_xfrm")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
>  net/xfrm/xfrm_device.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 

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

Re: [PATCH ipsec 2/3] Revert "xfrm: Remove unneeded device check from validate_xmit_xfrm"

[Cosmin Ratiu]

On Mon, 2025-07-28 at 17:17 +0200, Sabrina Dubroca wrote:
> This reverts commit d53dda291bbd993a29b84d358d282076e3d01506.
> 
> This change causes traffic using GSO with SW crypto running through a
> NIC capable of HW offload to no longer get segmented during
> validate_xmit_xfrm.
> 
> Fixes: d53dda291bbd ("xfrm: Remove unneeded device check from
> validate_xmit_xfrm")
> 

Thanks for the fix, but I'm curious about details.

In that commit, I tried to map all of the possible code paths. Can you
please explain what code paths I missed that need real_dev given that
only bonding should use it now?

Thanks,
Cosmin.

Re: [PATCH ipsec 2/3] Revert "xfrm: Remove unneeded device check from validate_xmit_xfrm"

[Sabrina Dubroca]

2025-07-29, 15:27:39 +0000, Cosmin Ratiu wrote:
> On Mon, 2025-07-28 at 17:17 +0200, Sabrina Dubroca wrote:
> > This reverts commit d53dda291bbd993a29b84d358d282076e3d01506.
> > 
> > This change causes traffic using GSO with SW crypto running through a
> > NIC capable of HW offload to no longer get segmented during
> > validate_xmit_xfrm.
> > 
> > Fixes: d53dda291bbd ("xfrm: Remove unneeded device check from
> > validate_xmit_xfrm")
> > 
> 
> Thanks for the fix, but I'm curious about details.
> 
> In that commit, I tried to map all of the possible code paths. Can you
> please explain what code paths I missed that need real_dev given that
> only bonding should use it now?

After running some more tests, it's not about real_dev, it's the other
check ("unlikely(x->xso.dev != dev)" below) that you also removed in
that patch that causes the issue in my setup. I don't know how you
decided that it should be dropped, since it predates bonding's ipsec
offload.
The codepath is the usual:
__dev_queue_xmit -> validate_xmit_skb -> validate_xmit_xfrm

Since the commit message made the incorrect claim "ESP offload off:
validate_xmit_xfrm returns early on !xo." I didn't check if a partial
revert was enough to fix the issue. My bad.

-- 
Sabrina

Re: [PATCH ipsec 2/3] Revert "xfrm: Remove unneeded device check from validate_xmit_xfrm"

[Cosmin Ratiu]

On Wed, 2025-07-30 at 12:26 +0200, Sabrina Dubroca wrote:
> 2025-07-29, 15:27:39 +0000, Cosmin Ratiu wrote:
> > On Mon, 2025-07-28 at 17:17 +0200, Sabrina Dubroca wrote:
> > > This reverts commit d53dda291bbd993a29b84d358d282076e3d01506.
> > > 
> > > This change causes traffic using GSO with SW crypto running
> > > through a
> > > NIC capable of HW offload to no longer get segmented during
> > > validate_xmit_xfrm.
> > > 
> > > Fixes: d53dda291bbd ("xfrm: Remove unneeded device check from
> > > validate_xmit_xfrm")
> > > 
> > 
> > Thanks for the fix, but I'm curious about details.
> > 
> > In that commit, I tried to map all of the possible code paths. Can
> > you
> > please explain what code paths I missed that need real_dev given
> > that
> > only bonding should use it now?
> 
> After running some more tests, it's not about real_dev, it's the
> other
> check ("unlikely(x->xso.dev != dev)" below) that you also removed in
> that patch that causes the issue in my setup. I don't know how you
> decided that it should be dropped, since it predates bonding's ipsec
> offload.

Apologies for that, I think I assumed that if offload is off, then
xfrm_offload(skb) is NULL and the code bails out early on "if (!xo)".
Seems I was wrong. On the TX side, the only place that adds a secpath
and increments sp->olen (and thus add an xfrm_offload) is in
xfrm_output, after the xfrm_dev_offload_ok check.

> The codepath is the usual:
> __dev_queue_xmit -> validate_xmit_skb -> validate_xmit_xfrm
> 
> Since the commit message made the incorrect claim "ESP offload off:
> validate_xmit_xfrm returns early on !xo." I didn't check if a partial
> revert was enough to fix the issue. My bad.
> 
No problem, good that we caught the actual issue. Will you prepare a
follow-up patch then?

Cosmin.

Re: [PATCH ipsec 2/3] Revert "xfrm: Remove unneeded device check from validate_xmit_xfrm"

[Sabrina Dubroca]

2025-07-30, 12:32:13 +0000, Cosmin Ratiu wrote:
> On Wed, 2025-07-30 at 12:26 +0200, Sabrina Dubroca wrote:
> > 2025-07-29, 15:27:39 +0000, Cosmin Ratiu wrote:
> > > On Mon, 2025-07-28 at 17:17 +0200, Sabrina Dubroca wrote:
> > > > This reverts commit d53dda291bbd993a29b84d358d282076e3d01506.
> > > > 
> > > > This change causes traffic using GSO with SW crypto running
> > > > through a
> > > > NIC capable of HW offload to no longer get segmented during
> > > > validate_xmit_xfrm.
> > > > 
> > > > Fixes: d53dda291bbd ("xfrm: Remove unneeded device check from
> > > > validate_xmit_xfrm")
> > > > 
> > > 
> > > Thanks for the fix, but I'm curious about details.
> > > 
> > > In that commit, I tried to map all of the possible code paths. Can
> > > you
> > > please explain what code paths I missed that need real_dev given
> > > that
> > > only bonding should use it now?
> > 
> > After running some more tests, it's not about real_dev, it's the
> > other
> > check ("unlikely(x->xso.dev != dev)" below) that you also removed in
> > that patch that causes the issue in my setup. I don't know how you
> > decided that it should be dropped, since it predates bonding's ipsec
> > offload.
> 
> Apologies for that, I think I assumed that if offload is off, then
> xfrm_offload(skb) is NULL and the code bails out early on "if (!xo)".
> Seems I was wrong. On the TX side, the only place that adds a secpath
> and increments sp->olen (and thus add an xfrm_offload) is in
> xfrm_output, after the xfrm_dev_offload_ok check.

Yes, the "offload" code is used for both HW offload and "SW offloads"
(aka GSO/GRO).

> > The codepath is the usual:
> > __dev_queue_xmit -> validate_xmit_skb -> validate_xmit_xfrm
> > 
> > Since the commit message made the incorrect claim "ESP offload off:
> > validate_xmit_xfrm returns early on !xo." I didn't check if a partial
> > revert was enough to fix the issue. My bad.
> > 
> No problem, good that we caught the actual issue. Will you prepare a
> follow-up patch then?

I'll send a v2 of this series with this patch updated. Thanks.

-- 
Sabrina

[PATCH ipsec 3/3] udp: also consider secpath when evaluating ipsec use for checksumming

[Sabrina Dubroca]

Commit b40c5f4fde22 ("udp: disable inner UDP checksum offloads in
IPsec case") tried to fix checksumming in UFO when the packets are
going through IPsec, so that we can't rely on offloads because the UDP
header and payload will be encrypted.

But when doing a TCP test over VXLAN going through IPsec transport
mode with GSO enabled (esp4_offload module loaded), I'm seeing broken
UDP checksums on the encap after successful decryption.

The skbs get to udp4_ufo_fragment/__skb_udp_tunnel_segment via
__dev_queue_xmit -> validate_xmit_skb -> skb_gso_segment and at this
point we've already dropped the dst (unless the device sets
IFF_XMIT_DST_RELEASE, which is not common), so need_ipsec is false and
we proceed with checksum offload.

Make need_ipsec also check the secpath, which is not dropped on this
callpath.

Fixes: b40c5f4fde22 ("udp: disable inner UDP checksum offloads in IPsec case")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
Since the issue is related to IPsec and currently not visible since
GSO is currently broken for SW crypto, I'm including this patch in the
same series for the ipsec tree. I can split it out if that's prefered,
just let me know.

 net/ipv4/udp_offload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 85b5aa82d7d7..8758701c67d0 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -224,7 +224,7 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
 	remcsum = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TUNNEL_REMCSUM);
 	skb->remcsum_offload = remcsum;
 
-	need_ipsec = skb_dst(skb) && dst_xfrm(skb_dst(skb));
+	need_ipsec = (skb_dst(skb) && dst_xfrm(skb_dst(skb))) || skb_sec_path(skb);
 	/* Try to offload checksum if possible */
 	offload_csum = !!(need_csum &&
 			  !need_ipsec &&
-- 
2.50.0