Hi all,
The following patch introduced a regression in Chelsio cxgb4 driver, causing port failure when running heavy TSO traffic:
commit 10d3be569243def8d92ac3722395ef5a59c504e6
Author: Eric Dumazet [off-list ref]
Date: Thu Apr 21 10:55:23 2016 -0700
tcp-tso: do not split TSO packets at retransmit time
Linux TCP stack painfully segments all TSO/GSO packets before retransmits.
This was fine back in the days when TSO/GSO were emerging, with their
bugs, but we believe the dark age is over.
Keeping big packets in write queues, but also in stack traversal
has a lot of benefits.
- Less memory overhead, because write queues have less skbs
- Less cpu overhead at ACK processing.
- Better SACK processing, as lot of studies mentioned how
awful linux was at this ;)
- Less cpu overhead to send the rtx packets
(IP stack traversal, netfilter traversal, drivers...)
- Better latencies in presence of losses.
- Smaller spikes in fq like packet schedulers, as retransmits
are not constrained by TCP Small Queues.
1 % packet losses are common today, and at 100Gbit speeds, this
translates to ~80,000 losses per second.
Losses are often correlated, and we see many retransmit events
leading to 1-MSS train of packets, at the time hosts are already
under stress.
Signed-off-by: Eric Dumazet [off-list ref]
Acked-by: Yuchung Cheng [off-list ref]
Signed-off-by: David S. Miller davem@davemloft.net
When the number of TCP retransmissions are quite high, the packet length coming from stack does not seems to be proper, due to which our TSO module gets stuck.
If I change segs back to 1 in __tcp_retransmit_skb(), traffic is running fine. Please let us know if we are missing something.
Thanks,
Arjun.
From: Eric Dumazet <edumazet@google.com> Date: 2016-06-23 11:51:12
On Thu, Jun 23, 2016 at 3:08 AM, Arjun V. [off-list ref] wrote:
Hi all,
The following patch introduced a regression in Chelsio cxgb4 driver, causing port failure when running heavy TSO traffic:
commit 10d3be569243def8d92ac3722395ef5a59c504e6
Author: Eric Dumazet [off-list ref]
Date: Thu Apr 21 10:55:23 2016 -0700
tcp-tso: do not split TSO packets at retransmit time
Linux TCP stack painfully segments all TSO/GSO packets before retransmits.
This was fine back in the days when TSO/GSO were emerging, with their
bugs, but we believe the dark age is over.
Keeping big packets in write queues, but also in stack traversal
has a lot of benefits.
- Less memory overhead, because write queues have less skbs
- Less cpu overhead at ACK processing.
- Better SACK processing, as lot of studies mentioned how
awful linux was at this ;)
- Less cpu overhead to send the rtx packets
(IP stack traversal, netfilter traversal, drivers...)
- Better latencies in presence of losses.
- Smaller spikes in fq like packet schedulers, as retransmits
are not constrained by TCP Small Queues.
1 % packet losses are common today, and at 100Gbit speeds, this
translates to ~80,000 losses per second.
Losses are often correlated, and we see many retransmit events
leading to 1-MSS train of packets, at the time hosts are already
under stress.
Signed-off-by: Eric Dumazet [off-list ref]
Acked-by: Yuchung Cheng [off-list ref]
Signed-off-by: David S. Miller davem@davemloft.net
When the number of TCP retransmissions are quite high, the packet length coming from stack does not seems to be proper, due to which our TSO module gets stuck.
If I change segs back to 1 in __tcp_retransmit_skb(), traffic is running fine. Please let us know if we are missing something.
Thanks,
Arjun.
Hmm... I see nothing wrong in TCP stack.
Can you give me more details on the wrong packet length you see ?
Eric,
We are seeing skb's with length(skb->len) greater than 65536 coming into our ndo_start_xmit() callback routine.
We can add a check in our eth_xmit() routine to skip those packets, but it will be better if you fix this in kernel.
I have attached pcap file obtained from tcpdump. In the pcap file there are 2 such packets(I used tcpdump filter to extract out those packets).
Let us know if you need anything else.
Thanks,
Arjun.
-----Original Message-----
From: Eric Dumazet [mailto:edumazet@google.com]
Sent: Thursday, June 23, 2016 5:21 PM
To: Arjun V.
Cc: netdev@vger.kernel.org; Hariprasad S; Casey Leedom; Kumar A S; Santosh Rastapur; Nirranjan Kirubaharan; davem@davemloft.net; ycheng@google.com
Subject: Re: [REGRESSION, bisect]cxgb4 port failure with TSO traffic after commit 10d3be569243def8("tcp-tso: do not split TSO packets at retransmit time")
On Thu, Jun 23, 2016 at 3:08 AM, Arjun V. [off-list ref] wrote:
Hi all,
The following patch introduced a regression in Chelsio cxgb4 driver, causing port failure when running heavy TSO traffic:
commit 10d3be569243def8d92ac3722395ef5a59c504e6
Author: Eric Dumazet [off-list ref]
Date: Thu Apr 21 10:55:23 2016 -0700
tcp-tso: do not split TSO packets at retransmit time
Linux TCP stack painfully segments all TSO/GSO packets before retransmits.
This was fine back in the days when TSO/GSO were emerging, with their
bugs, but we believe the dark age is over.
Keeping big packets in write queues, but also in stack traversal
has a lot of benefits.
- Less memory overhead, because write queues have less skbs
- Less cpu overhead at ACK processing.
- Better SACK processing, as lot of studies mentioned how
awful linux was at this ;)
- Less cpu overhead to send the rtx packets
(IP stack traversal, netfilter traversal, drivers...)
- Better latencies in presence of losses.
- Smaller spikes in fq like packet schedulers, as retransmits
are not constrained by TCP Small Queues.
1 % packet losses are common today, and at 100Gbit speeds, this
translates to ~80,000 losses per second.
Losses are often correlated, and we see many retransmit events
leading to 1-MSS train of packets, at the time hosts are already
under stress.
Signed-off-by: Eric Dumazet [off-list ref]
Acked-by: Yuchung Cheng [off-list ref]
Signed-off-by: David S. Miller davem@davemloft.net
When the number of TCP retransmissions are quite high, the packet length coming from stack does not seems to be proper, due to which our TSO module gets stuck.
If I change segs back to 1 in __tcp_retransmit_skb(), traffic is running fine. Please let us know if we are missing something.
Thanks,
Arjun.
Hmm... I see nothing wrong in TCP stack.
Can you give me more details on the wrong packet length you see ?
From: Eric Dumazet <edumazet@google.com> Date: 2016-06-24 14:25:55
Please do not top post on netdev mailing list
On Fri, Jun 24, 2016 at 4:38 AM, Arjun V. [off-list ref] wrote:
Eric,
We are seeing skb's with length(skb->len) greater than 65536 coming into our ndo_start_xmit() callback routine.
We can add a check in our eth_xmit() routine to skip those packets, but it will be better if you fix this in kernel.
I have attached pcap file obtained from tcpdump. In the pcap file there are 2 such packets(I used tcpdump filter to extract out those packets).
Let us know if you need anything else.
Beats me really.
At retransmit time, we only can eventually reduce packet sizes
(assuming GSO is used, because we might coalesce sub-mss packets in
tcp_retrans_try_collapse())
So why are you seeing too big packets at retransmit, I really have no idea.
MIght be some bug related to MSS computation, overflowing somehow ?
From: Eric Dumazet <hidden> Date: 2016-06-24 16:25:11
On Fri, 2016-06-24 at 07:25 -0700, Eric Dumazet wrote:
Please do not top post on netdev mailing list
On Fri, Jun 24, 2016 at 4:38 AM, Arjun V. [off-list ref] wrote:
quoted
Eric,
We are seeing skb's with length(skb->len) greater than 65536 coming into our ndo_start_xmit() callback routine.
We can add a check in our eth_xmit() routine to skip those packets, but it will be better if you fix this in kernel.
I have attached pcap file obtained from tcpdump. In the pcap file there are 2 such packets(I used tcpdump filter to extract out those packets).
Let us know if you need anything else.
Beats me really.
At retransmit time, we only can eventually reduce packet sizes
(assuming GSO is used, because we might coalesce sub-mss packets in
tcp_retrans_try_collapse())
So why are you seeing too big packets at retransmit, I really have no idea.
MIght be some bug related to MSS computation, overflowing somehow ?
On Friday, June 06/24/16, 2016 at 21:55:07 +0530, Eric Dumazet wrote:
quoted hunk
On Fri, 2016-06-24 at 07:25 -0700, Eric Dumazet wrote:
quoted
Please do not top post on netdev mailing list
On Fri, Jun 24, 2016 at 4:38 AM, Arjun V. [off-list ref] wrote:
quoted
Eric,
We are seeing skb's with length(skb->len) greater than 65536 coming into our ndo_start_xmit() callback routine.
We can add a check in our eth_xmit() routine to skip those packets, but it will be better if you fix this in kernel.
I have attached pcap file obtained from tcpdump. In the pcap file there are 2 such packets(I used tcpdump filter to extract out those packets).
Let us know if you need anything else.
Beats me really.
At retransmit time, we only can eventually reduce packet sizes
(assuming GSO is used, because we might coalesce sub-mss packets in
tcp_retrans_try_collapse())
So why are you seeing too big packets at retransmit, I really have no idea.
MIght be some bug related to MSS computation, overflowing somehow ?
From: Eric Dumazet <hidden> Date: 2016-06-27 08:06:30
On Mon, 2016-06-27 at 12:47 +0530, Arjun V wrote:
Eric,
Thanks for the quick patch. It didn't help much. We still see packets larger than 65536 with the patch.
Below is the dump_stack() trace I am seeing for packets larger than 65536 in our xmit routine:
What values do you get for skb->len ?
Again, at retransmit time we never grow packets, we only can split
existing packets. So there is something very wrong.
The original xmit should already have hit this issue.
On Monday, June 06/27/16, 2016 at 13:36:22 +0530, Eric Dumazet wrote:
On Mon, 2016-06-27 at 12:47 +0530, Arjun V wrote:
quoted
Eric,
Thanks for the quick patch. It didn't help much. We still see packets larger than 65536 with the patch.
Below is the dump_stack() trace I am seeing for packets larger than 65536 in our xmit routine:
What values do you get for skb->len ?
These are the skb->len values I am hitting in t4_eth_xmit() for around half hour run of netperf bidi(800 connections total)
packet with skb->len 69570 detected
packet with skb->len 66674 detected
packet with skb->len 68122 detected
packet with skb->len 68122 detected
packet with skb->len 72466 detected
packet with skb->len 76810 detected
packet with skb->len 69570 detected
packet with skb->len 66674 detected
packet with skb->len 66674 detected
packet with skb->len 68122 detected
packet with skb->len 71018 detected
packet with skb->len 66674 detected
packet with skb->len 66674 detected
packet with skb->len 66674 detected
packet with skb->len 69570 detected
packet with skb->len 68122 detected
packet with skb->len 73914 detected
packet with skb->len 66674 detected
packet with skb->len 69570 detected
packet with skb->len 69570 detected
packet with skb->len 69570 detected
packet with skb->len 68122 detected
packet with skb->len 72466 detected
packet with skb->len 68122 detected
packet with skb->len 69570 detected
packet with skb->len 69570 detected
packet with skb->len 72466 detected
packet with skb->len 72466 detected
Again, at retransmit time we never grow packets, we only can split
existing packets. So there is something very wrong.
The original xmit should already have hit this issue.
From: Eric Dumazet <hidden> Date: 2016-06-27 09:21:27
On Mon, 2016-06-27 at 14:29 +0530, Arjun V wrote:
On Monday, June 06/27/16, 2016 at 13:36:22 +0530, Eric Dumazet wrote:
quoted
On Mon, 2016-06-27 at 12:47 +0530, Arjun V wrote:
quoted
Eric,
Thanks for the quick patch. It didn't help much. We still see packets larger than 65536 with the patch.
Below is the dump_stack() trace I am seeing for packets larger than 65536 in our xmit routine:
What values do you get for skb->len ?
These are the skb->len values I am hitting in t4_eth_xmit() for around half hour run of netperf bidi(800 connections total)
packet with skb->len 69570 detected
packet with skb->len 66674 detected
On Monday, June 06/27/16, 2016 at 14:51:19 +0530, Eric Dumazet wrote:
quoted hunk
On Mon, 2016-06-27 at 14:29 +0530, Arjun V wrote:
quoted
On Monday, June 06/27/16, 2016 at 13:36:22 +0530, Eric Dumazet wrote:
quoted
On Mon, 2016-06-27 at 12:47 +0530, Arjun V wrote:
quoted
Eric,
Thanks for the quick patch. It didn't help much. We still see packets larger than 65536 with the patch.
Below is the dump_stack() trace I am seeing for packets larger than 65536 in our xmit routine:
What values do you get for skb->len ?
These are the skb->len values I am hitting in t4_eth_xmit() for around half hour run of netperf bidi(800 connections total)
packet with skb->len 69570 detected
packet with skb->len 66674 detected
@@ -2784,6 +2784,10 @@ void tcp_xmit_retransmit_queue(struct sock *sk)segs=tp->snd_cwnd-tcp_packets_in_flight(tp);if(segs<=0)return;+/* In case tcp_shift_skb_data() have aggregated large skbs,+*weneedtomakesurenotsendingtoobigTSOpackets.+*/+segs=min_t(int,segs,tp->gso_segs);if(fwd_rexmitting){begin_fwd:
@@ -2784,6 +2784,10 @@ void tcp_xmit_retransmit_queue(struct sock *sk)segs=tp->snd_cwnd-tcp_packets_in_flight(tp);if(segs<=0)return;+/* In case tcp_shift_skb_data() have aggregated large skbs,+*weneedtomakesurenotsendingtoobigTSOpackets.+*/+segs=min_t(int,segs,tp->gso_segs);if(fwd_rexmitting){begin_fwd:
Nice catch, Eric. What do you think about using tcp_tso_autosize()
instead of tp->gso_segs? The goal would be to get autosized skbs in
this case, instead of 64KByte skbs. In addition to helping this corner
case of SACK reneging, this might also help things in general, since
if we are retransmitting packets then the cwnd and hence pacing rate
and hence autosized skb length might be smaller now than they were
when the packets were first sent. Just a thought.
neal
@@ -2784,6 +2784,10 @@ void tcp_xmit_retransmit_queue(struct sock *sk)segs=tp->snd_cwnd-tcp_packets_in_flight(tp);if(segs<=0)return;+/* In case tcp_shift_skb_data() have aggregated large skbs,+*weneedtomakesurenotsendingtoobigTSOpackets.+*/+segs=min_t(int,segs,tp->gso_segs);if(fwd_rexmitting){begin_fwd:
Nice catch, Eric. What do you think about using tcp_tso_autosize()
instead of tp->gso_segs? The goal would be to get autosized skbs in
this case, instead of 64KByte skbs. In addition to helping this corner
case of SACK reneging, this might also help things in general, since
if we are retransmitting packets then the cwnd and hence pacing rate
and hence autosized skb length might be smaller now than they were
when the packets were first sent. Just a thought.
@@ -2784,6 +2785,10 @@ void tcp_xmit_retransmit_queue(struct sock *sk)segs=tp->snd_cwnd-tcp_packets_in_flight(tp);if(segs<=0)return;+/* In case tcp_shift_skb_data() have aggregated large skbs,+*weneedtomakesurenotsendingtoobigTSOpackets.+*/+segs=min_t(int,segs,max_segs);if(fwd_rexmitting){begin_fwd:
@@ -2784,6 +2785,10 @@ void tcp_xmit_retransmit_queue(struct sock *sk)segs=tp->snd_cwnd-tcp_packets_in_flight(tp);if(segs<=0)return;+/* In case tcp_shift_skb_data() have aggregated large skbs,+*weneedtomakesurenotsendingtoobigTSOpackets.+*/+segs=min_t(int,segs,max_segs);if(fwd_rexmitting){begin_fwd:
@@ -2784,6 +2785,10 @@ void tcp_xmit_retransmit_queue(struct sock *sk)segs=tp->snd_cwnd-tcp_packets_in_flight(tp);if(segs<=0)return;+/* In case tcp_shift_skb_data() have aggregated large skbs,+*weneedtomakesurenotsendingtoobigTSOpackets.+*/+segs=min_t(int,segs,max_segs);if(fwd_rexmitting){begin_fwd:
Looks great to me. Thanks, Eric!
neal
This patch works fine. Now I am not seeing any packets with len > 64K. Thanks Eric for your support.
Arjun.
From: Eric Dumazet <hidden> Date: 2016-06-27 15:39:10
From: Eric Dumazet <edumazet@google.com>
Arjun reported a bug in TCP stack and bisected it to a recent commit.
In case where we process SACK, we can coalesce multiple skbs
into fat ones (tcp_shift_skb_data()), to lower write queue
overhead, because we do not expect to retransmit these packets.
However, SACK reneging can happen, forcing the sender to retransmit
all these packets. If skb->len is above 64KB, we then send buggy
IP packets that could hang TSO engine on cxgb4.
Neal suggested to use tcp_tso_autosize() instead of tp->gso_segs
so that we cook packets of optimal size vs TCP/pacing.
Thanks to Arjun for reporting the bug and running the tests !
Fixes: 10d3be569243 ("tcp-tso: do not split TSO packets at retransmit time")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Arjun V <redacted>
Tested-by: Arjun V <redacted>
---
net/ipv4/tcp_output.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
@@ -2784,6 +2785,10 @@ void tcp_xmit_retransmit_queue(struct sock *sk)segs=tp->snd_cwnd-tcp_packets_in_flight(tp);if(segs<=0)return;+/* In case tcp_shift_skb_data() have aggregated large skbs,+*weneedtomakesurenotsendingtoobigsTSOpackets+*/+segs=min_t(int,segs,max_segs);if(fwd_rexmitting){begin_fwd:
On Mon, Jun 27, 2016 at 11:38 AM, Eric Dumazet [off-list ref] wrote:
From: Eric Dumazet <edumazet@google.com>
Arjun reported a bug in TCP stack and bisected it to a recent commit.
In case where we process SACK, we can coalesce multiple skbs
into fat ones (tcp_shift_skb_data()), to lower write queue
overhead, because we do not expect to retransmit these packets.
However, SACK reneging can happen, forcing the sender to retransmit
all these packets. If skb->len is above 64KB, we then send buggy
IP packets that could hang TSO engine on cxgb4.
Neal suggested to use tcp_tso_autosize() instead of tp->gso_segs
so that we cook packets of optimal size vs TCP/pacing.
Thanks to Arjun for reporting the bug and running the tests !
Fixes: 10d3be569243 ("tcp-tso: do not split TSO packets at retransmit time")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Arjun V <redacted>
Tested-by: Arjun V <redacted>
---
net/ipv4/tcp_output.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
From: David Miller <davem@davemloft.net> Date: 2016-06-29 09:26:48
From: Eric Dumazet <redacted>
Date: Mon, 27 Jun 2016 17:38:50 +0200
From: Eric Dumazet <edumazet@google.com>
Arjun reported a bug in TCP stack and bisected it to a recent commit.
In case where we process SACK, we can coalesce multiple skbs
into fat ones (tcp_shift_skb_data()), to lower write queue
overhead, because we do not expect to retransmit these packets.
However, SACK reneging can happen, forcing the sender to retransmit
all these packets. If skb->len is above 64KB, we then send buggy
IP packets that could hang TSO engine on cxgb4.
Neal suggested to use tcp_tso_autosize() instead of tp->gso_segs
so that we cook packets of optimal size vs TCP/pacing.
Thanks to Arjun for reporting the bug and running the tests !
Fixes: 10d3be569243 ("tcp-tso: do not split TSO packets at retransmit time")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Arjun V <redacted>
Tested-by: Arjun V <redacted>