lkml.org 
[lkml]   [2013]   [May]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[097/118] tcp: call tcp_replace_ts_recent() from tcp_ack()
    3.2.45-rc1 review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Eric Dumazet <edumazet@google.com>

    [ Upstream commit 12fb3dd9dc3c64ba7d64cec977cca9b5fb7b1d4e ]

    commit bd090dfc634d (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: Yuchung Cheng <ycheng@google.com>
    Signed-off-by: Eric Dumazet <edumazet@google.com>
    Cc: Neal Cardwell <ncardwell@google.com>
    Acked-by: Neal Cardwell <ncardwell@google.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    net/ipv4/tcp_input.c | 65 ++++++++++++++++++++++++++--------------------------
    1 file changed, 32 insertions(+), 33 deletions(-)

    --- a/net/ipv4/tcp_input.c
    +++ b/net/ipv4/tcp_input.c
    @@ -115,6 +115,7 @@ int sysctl_tcp_abc __read_mostly;
    #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_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_NOT_DUP (FLAG_DATA|FLAG_WIN_UPDATE|FLAG_ACKED)
    @@ -3723,6 +3724,27 @@ static void tcp_send_challenge_ack(struc
    }
    }

    +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. */
    static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
    {
    @@ -3771,6 +3793,12 @@ static int tcp_ack(struct sock *sk, cons
    prior_fackets = tp->fackets_out;
    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)) {
    /* Window is constant, pure forward advance.
    * No more checks are required.
    @@ -4061,27 +4089,6 @@ const u8 *tcp_parse_md5sig_option(const
    EXPORT_SYMBOL(tcp_parse_md5sig_option);
    #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
    *
    * It is not fatal. If this ACK does _not_ change critical state (seqs, window)
    @@ -5552,14 +5559,10 @@ slow_path:
    return 0;

    step5:
    - if (th->ack && tcp_ack(sk, skb, FLAG_SLOWPATH) < 0)
    + if (th->ack &&
    + tcp_ack(sk, skb, FLAG_SLOWPATH | FLAG_UPDATE_TS_RECENT) < 0)
    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);

    /* Process urgent data. */
    @@ -5923,7 +5926,8 @@ int tcp_rcv_state_process(struct sock *s

    /* step 5: check the ACK field */
    if (th->ack) {
    - 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) {
    case TCP_SYN_RECV:
    @@ -6030,11 +6034,6 @@ int tcp_rcv_state_process(struct sock *s
    } else
    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);
    -
    /* step 6: check the URG bit */
    tcp_urg(sk, skb, th);



    \
     
     \ /
      Last update: 2013-05-10 19:21    [W:2.571 / U:1.228 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site