Commit 12fb3dd9 authored by Eric Dumazet's avatar Eric Dumazet Committed by David S. Miller

tcp: call tcp_replace_ts_recent() from tcp_ack()

commit bd090dfc (tcp: tcp_replace_ts_recent() should not be called
from tcp_validate_incoming()) introduced a TS ecr bug in slow path
processing.

1 A > B P. 1:10001(10000) ack 1 <nop,nop,TS val 1001 ecr 200>
2 B < A . 1:1(0) ack 1 win 257 <sack 9001:10001,TS val 300 ecr 1001>
3 A > B . 1:1001(1000) ack 1 win 227 <nop,nop,TS val 1002 ecr 200>
4 A > B . 1001:2001(1000) ack 1 win 227 <nop,nop,TS val 1002 ecr 200>

(ecr 200 should be ecr 300 in packets 3 & 4)

Problem is tcp_ack() can trigger send of new packets (retransmits),
reflecting the prior TSval, instead of the TSval contained in the
currently processed incoming packet.

Fix this by calling tcp_replace_ts_recent() from tcp_ack() after the
checks, but before the actions.
Reported-by: default avatarYuchung Cheng <ycheng@google.com>
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Acked-by: default avatarNeal Cardwell <ncardwell@google.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 4394542c
...@@ -113,6 +113,7 @@ int sysctl_tcp_early_retrans __read_mostly = 2; ...@@ -113,6 +113,7 @@ int sysctl_tcp_early_retrans __read_mostly = 2;
#define FLAG_DSACKING_ACK 0x800 /* SACK blocks contained D-SACK info */ #define FLAG_DSACKING_ACK 0x800 /* SACK blocks contained D-SACK info */
#define FLAG_NONHEAD_RETRANS_ACKED 0x1000 /* Non-head rexmitted data was ACKed */ #define FLAG_NONHEAD_RETRANS_ACKED 0x1000 /* Non-head rexmitted data was ACKed */
#define FLAG_SACK_RENEGING 0x2000 /* snd_una advanced to a sacked seq */ #define FLAG_SACK_RENEGING 0x2000 /* snd_una advanced to a sacked seq */
#define FLAG_UPDATE_TS_RECENT 0x4000 /* tcp_replace_ts_recent() */
#define FLAG_ACKED (FLAG_DATA_ACKED|FLAG_SYN_ACKED) #define FLAG_ACKED (FLAG_DATA_ACKED|FLAG_SYN_ACKED)
#define FLAG_NOT_DUP (FLAG_DATA|FLAG_WIN_UPDATE|FLAG_ACKED) #define FLAG_NOT_DUP (FLAG_DATA|FLAG_WIN_UPDATE|FLAG_ACKED)
...@@ -3564,6 +3565,27 @@ static void tcp_send_challenge_ack(struct sock *sk) ...@@ -3564,6 +3565,27 @@ static void tcp_send_challenge_ack(struct sock *sk)
} }
} }
static void tcp_store_ts_recent(struct tcp_sock *tp)
{
tp->rx_opt.ts_recent = tp->rx_opt.rcv_tsval;
tp->rx_opt.ts_recent_stamp = get_seconds();
}
static void tcp_replace_ts_recent(struct tcp_sock *tp, u32 seq)
{
if (tp->rx_opt.saw_tstamp && !after(seq, tp->rcv_wup)) {
/* PAWS bug workaround wrt. ACK frames, the PAWS discard
* extra check below makes sure this can only happen
* for pure ACK frames. -DaveM
*
* Not only, also it occurs for expired timestamps.
*/
if (tcp_paws_check(&tp->rx_opt, 0))
tcp_store_ts_recent(tp);
}
}
/* This routine deals with incoming acks, but not outgoing ones. */ /* This routine deals with incoming acks, but not outgoing ones. */
static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag) static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
{ {
...@@ -3607,6 +3629,12 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag) ...@@ -3607,6 +3629,12 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
prior_fackets = tp->fackets_out; prior_fackets = tp->fackets_out;
prior_in_flight = tcp_packets_in_flight(tp); prior_in_flight = tcp_packets_in_flight(tp);
/* ts_recent update must be made after we are sure that the packet
* is in window.
*/
if (flag & FLAG_UPDATE_TS_RECENT)
tcp_replace_ts_recent(tp, TCP_SKB_CB(skb)->seq);
if (!(flag & FLAG_SLOWPATH) && after(ack, prior_snd_una)) { if (!(flag & FLAG_SLOWPATH) && after(ack, prior_snd_una)) {
/* Window is constant, pure forward advance. /* Window is constant, pure forward advance.
* No more checks are required. * No more checks are required.
...@@ -3927,27 +3955,6 @@ const u8 *tcp_parse_md5sig_option(const struct tcphdr *th) ...@@ -3927,27 +3955,6 @@ const u8 *tcp_parse_md5sig_option(const struct tcphdr *th)
EXPORT_SYMBOL(tcp_parse_md5sig_option); EXPORT_SYMBOL(tcp_parse_md5sig_option);
#endif #endif
static inline void tcp_store_ts_recent(struct tcp_sock *tp)
{
tp->rx_opt.ts_recent = tp->rx_opt.rcv_tsval;
tp->rx_opt.ts_recent_stamp = get_seconds();
}
static inline void tcp_replace_ts_recent(struct tcp_sock *tp, u32 seq)
{
if (tp->rx_opt.saw_tstamp && !after(seq, tp->rcv_wup)) {
/* PAWS bug workaround wrt. ACK frames, the PAWS discard
* extra check below makes sure this can only happen
* for pure ACK frames. -DaveM
*
* Not only, also it occurs for expired timestamps.
*/
if (tcp_paws_check(&tp->rx_opt, 0))
tcp_store_ts_recent(tp);
}
}
/* Sorry, PAWS as specified is broken wrt. pure-ACKs -DaveM /* Sorry, PAWS as specified is broken wrt. pure-ACKs -DaveM
* *
* It is not fatal. If this ACK does _not_ change critical state (seqs, window) * It is not fatal. If this ACK does _not_ change critical state (seqs, window)
...@@ -5543,14 +5550,9 @@ int tcp_rcv_established(struct sock *sk, struct sk_buff *skb, ...@@ -5543,14 +5550,9 @@ int tcp_rcv_established(struct sock *sk, struct sk_buff *skb,
return 0; return 0;
step5: step5:
if (tcp_ack(sk, skb, FLAG_SLOWPATH) < 0) if (tcp_ack(sk, skb, FLAG_SLOWPATH | FLAG_UPDATE_TS_RECENT) < 0)
goto discard; goto discard;
/* ts_recent update must be made after we are sure that the packet
* is in window.
*/
tcp_replace_ts_recent(tp, TCP_SKB_CB(skb)->seq);
tcp_rcv_rtt_measure_ts(sk, skb); tcp_rcv_rtt_measure_ts(sk, skb);
/* Process urgent data. */ /* Process urgent data. */
...@@ -5986,7 +5988,8 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb, ...@@ -5986,7 +5988,8 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
/* step 5: check the ACK field */ /* step 5: check the ACK field */
if (true) { if (true) {
int acceptable = tcp_ack(sk, skb, FLAG_SLOWPATH) > 0; int acceptable = tcp_ack(sk, skb, FLAG_SLOWPATH |
FLAG_UPDATE_TS_RECENT) > 0;
switch (sk->sk_state) { switch (sk->sk_state) {
case TCP_SYN_RECV: case TCP_SYN_RECV:
...@@ -6137,11 +6140,6 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb, ...@@ -6137,11 +6140,6 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
} }
} }
/* ts_recent update must be made after we are sure that the packet
* is in window.
*/
tcp_replace_ts_recent(tp, TCP_SKB_CB(skb)->seq);
/* step 6: check the URG bit */ /* step 6: check the URG bit */
tcp_urg(sk, skb, th); tcp_urg(sk, skb, th);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment