diff mbox

[REGRESSION,bisect] cxgb4 port failure with TSO traffic after commit 10d3be569243def8("tcp-tso: do not split TSO packets at retransmit time")

Message ID 1467031616.6850.150.camel@edumazet-glaptop3.roam.corp.google.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Dumazet June 27, 2016, 12:46 p.m. UTC 
On Mon, 2016-06-27 at 12:50 +0200, Eric Dumazet wrote:
> On Mon, 2016-06-27 at 15:24 +0530, Arjun V wrote:
> 
> > Applied your patch.
> > The above debug print is not getting invoked, when skb->len is greater than 65536.
> 
> Interesting.
> 
> It looks like we might have a bug in tcp_shift_skb_data() not respecting
> sk->sk_gso_max_size at all ...

Note that this also means TCP receiver misbehaves and force us to SACK
reneging.

Patch would be :

Comments

Neal Cardwell June 27, 2016, 1:12 p.m. UTC | #1 
On Mon, Jun 27, 2016 at 8:46 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Patch would be :
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 8bd9911fdd16..3587efe22864 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -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,
> +                * we need to make sure not sending too big TSO packets.
> +                */
> +               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
diff mbox

Patch

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 8bd9911fdd16..3587efe22864 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -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,
+		 * we need to make sure not sending too big TSO packets.
+		 */
+		segs = min_t(int, segs, tp->gso_segs);
 
 		if (fwd_rexmitting) {
 begin_fwd: