[PATCH 0/5] pull request (net): ipsec 2024-03-06

[PATCH 0/5] pull request (net): ipsec 2024-03-06

[Steffen Klassert]

1) Clear the ECN bits flowi4_tos in decode_session4().
   This was already fixed but the bug was reintroduced
   when decode_session4() switched to us the flow dissector.
   From Guillaume Nault.

2) Fix UDP encapsulation in the TX path with packet offload mode.
   From Leon Romanovsky,

3) Avoid clang fortify warning in copy_to_user_tmpl().
   From Nathan Chancellor.

4) Fix inter address family tunnel in packet offload mode.
   From Mike Yu.

Please pull or let me know if there are problems.

Thanks!

The following changes since commit e327b2372bc0f18c30433ac40be07741b59231c5:

  net: ravb: Fix dma_addr_t truncation in error case (2024-01-14 16:41:51 +0000)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git tags/ipsec-2024-03-06

for you to fetch changes up to 2ce0eae694cfa8d0f5e4fa396015fc68c5958e8d:

  Merge branch 'Improve packet offload for dual stack' (2024-03-06 10:33:24 +0100)

----------------------------------------------------------------
ipsec-2024-03-06

----------------------------------------------------------------
Guillaume Nault (1):
      xfrm: Clear low order bits of ->flowi4_tos in decode_session4().

Leon Romanovsky (1):
      xfrm: Pass UDP encapsulation in TX packet offload

Mike Yu (2):
      xfrm: fix xfrm child route lookup for packet offload
      xfrm: set skb control buffer based on packet offload as well

Nathan Chancellor (1):
      xfrm: Avoid clang fortify warning in copy_to_user_tmpl()

Steffen Klassert (1):
      Merge branch 'Improve packet offload for dual stack'

 net/xfrm/xfrm_device.c | 2 +-
 net/xfrm/xfrm_output.c | 6 +++++-
 net/xfrm/xfrm_policy.c | 6 ++++--
 net/xfrm/xfrm_user.c   | 3 +++
 4 files changed, 13 insertions(+), 4 deletions(-)

[PATCH 1/5] xfrm: Clear low order bits of ->flowi4_tos in decode_session4().

[Steffen Klassert]

From: Guillaume Nault <gnault@redhat.com>

Commit 23e7b1bfed61 ("xfrm: Don't accidentally set RTO_ONLINK in
decode_session4()") fixed a problem where decode_session4() could
erroneously set the RTO_ONLINK flag for IPv4 route lookups. This
problem was reintroduced when decode_session4() was modified to
use the flow dissector.

Fix this by clearing again the two low order bits of ->flowi4_tos.
Found by code inspection, compile tested only.

Fixes: 7a0207094f1b ("xfrm: policy: replace session decode with flow dissector")
Signed-off-by: Guillaume Nault <gnault@redhat.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 1b7e75159727..7351f32052dc 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -3416,7 +3416,7 @@ decode_session4(const struct xfrm_flow_keys *flkeys, struct flowi *fl, bool reve
 	}
 
 	fl4->flowi4_proto = flkeys->basic.ip_proto;
-	fl4->flowi4_tos = flkeys->ip.tos;
+	fl4->flowi4_tos = flkeys->ip.tos & ~INET_ECN_MASK;
 }
 
 #if IS_ENABLED(CONFIG_IPV6)
-- 
2.34.1

[PATCH 2/5] xfrm: Pass UDP encapsulation in TX packet offload

[Steffen Klassert]

From: Leon Romanovsky <leonro@nvidia.com>

In addition to citied commit in Fixes line, allow UDP encapsulation in
TX path too.

Fixes: 89edf40220be ("xfrm: Support UDP encapsulation in packet offload mode")
CC: Steffen Klassert <steffen.klassert@secunet.com>
Reported-by: Mike Yu <yumike@google.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index 3784534c9185..653e51ae3964 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -407,7 +407,7 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
 	struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
 	struct net_device *dev = x->xso.dev;
 
