mirror of
https://github.com/rd-stuffs/msm-4.14.git
synced 2025-02-20 11:45:48 +08:00
tcp: fix early ETIMEDOUT after spurious non-SACK RTO
[ Upstream commit 686dc2db2a0fdc1d34b424ec2c0a735becd8d62b ] Fix a bug reported and analyzed by Nagaraj Arankal, where the handling of a spurious non-SACK RTO could cause a connection to fail to clear retrans_stamp, causing a later RTO to very prematurely time out the connection with ETIMEDOUT. Here is the buggy scenario, expanding upon Nagaraj Arankal's excellent report: (*1) Send one data packet on a non-SACK connection (*2) Because no ACK packet is received, the packet is retransmitted and we enter CA_Loss; but this retransmission is spurious. (*3) The ACK for the original data is received. The transmitted packet is acknowledged. The TCP timestamp is before the retrans_stamp, so tcp_may_undo() returns true, and tcp_try_undo_loss() returns true without changing state to Open (because tcp_is_sack() is false), and tcp_process_loss() returns without calling tcp_try_undo_recovery(). Normally after undoing a CA_Loss episode, tcp_fastretrans_alert() would see that the connection has returned to CA_Open and fall through and call tcp_try_to_open(), which would set retrans_stamp to 0. However, for non-SACK connections we hold the connection in CA_Loss, so do not fall through to call tcp_try_to_open() and do not set retrans_stamp to 0. So retrans_stamp is (erroneously) still non-zero. At this point the first "retransmission event" has passed and been recovered from. Any future retransmission is a completely new "event". However, retrans_stamp is erroneously still set. (And we are still in CA_Loss, which is correct.) (*4) After 16 minutes (to correspond with tcp_retries2=15), a new data packet is sent. Note: No data is transmitted between (*3) and (*4) and we disabled keep alives. The socket's timeout SHOULD be calculated from this point in time, but instead it's calculated from the prior "event" 16 minutes ago (step (*2)). (*5) Because no ACK packet is received, the packet is retransmitted. (*6) At the time of the 2nd retransmission, the socket returns ETIMEDOUT, prematurely, because retrans_stamp is (erroneously) too far in the past (set at the time of (*2)). This commit fixes this bug by ensuring that we reuse in tcp_try_undo_loss() the same careful logic for non-SACK connections that we have in tcp_try_undo_recovery(). To avoid duplicating logic, we factor out that logic into a new tcp_is_non_sack_preventing_reopen() helper and call that helper from both undo functions. Fixes: da34ac7626b5 ("tcp: only undo on partial ACKs in CA_Loss") Reported-by: Nagaraj Arankal <nagaraj.p.arankal@hpe.com> Link: https://lore.kernel.org/all/SJ0PR84MB1847BE6C24D274C46A1B9B0EB27A9@SJ0PR84MB1847.NAMPRD84.PROD.OUTLOOK.COM/ Signed-off-by: Neal Cardwell <ncardwell@google.com> Signed-off-by: Yuchung Cheng <ycheng@google.com> Reviewed-by: Eric Dumazet <edumazet@google.com> Link: https://lore.kernel.org/r/20220903121023.866900-1-ncardwell.kernel@gmail.com Signed-off-by: Paolo Abeni <pabeni@redhat.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
parent
dc9dbd65c8
commit
e50b799a9d
@ -2425,6 +2425,21 @@ static inline bool tcp_may_undo(const struct tcp_sock *tp)
|
||||
return tp->undo_marker && (!tp->undo_retrans || tcp_packet_delayed(tp));
|
||||
}
|
||||
|
||||
static bool tcp_is_non_sack_preventing_reopen(struct sock *sk)
|
||||
{
|
||||
struct tcp_sock *tp = tcp_sk(sk);
|
||||
|
||||
if (tp->snd_una == tp->high_seq && tcp_is_reno(tp)) {
|
||||
/* Hold old state until something *above* high_seq
|
||||
* is ACKed. For Reno it is MUST to prevent false
|
||||
* fast retransmits (RFC2582). SACK TCP is safe. */
|
||||
if (!tcp_any_retrans_done(sk))
|
||||
tp->retrans_stamp = 0;
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/* People celebrate: "We love our President!" */
|
||||
static bool tcp_try_undo_recovery(struct sock *sk)
|
||||
{
|
||||
@ -2445,14 +2460,8 @@ static bool tcp_try_undo_recovery(struct sock *sk)
|
||||
|
||||
NET_INC_STATS(sock_net(sk), mib_idx);
|
||||
}
|
||||
if (tp->snd_una == tp->high_seq && tcp_is_reno(tp)) {
|
||||
/* Hold old state until something *above* high_seq
|
||||
* is ACKed. For Reno it is MUST to prevent false
|
||||
* fast retransmits (RFC2582). SACK TCP is safe. */
|
||||
if (!tcp_any_retrans_done(sk))
|
||||
tp->retrans_stamp = 0;
|
||||
if (tcp_is_non_sack_preventing_reopen(sk))
|
||||
return true;
|
||||
}
|
||||
tcp_set_ca_state(sk, TCP_CA_Open);
|
||||
tp->is_sack_reneg = 0;
|
||||
return false;
|
||||
@ -2486,6 +2495,8 @@ static bool tcp_try_undo_loss(struct sock *sk, bool frto_undo)
|
||||
NET_INC_STATS(sock_net(sk),
|
||||
LINUX_MIB_TCPSPURIOUSRTOS);
|
||||
inet_csk(sk)->icsk_retransmits = 0;
|
||||
if (tcp_is_non_sack_preventing_reopen(sk))
|
||||
return true;
|
||||
if (frto_undo || tcp_is_sack(tp)) {
|
||||
tcp_set_ca_state(sk, TCP_CA_Open);
|
||||
tp->is_sack_reneg = 0;
|
||||
|
Loading…
x
Reference in New Issue
Block a user