Patchwork [2/2] tcp: remove the sk_can_gso() check from tcp_set_skb_tso_segs()

login
register
mail settings
Submitter Eric Dumazet
Date Oct. 15, 2013, 7:24 p.m.
Message ID <1381865094.2045.69.camel@edumazet-glaptop.roam.corp.google.com>
Download mbox | patch
Permalink /patch/283775/
State Accepted
Delegated to: David Miller
Headers show

Comments

Eric Dumazet - Oct. 15, 2013, 7:24 p.m.
From: Eric Dumazet <edumazet@google.com>

sk_can_gso() should only be used as a hint in tcp_sendmsg() to build GSO
packets in the first place. (As a performance hint)

Once we have GSO packets in write queue, we can not decide they are no
longer GSO only because flow now uses a route which doesn't handle
TSO/GSO.

Core networking stack handles the case very well for us, all we need
is keeping track of packet counts in MSS terms, regardless of
segmentation done later (in GSO or hardware)

Right now, if  tcp_fragment() splits a GSO packet in two parts,
@left and @right, and route changed through a non GSO device,
both @left and @right have pcount set to 1, which is wrong,
and leads to incorrect packet_count tracking.

This problem was added in commit d5ac99a648 ("[TCP]: skb pcount with MTU
discovery")

Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Yuchung Cheng <ycheng@google.com>
---
 net/ipv4/tcp_output.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet - Oct. 15, 2013, 7:44 p.m.
On Tue, 2013-10-15 at 12:24 -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>

> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> ---

Reported-by: Maciej ┼╗enczykowski <maze@google.com>


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Oct. 17, 2013, 8:09 p.m.
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 15 Oct 2013 12:24:54 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> sk_can_gso() should only be used as a hint in tcp_sendmsg() to build GSO
> packets in the first place. (As a performance hint)
> 
> Once we have GSO packets in write queue, we can not decide they are no
> longer GSO only because flow now uses a route which doesn't handle
> TSO/GSO.
> 
> Core networking stack handles the case very well for us, all we need
> is keeping track of packet counts in MSS terms, regardless of
> segmentation done later (in GSO or hardware)
> 
> Right now, if  tcp_fragment() splits a GSO packet in two parts,
> @left and @right, and route changed through a non GSO device,
> both @left and @right have pcount set to 1, which is wrong,
> and leads to incorrect packet_count tracking.
> 
> This problem was added in commit d5ac99a648 ("[TCP]: skb pcount with MTU
> discovery")
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>

Also applied and queued up for -stable, thanks everyone.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 8fad1c1..d46f214 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -989,8 +989,7 @@  static void tcp_set_skb_tso_segs(const struct sock *sk, struct sk_buff *skb,
 	/* Make sure we own this skb before messing gso_size/gso_segs */
 	WARN_ON_ONCE(skb_cloned(skb));
 
-	if (skb->len <= mss_now || !sk_can_gso(sk) ||
-	    skb->ip_summed == CHECKSUM_NONE) {
+	if (skb->len <= mss_now || skb->ip_summed == CHECKSUM_NONE) {
 		/* Avoid the costly divide in the normal
 		 * non-TSO case.
 		 */