netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v2 1/2] udp: fix gso_segs calculations
@ 2019-10-02 17:29 Josh Hunt
  2019-10-02 17:29 ` [PATCH net v2 2/2] udp: only do GSO if # of segs > 1 Josh Hunt
  2019-10-03 15:49 ` [PATCH net v2 1/2] udp: fix gso_segs calculations David Miller
  0 siblings, 2 replies; 6+ messages in thread
From: Josh Hunt @ 2019-10-02 17:29 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, willemb, alexander.h.duyck, Josh Hunt

Commit dfec0ee22c0a ("udp: Record gso_segs when supporting UDP segmentation offload")
added gso_segs calculation, but incorrectly got sizeof() the pointer and
not the underlying data type. In addition let's fix the v6 case.

Fixes: bec1f6f69736 ("udp: generate gso with UDP_SEGMENT")
Fixes: dfec0ee22c0a ("udp: Record gso_segs when supporting UDP segmentation offload")
Signed-off-by: Josh Hunt <johunt@akamai.com>
Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
Acked-by: Willem de Bruijn <willemb@google.com>
---
 net/ipv4/udp.c | 2 +-
 net/ipv6/udp.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index cf755156a684..be98d0b8f014 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -856,7 +856,7 @@ static int udp_send_skb(struct sk_buff *skb, struct flowi4 *fl4,
 
 		skb_shinfo(skb)->gso_size = cork->gso_size;
 		skb_shinfo(skb)->gso_type = SKB_GSO_UDP_L4;
-		skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(len - sizeof(uh),
+		skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(len - sizeof(*uh),
 							 cork->gso_size);
 		goto csum_partial;
 	}
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index aae4938f3dea..eb9a9934ac05 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1143,6 +1143,8 @@ static int udp_v6_send_skb(struct sk_buff *skb, struct flowi6 *fl6,
 
 		skb_shinfo(skb)->gso_size = cork->gso_size;
 		skb_shinfo(skb)->gso_type = SKB_GSO_UDP_L4;
+		skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(len - sizeof(*uh),
+							 cork->gso_size);
 		goto csum_partial;
 	}
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH net v2 2/2] udp: only do GSO if # of segs > 1
  2019-10-02 17:29 [PATCH net v2 1/2] udp: fix gso_segs calculations Josh Hunt
@ 2019-10-02 17:29 ` Josh Hunt
  2019-10-02 20:41   ` Willem de Bruijn
                     ` (2 more replies)
  2019-10-03 15:49 ` [PATCH net v2 1/2] udp: fix gso_segs calculations David Miller
  1 sibling, 3 replies; 6+ messages in thread
From: Josh Hunt @ 2019-10-02 17:29 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, willemb, alexander.h.duyck, Josh Hunt

Prior to this change an application sending <= 1MSS worth of data and
enabling UDP GSO would fail if the system had SW GSO enabled, but the
same send would succeed if HW GSO offload is enabled. In addition to this
inconsistency the error in the SW GSO case does not get back to the
application if sending out of a real device so the user is unaware of this
failure.

With this change we only perform GSO if the # of segments is > 1 even
if the application has enabled segmentation. I've also updated the
relevant udpgso selftests.

