| Message ID | 66a8a101a2bfe645432ec393d22b9da44b9739cf.1415110605.git.mleitner@redhat.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Nov 4, 2014 at 9:18 AM, Marcelo Ricardo Leitner <mleitner@redhat.com> wrote: > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c > index a12b455928e52211efdc6b471ef54de6218f5df0..65686efeaaf3c36706390d3bfd260fd1fb942b7f 100644 > --- a/net/ipv4/tcp_input.c > +++ b/net/ipv4/tcp_input.c > @@ -2410,6 +2410,8 @@ static bool tcp_try_undo_recovery(struct sock *sk) > * is ACKed. For Reno it is MUST to prevent false > * fast retransmits (RFC2582). SACK TCP is safe. */ > tcp_moderate_cwnd(tp); > + if (!tcp_any_retrans_done(sk)) > + tp->retrans_stamp = 0; I ran into a compilation error with this, since tcp_any_retrans_done() is defined below this spot in the file. I'd recommend moving the tcp_any_retrans_done() code and its preceding block comment to just above the spot in tcp_input.c that says "/* Undo procedures. */". That way it compiles. In the meantime, Yuchung put together a nice packetdrill test case for this, so I'll run it on that version of the patch. neal -- 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
On Tue, Nov 4, 2014 at 1:51 PM, Neal Cardwell <ncardwell@google.com> wrote: > In the meantime, Yuchung put together a nice packetdrill test case for > this, so I'll run it on that version of the patch. With the compilation issue fixed, Yuchung's packetdrill test passes. So this looks good to me once the compilation issue with tcp_any_retrans_done() is fixed. neal -- 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
On 04-11-2014 17:03, Neal Cardwell wrote: > On Tue, Nov 4, 2014 at 1:51 PM, Neal Cardwell <ncardwell@google.com> wrote: >> In the meantime, Yuchung put together a nice packetdrill test case for >> this, so I'll run it on that version of the patch. > > With the compilation issue fixed, Yuchung's packetdrill test passes. > So this looks good to me once the compilation issue with > tcp_any_retrans_done() is fixed. Awesome! Thanks. Sending it now. Marcelo -- 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
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index a12b455928e52211efdc6b471ef54de6218f5df0..65686efeaaf3c36706390d3bfd260fd1fb942b7f 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -2410,6 +2410,8 @@ static bool tcp_try_undo_recovery(struct sock *sk) * is ACKed. For Reno it is MUST to prevent false * fast retransmits (RFC2582). SACK TCP is safe. */ tcp_moderate_cwnd(tp); + if (!tcp_any_retrans_done(sk)) + tp->retrans_stamp = 0; return true; } tcp_set_ca_state(sk, TCP_CA_Open);
Ueki Kohei reported that when we are using NewReno with connections that have a very low traffic, we may timeout the connection too early if a second loss occurs after the first one was successfully acked but no data was transfered later. Below is his description of it: When SACK is disabled, and a socket suffers multiple separate TCP retransmissions, that socket's ETIMEDOUT value is calculated from the time of the *first* retransmission instead of the *latest* retransmission. This happens because the tcp_sock's retrans_stamp is set once then never cleared. Take the following connection: Linux remote-machine | | send#1---->(*1)|--------> data#1 --------->| | | | RTO : : | | | ---(*2)|----> data#1(retrans) ---->| | (*3)|<---------- ACK <----------| | | | | : : | : : | : : 16 minutes (or more) : | : : | : : | : : | | | send#2---->(*4)|--------> data#2 --------->| | | | RTO : : | | | ---(*5)|----> data#2(retrans) ---->| | | | | | | RTO*2 : : | | | | | | ETIMEDOUT<----(*6)| | (*1) One data packet sent. (*2) Because no ACK packet is received, the packet is retransmitted. (*3) The ACK packet is received. The transmitted packet is acknowledged. At this point the first "retransmission event" has passed and been recovered from. Any future retransmission is a completely new "event". (*4) After 16 minutes (to correspond with retries2=15), a new data packet is sent. Note: No data is transmitted between (*3) and (*4). The socket's timeout SHOULD be calculated from this point in time, but instead it's calculated from the prior "event" 16 minutes ago. (*5) Because no ACK packet is received, the packet is retransmitted. (*6) At the time of the 2nd retransmission, the socket returns ETIMEDOUT. Therefore, now we clear retrans_stamp as soon as all data during the loss window is fully acked. Reported-by: Ueki Kohei Cc: Neal Cardwell <ncardwell@google.com> Cc: Yuchung Cheng <ycheng@google.com> Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com> --- net/ipv4/tcp_input.c | 2 ++ 1 file changed, 2 insertions(+)