| Message ID | 20240306100438.3953516-3-steffen.klassert@secunet.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 983a73da1f996faee9997149eb05b12fa7bd8cbf |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [1/5] xfrm: Clear low order bits of ->flowi4_tos in decode_session4(). | expand |
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
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
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.
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?
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 >
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.
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 || >
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!
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 ||