Fixes: bec1f6f69736 ("udp: generate gso with UDP_SEGMENT")
Signed-off-by: Josh Hunt <johunt@akamai.com>
---
 net/ipv4/udp.c                       | 11 +++++++----
 net/ipv6/udp.c                       | 11 +++++++----
 tools/testing/selftests/net/udpgso.c | 16 ++++------------
 3 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index be98d0b8f014..c31c7c353fcb 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -821,6 +821,7 @@ static int udp_send_skb(struct sk_buff *skb, struct flowi4 *fl4,
 	int is_udplite = IS_UDPLITE(sk);
 	int offset = skb_transport_offset(skb);
 	int len = skb->len - offset;
+	int datalen = len - sizeof(*uh);
 	__wsum csum = 0;
 
 	/*
@@ -854,10 +855,12 @@ static int udp_send_skb(struct sk_buff *skb, struct flowi4 *fl4,
 			return -EIO;
 		}
 
-		skb_shinfo(skb)->gso_size = cork->gso_size;
-		skb_shinfo(skb)->gso_type = SKB_GSO_UDP_L4;
-		skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(len - sizeof(*uh),
-							 cork->gso_size);
+		if (datalen > cork->gso_size) {
+			skb_shinfo(skb)->gso_size = cork->gso_size;
+			skb_shinfo(skb)->gso_type = SKB_GSO_UDP_L4;
+			skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(datalen,
+								 cork->gso_size);
+		}
 		goto csum_partial;
 	}
 
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index eb9a9934ac05..6324d3a8cb53 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1109,6 +1109,7 @@ static int udp_v6_send_skb(struct sk_buff *skb, struct flowi6 *fl6,
 	__wsum csum = 0;
 	int offset = skb_transport_offset(skb);
 	int len = skb->len - offset;
+	int datalen = len - sizeof(*uh);
 
 	/*
 	 * Create a UDP header
@@ -1141,10 +1142,12 @@ static int udp_v6_send_skb(struct sk_buff *skb, struct flowi6 *fl6,
 			return -EIO;
 		}
 
-		skb_shinfo(skb)->gso_size = cork->gso_size;
-		skb_shinfo(skb)->gso_type = SKB_GSO_UDP_L4;
-		skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(len - sizeof(*uh),
-							 cork->gso_size);
+		if (datalen > cork->gso_size) {
+			skb_shinfo(skb)->gso_size = cork->gso_size;
+			skb_shinfo(skb)->gso_type = SKB_GSO_UDP_L4;
+			skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(datalen,
+								 cork->gso_size);
+		}
 		goto csum_partial;
 	}
 
diff --git a/tools/testing/selftests/net/udpgso.c b/tools/testing/selftests/net/udpgso.c
index b8265ee9923f..614b31aad168 100644
--- a/tools/testing/selftests/net/udpgso.c
+++ b/tools/testing/selftests/net/udpgso.c
@@ -89,12 +89,9 @@ struct testcase testcases_v4[] = {
 		.tfail = true,
 	},
 	{
-		/* send a single MSS: will fail with GSO, because the segment
-		 * logic in udp4_ufo_fragment demands a gso skb to be > MTU
-		 */
+		/* send a single MSS: will fall back to no GSO */
 		.tlen = CONST_MSS_V4,
 		.gso_len = CONST_MSS_V4,
-		.tfail = true,
 		.r_num_mss = 1,
 	},
 	{
@@ -139,10 +136,9 @@ struct testcase testcases_v4[] = {
 		.tfail = true,
 	},
 	{
-		/* send a single 1B MSS: will fail, see single MSS above */
+		/* send a single 1B MSS: will fall back to no GSO */
 		.tlen = 1,
 		.gso_len = 1,
-		.tfail = true,
 		.r_num_mss = 1,
 	},
 	{
@@ -196,12 +192,9 @@ struct testcase testcases_v6[] = {
 		.tfail = true,
 	},
 	{
-		/* send a single MSS: will fail with GSO, because the segment
-		 * logic in udp4_ufo_fragment demands a gso skb to be > MTU
-		 */
+		/* send a single MSS: will fall back to no GSO */
 		.tlen = CONST_MSS_V6,
 		.gso_len = CONST_MSS_V6,
-		.tfail = true,
 		.r_num_mss = 1,
 	},
 	{
@@ -246,10 +239,9 @@ struct testcase testcases_v6[] = {
 		.tfail = true,
 	},
 	{
-		/* send a single 1B MSS: will fail, see single MSS above */
+		/* send a single 1B MSS: will fall back to no GSO */
 		.tlen = 1,
 		.gso_len = 1,
-		.tfail = true,
 		.r_num_mss = 1,
 	},
 	{
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2 2/2] udp: only do GSO if # of segs > 1
  2019-10-02 17:29 ` [PATCH net v2 2/2] udp: only do GSO if # of segs > 1 Josh Hunt
@ 2019-10-02 20:41   ` Willem de Bruijn
  2019-10-02 21:11   ` Duyck, Alexander H
  2019-10-03 15:50   ` David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: Willem de Bruijn @ 2019-10-02 20:41 UTC (permalink / raw)
  To: Josh Hunt
  Cc: David Miller, Network Development, Eric Dumazet, Alexander Duyck

On Wed, Oct 2, 2019 at 1:29 PM Josh Hunt <johunt@akamai.com> wrote:
>
> Prior to this change an application sending <= 1MSS worth of data and
> enabling UDP GSO would fail if the system had SW GSO enabled, but the
> same send would succeed if HW GSO offload is enabled. In addition to this
> inconsistency the error in the SW GSO case does not get back to the
> application if sending out of a real device so the user is unaware of this
> failure.
>
> With this change we only perform GSO if the # of segments is > 1 even
> if the application has enabled segmentation. I've also updated the
> relevant udpgso selftests.
>
> Fixes: bec1f6f69736 ("udp: generate gso with UDP_SEGMENT")
> Signed-off-by: Josh Hunt <johunt@akamai.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Thanks Josh.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2 2/2] udp: only do GSO if # of segs > 1
  2019-10-02 17:29 ` [PATCH net v2 2/2] udp: only do GSO if # of segs > 1 Josh Hunt
  2019-10-02 20:41   ` Willem de Bruijn
@ 2019-10-02 21:11   ` Duyck, Alexander H
  2019-10-03 15:50   ` David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: Duyck, Alexander H @ 2019-10-02 21:11 UTC (permalink / raw)
  To: davem, johunt; +Cc: netdev, willemb, edumazet

On Wed, 2019-10-02 at 13:29 -0400, Josh Hunt wrote:
> Prior to this change an application sending <= 1MSS worth of data and
> enabling UDP GSO would fail if the system had SW GSO enabled, but the
> same send would succeed if HW GSO offload is enabled. In addition to this
> inconsistency the error in the SW GSO case does not get back to the
> application if sending out of a real device so the user is unaware of this
> failure.
> 
> With this change we only perform GSO if the # of segments is > 1 even
> if the application has enabled segmentation. I've also updated the
> relevant udpgso selftests.
> 
> Fixes: bec1f6f69736 ("udp: generate gso with UDP_SEGMENT")
> Signed-off-by: Josh Hunt <johunt@akamai.com>
> ---
>  net/ipv4/udp.c                       | 11 +++++++----
>  net/ipv6/udp.c                       | 11 +++++++----
>  tools/testing/selftests/net/udpgso.c | 16 ++++------------
>  3 files changed, 18 insertions(+), 20 deletions(-)

Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2 1/2] udp: fix gso_segs calculations
  2019-10-02 17:29 [PATCH net v2 1/2] udp: fix gso_segs calculations Josh Hunt
  2019-10-02 17:29 ` [PATCH net v2 2/2] udp: only do GSO if # of segs > 1 Josh Hunt
@ 2019-10-03 15:49 ` David Miller
  1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2019-10-03 15:49 UTC (permalink / raw)
  To: johunt; +Cc: netdev, edumazet, willemb, alexander.h.duyck

From: Josh Hunt <johunt@akamai.com>
Date: Wed,  2 Oct 2019 13:29:22 -0400

> Commit dfec0ee22c0a ("udp: Record gso_segs when supporting UDP segmentation offload")
> added gso_segs calculation, but incorrectly got sizeof() the pointer and
> not the underlying data type. In addition let's fix the v6 case.
> 
> Fixes: bec1f6f69736 ("udp: generate gso with UDP_SEGMENT")
> Fixes: dfec0ee22c0a ("udp: Record gso_segs when supporting UDP segmentation offload")
> Signed-off-by: Josh Hunt <johunt@akamai.com>
> Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> Acked-by: Willem de Bruijn <willemb@google.com>

Applied and queued up for -stable.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2 2/2] udp: only do GSO if # of segs > 1
  2019-10-02 17:29 ` [PATCH net v2 2/2] udp: only do GSO if # of segs > 1 Josh Hunt
  2019-10-02 20:41   ` Willem de Bruijn
  2019-10-02 21:11   ` Duyck, Alexander H
@ 2019-10-03 15:50   ` David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2019-10-03 15:50 UTC (permalink / raw)
  To: johunt; +Cc: netdev, edumazet, willemb, alexander.h.duyck