-	if (!x->type_offload || x->encap)
+	if (!x->type_offload)
 		return false;
 
 	if (x->xso.type == XFRM_DEV_OFFLOAD_PACKET ||
-- 
2.34.1

[PATCH 3/5] xfrm: Avoid clang fortify warning in copy_to_user_tmpl()

[Steffen Klassert]

From: Nathan Chancellor <nathan@kernel.org>

After a couple recent changes in LLVM, there is a warning (or error with
CONFIG_WERROR=y or W=e) from the compile time fortify source routines,
specifically the memset() in copy_to_user_tmpl().

  In file included from net/xfrm/xfrm_user.c:14:
  ...
  include/linux/fortify-string.h:438:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
    438 |                         __write_overflow_field(p_size_field, size);
        |                         ^
  1 error generated.

While ->xfrm_nr has been validated against XFRM_MAX_DEPTH when its value
is first assigned in copy_templates() by calling validate_tmpl() first
(so there should not be any issue in practice), LLVM/clang cannot really
deduce that across the boundaries of these functions. Without that
knowledge, it cannot assume that the loop stops before i is greater than
XFRM_MAX_DEPTH, which would indeed result a stack buffer overflow in the
memset().

To make the bounds of ->xfrm_nr clear to the compiler and add additional
defense in case copy_to_user_tmpl() is ever used in a path where
->xfrm_nr has not been properly validated against XFRM_MAX_DEPTH first,
add an explicit bound check and early return, which clears up the
warning.

Cc: stable@vger.kernel.org
Link: https://github.com/ClangBuiltLinux/linux/issues/1985
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_user.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index ad01997c3aa9..444e58bc3f44 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2017,6 +2017,9 @@ static int copy_to_user_tmpl(struct xfrm_policy *xp, struct sk_buff *skb)
 	if (xp->xfrm_nr == 0)
 		return 0;
 
+	if (xp->xfrm_nr > XFRM_MAX_DEPTH)
+		return -ENOBUFS;
+
 	for (i = 0; i < xp->xfrm_nr; i++) {
 		struct xfrm_user_tmpl *up = &vec[i];
 		struct xfrm_tmpl *kp = &xp->xfrm_vec[i];
-- 
2.34.1

[PATCH 4/5] xfrm: fix xfrm child route lookup for packet offload

[Steffen Klassert]

From: Mike Yu <yumike@google.com>

In current code, xfrm_bundle_create() always uses the matched
SA's family type to look up a xfrm child route for the skb.
The route returned by xfrm_dst_lookup() will eventually be
used in xfrm_output_resume() (skb_dst(skb)->ops->local_out()).

If packet offload is used, the above behavior can lead to
calling ip_local_out() for an IPv6 packet or calling
ip6_local_out() for an IPv4 packet, which is likely to fail.

This change fixes the behavior by checking if the matched SA
has packet offload enabled. If not, keep the same behavior;
if yes, use the matched SP's family type for the lookup.

Test: verified IPv6-in-IPv4 packets on Android device with
      IPsec packet offload enabled
Signed-off-by: Mike Yu <yumike@google.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 7351f32052dc..da6ecc6b3e15 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2694,7 +2694,9 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
 			if (xfrm[i]->props.smark.v || xfrm[i]->props.smark.m)
 				mark = xfrm_smark_get(fl->flowi_mark, xfrm[i]);
 
-			family = xfrm[i]->props.family;
+			if (xfrm[i]->xso.type != XFRM_DEV_OFFLOAD_PACKET)
+				family = xfrm[i]->props.family;
+
 			oif = fl->flowi_oif ? : fl->flowi_l3mdev;
 			dst = xfrm_dst_lookup(xfrm[i], tos, oif,
 					      &saddr, &daddr, family, mark);
-- 
2.34.1

[PATCH 5/5] xfrm: set skb control buffer based on packet offload as well

[Steffen Klassert]

From: Mike Yu <yumike@google.com>

In packet offload, packets are not encrypted in XFRM stack, so
the next network layer which the packets will be forwarded to
should depend on where the packet came from (either xfrm4_output
or xfrm6_output) rather than the matched SA's family type.

Test: verified IPv6-in-IPv4 packets on Android device with
      IPsec packet offload enabled
Signed-off-by: Mike Yu <yumike@google.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_output.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index 662c83beb345..e5722c95b8bb 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -704,9 +704,13 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb)
 {
 	struct net *net = dev_net(skb_dst(skb)->dev);
 	struct xfrm_state *x = skb_dst(skb)->xfrm;
+	int family;
 	int err;
 
-	switch (x->outer_mode.family) {
+	family = (x->xso.type != XFRM_DEV_OFFLOAD_PACKET) ? x->outer_mode.family
+		: skb_dst(skb)->ops->family;
+
+	switch (family) {
 	case AF_INET:
 		memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
 		IPCB(skb)->flags |= IPSKB_XFRM_TRANSFORMED;
-- 
2.34.1

Re: [PATCH 1/5] xfrm: Clear low order bits of ->flowi4_tos in decode_session4().

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Steffen Klassert <steffen.klassert@secunet.com>:

On Wed, 6 Mar 2024 11:04:34 +0100 you wrote:
> From: Guillaume Nault <gnault@redhat.com>
> 
> Commit 23e7b1bfed61 ("xfrm: Don't accidentally set RTO_ONLINK in
> decode_session4()") fixed a problem where decode_session4() could
> erroneously set the RTO_ONLINK flag for IPv4 route lookups. This
> problem was reintroduced when decode_session4() was modified to
> use the flow dissector.
> 
> [...]

Here is the summary with links:
  - [1/5] xfrm: Clear low order bits of ->flowi4_tos in decode_session4().
    https://git.kernel.org/netdev/net/c/1982a2a02c91
  - [2/5] xfrm: Pass UDP encapsulation in TX packet offload
    https://git.kernel.org/netdev/net/c/983a73da1f99
  - [3/5] xfrm: Avoid clang fortify warning in copy_to_user_tmpl()
    https://git.kernel.org/netdev/net/c/1a807e46aa93
  - [4/5] xfrm: fix xfrm child route lookup for packet offload
    https://git.kernel.org/netdev/net/c/d4872d70fc6f
  - [5/5] xfrm: set skb control buffer based on packet offload as well
    https://git.kernel.org/netdev/net/c/8688ab2170a5

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH 2/5] xfrm: Pass UDP encapsulation in TX packet offload

[Paolo Abeni]

Hi,

On Wed, 2024-03-06 at 11:04 +0100, Steffen Klassert wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> In addition to citied commit in Fixes line, allow UDP encapsulation in
> TX path too.
> 
> Fixes: 89edf40220be ("xfrm: Support UDP encapsulation in packet offload mode")
> CC: Steffen Klassert <steffen.klassert@secunet.com>
> Reported-by: Mike Yu <yumike@google.com>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

This is causing self-test failures:

https://netdev.bots.linux.dev/flakes.html?tn-needle=pmtu-sh

reverting this change locally resolves the issue.

@Leon, @Steffen: could you please have a look?

Thanks!

Paolo

Re: [PATCH 2/5] xfrm: Pass UDP encapsulation in TX packet offload

[Jakub Kicinski]

On Mon, 11 Mar 2024 17:25:03 +0100 Paolo Abeni wrote:
> Hi,
> 
> On Wed, 2024-03-06 at 11:04 +0100, Steffen Klassert wrote:
> > From: Leon Romanovsky <leonro@nvidia.com>
> > 
> > In addition to citied commit in Fixes line, allow UDP encapsulation in
> > TX path too.
> > 
> > Fixes: 89edf40220be ("xfrm: Support UDP encapsulation in packet offload mode")
> > CC: Steffen Klassert <steffen.klassert@secunet.com>
> > Reported-by: Mike Yu <yumike@google.com>
> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
> > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>  
> 
> This is causing self-test failures:
> 
> https://netdev.bots.linux.dev/flakes.html?tn-needle=pmtu-sh
> 
> reverting this change locally resolves the issue.
> 
> @Leon, @Steffen: could you please have a look?

The failure in rtnetlink.sh seems to also be xfrm related:

# FAIL: ipsec_offload can't create SA

https://netdev-3.bots.linux.dev/vmksft-net-dbg/results/502821/10-rtnetlink-sh/stdout

Re: [PATCH 2/5] xfrm: Pass UDP encapsulation in TX packet offload

[Jakub Kicinski]

On Mon, 11 Mar 2024 10:05:10 -0700 Jakub Kicinski wrote:
> > This is causing self-test failures:
> > 
> > https://netdev.bots.linux.dev/flakes.html?tn-needle=pmtu-sh
> > 
> > reverting this change locally resolves the issue.
> > 
> > @Leon, @Steffen: could you please have a look?  
> 
> The failure in rtnetlink.sh seems to also be xfrm related:
> 
> # FAIL: ipsec_offload can't create SA
> 
> https://netdev-3.bots.linux.dev/vmksft-net-dbg/results/502821/10-rtnetlink-sh/stdout

That failure resolved itself, FWIW, so ignore that.

Re: [PATCH 2/5] xfrm: Pass UDP encapsulation in TX packet offload

[Steffen Klassert]

On Mon, Mar 11, 2024 at 05:25:03PM +0100, Paolo Abeni wrote:
> Hi,
> 
> On Wed, 2024-03-06 at 11:04 +0100, Steffen Klassert wrote:
> > From: Leon Romanovsky <leonro@nvidia.com>
> > 
> > In addition to citied commit in Fixes line, allow UDP encapsulation in
> > TX path too.
> > 
> > Fixes: 89edf40220be ("xfrm: Support UDP encapsulation in packet offload mode")
> > CC: Steffen Klassert <steffen.klassert@secunet.com>
> > Reported-by: Mike Yu <yumike@google.com>
> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
> > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> 
> This is causing self-test failures:
> 
> https://netdev.bots.linux.dev/flakes.html?tn-needle=pmtu-sh
> 
> reverting this change locally resolves the issue.
> 
> @Leon, @Steffen: could you please have a look?

Looks like the check for x->encap was removed unconditionally.
I should just be removed when XFRM_DEV_OFFLOAD_PACKET is set,
otherwise we might create a GSO packet with UPD encapsulation.

Leon?

Re: [PATCH 2/5] xfrm: Pass UDP encapsulation in TX packet offload

[Leon Romanovsky]

On Tue, Mar 12, 2024 at 07:20:06AM +0100, Steffen Klassert wrote:
> On Mon, Mar 11, 2024 at 05:25:03PM +0100, Paolo Abeni wrote:
> > Hi,
> > 
> > On Wed, 2024-03-06 at 11:04 +0100, Steffen Klassert wrote:
> > > From: Leon Romanovsky <leonro@nvidia.com>
> > > 
> > > In addition to citied commit in Fixes line, allow UDP encapsulation in
> > > TX path too.
> > > 
> > > Fixes: 89edf40220be ("xfrm: Support UDP encapsulation in packet offload mode")
> > > CC: Steffen Klassert <steffen.klassert@secunet.com>
> > > Reported-by: Mike Yu <yumike@google.com>
> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > > Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
> > > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> > 
> > This is causing self-test failures:
> > 
> > https://netdev.bots.linux.dev/flakes.html?tn-needle=pmtu-sh
> > 
> > reverting this change locally resolves the issue.
> > 
> > @Leon, @Steffen: could you please have a look?
> 
> Looks like the check for x->encap was removed unconditionally.
> I should just be removed when XFRM_DEV_OFFLOAD_PACKET is set,
> otherwise we might create a GSO packet with UPD encapsulation.
> 
> Leon?

Right, I missed IPsec SW path, that x->encap check can be removed
in packet offload because HW supports it and in crypto offload, because
there is a check in xfrm_dev_state_add() to prevent it.

What about this fix?

diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index 653e51ae3964..6e3e5a09cfeb 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -407,7 +407,7 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
        struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
        struct net_device *dev = x->xso.dev;

-       if (!x->type_offload)
+       if (!x->type_offload || x->xso.type == XFRM_DEV_OFFLOAD_UNSPECIFIED)
                return false;

        if (x->xso.type == XFRM_DEV_OFFLOAD_PACKET ||


Thanks



> 

Re: [PATCH 2/5] xfrm: Pass UDP encapsulation in TX packet offload

[Steffen Klassert]

On Tue, Mar 12, 2024 at 01:15:28PM +0200, Leon Romanovsky wrote:
> On Tue, Mar 12, 2024 at 07:20:06AM +0100, Steffen Klassert wrote:
> > On Mon, Mar 11, 2024 at 05:25:03PM +0100, Paolo Abeni wrote:
> > > Hi,
> > > 
> > > On Wed, 2024-03-06 at 11:04 +0100, Steffen Klassert wrote:
> > > > From: Leon Romanovsky <leonro@nvidia.com>
> > > > 
> > > > In addition to citied commit in Fixes line, allow UDP encapsulation in
> > > > TX path too.
> > > > 
> > > > Fixes: 89edf40220be ("xfrm: Support UDP encapsulation in packet offload mode")
> > > > CC: Steffen Klassert <steffen.klassert@secunet.com>
> > > > Reported-by: Mike Yu <yumike@google.com>
> > > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > > > Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
> > > > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> > > 
> > > This is causing self-test failures:
> > > 
> > > https://netdev.bots.linux.dev/flakes.html?tn-needle=pmtu-sh
> > > 
> > > reverting this change locally resolves the issue.
> > > 
> > > @Leon, @Steffen: could you please have a look?
> > 
> > Looks like the check for x->encap was removed unconditionally.
> > I should just be removed when XFRM_DEV_OFFLOAD_PACKET is set,
> > otherwise we might create a GSO packet with UPD encapsulation.
> > 
> > Leon?
> 
> Right, I missed IPsec SW path, that x->encap check can be removed
> in packet offload because HW supports it and in crypto offload, because
> there is a check in xfrm_dev_state_add() to prevent it.
> 
> What about this fix?
> 
> diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
> index 653e51ae3964..6e3e5a09cfeb 100644
> --- a/net/xfrm/xfrm_device.c
> +++ b/net/xfrm/xfrm_device.c
> @@ -407,7 +407,7 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
>         struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
>         struct net_device *dev = x->xso.dev;
> 
> -       if (!x->type_offload)
> +       if (!x->type_offload || x->xso.type == XFRM_DEV_OFFLOAD_UNSPECIFIED)
>                 return false;

Then we can't generate GSO packets for the SW path anymore. We just need
to reject UDP enacpsulation in SW here.

Re: [PATCH 2/5] xfrm: Pass UDP encapsulation in TX packet offload

[Leon Romanovsky]

On Tue, Mar 12, 2024 at 12:20:49PM +0100, Steffen Klassert wrote:
> On Tue, Mar 12, 2024 at 01:15:28PM +0200, Leon Romanovsky wrote:
> > On Tue, Mar 12, 2024 at 07:20:06AM +0100, Steffen Klassert wrote:
> > > On Mon, Mar 11, 2024 at 05:25:03PM +0100, Paolo Abeni wrote:
> > > > Hi,
> > > > 
> > > > On Wed, 2024-03-06 at 11:04 +0100, Steffen Klassert wrote:
> > > > > From: Leon Romanovsky <leonro@nvidia.com>
> > > > > 
> > > > > In addition to citied commit in Fixes line, allow UDP encapsulation in
> > > > > TX path too.
> > > > > 
> > > > > Fixes: 89edf40220be ("xfrm: Support UDP encapsulation in packet offload mode")
> > > > > CC: Steffen Klassert <steffen.klassert@secunet.com>
> > > > > Reported-by: Mike Yu <yumike@google.com>
> > > > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > > > > Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
> > > > > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> > > > 
> > > > This is causing self-test failures:
> > > > 
> > > > https://netdev.bots.linux.dev/flakes.html?tn-needle=pmtu-sh
> > > > 
> > > > reverting this change locally resolves the issue.
> > > > 
> > > > @Leon, @Steffen: could you please have a look?
> > > 
> > > Looks like the check for x->encap was removed unconditionally.
> > > I should just be removed when XFRM_DEV_OFFLOAD_PACKET is set,
> > > otherwise we might create a GSO packet with UPD encapsulation.
> > > 
> > > Leon?
> > 
> > Right, I missed IPsec SW path, that x->encap check can be removed
> > in packet offload because HW supports it and in crypto offload, because
> > there is a check in xfrm_dev_state_add() to prevent it.
> > 
> > What about this fix?
> > 
> > diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
> > index 653e51ae3964..6e3e5a09cfeb 100644
> > --- a/net/xfrm/xfrm_device.c
> > +++ b/net/xfrm/xfrm_device.c
> > @@ -407,7 +407,7 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
> >         struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
> >         struct net_device *dev = x->xso.dev;
> > 
> > -       if (!x->type_offload)
> > +       if (!x->type_offload || x->xso.type == XFRM_DEV_OFFLOAD_UNSPECIFIED)
> >                 return false;
> 
> Then we can't generate GSO packets for the SW path anymore. We just need
> to reject UDP enacpsulation in SW here.

Is it better?

diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index 653e51ae3964..6346690d5c69 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -407,7 +407,8 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
        struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
        struct net_device *dev = x->xso.dev;

-       if (!x->type_offload)
+       if (!x->type_offload ||
+           (x->xso.type == XFRM_DEV_OFFLOAD_UNSPECIFIED && x->encap))
                return false;

        if (x->xso.type == XFRM_DEV_OFFLOAD_PACKET ||

> 

Re: [PATCH 2/5] xfrm: Pass UDP encapsulation in TX packet offload

[Steffen Klassert]

On Tue, Mar 12, 2024 at 01:26:30PM +0200, Leon Romanovsky wrote:
> 
> Is it better?
> 
> diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
> index 653e51ae3964..6346690d5c69 100644
> --- a/net/xfrm/xfrm_device.c
> +++ b/net/xfrm/xfrm_device.c
> @@ -407,7 +407,8 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
>         struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
>         struct net_device *dev = x->xso.dev;
> 
> -       if (!x->type_offload)
> +       if (!x->type_offload ||
> +           (x->xso.type == XFRM_DEV_OFFLOAD_UNSPECIFIED && x->encap))
>                 return false;

Yes, that should do it.

Thanks!