From: Josh Hunt <johunt@akamai.com>
Date: Wed,  2 Oct 2019 13:29:23 -0400

> Prior to this change an application sending <= 1MSS worth of data and
> enabling UDP GSO would fail if the system had SW GSO enabled, but the
> same send would succeed if HW GSO offload is enabled. In addition to this
> inconsistency the error in the SW GSO case does not get back to the
> application if sending out of a real device so the user is unaware of this
> failure.
> 
> With this change we only perform GSO if the # of segments is > 1 even
> if the application has enabled segmentation. I've also updated the
> relevant udpgso selftests.
> 
> Fixes: bec1f6f69736 ("udp: generate gso with UDP_SEGMENT")
> Signed-off-by: Josh Hunt <johunt@akamai.com>

Applied and queued up for -stable.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-10-03 15:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-02 17:29 [PATCH net v2 1/2] udp: fix gso_segs calculations Josh Hunt
2019-10-02 17:29 ` [PATCH net v2 2/2] udp: only do GSO if # of segs > 1 Josh Hunt
2019-10-02 20:41   ` Willem de Bruijn
2019-10-02 21:11   ` Duyck, Alexander H
2019-10-03 15:50   ` David Miller
2019-10-03 15:49 ` [PATCH net v2 1/2] udp: fix gso_segs calculations David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).