lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 23 Oct 2019 14:59:50 -0700
From:   Yuchung Cheng <ycheng@...gle.com>
To:     Jason Baron <jbaron@...mai.com>
Cc:     David Miller <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        netdev <netdev@...r.kernel.org>,
        Neal Cardwell <ncardwell@...gle.com>,
        Christoph Paasch <cpaasch@...le.com>
Subject: Re: [net-next v2] tcp: add TCP_INFO status for failed client TFO

On Wed, Oct 23, 2019 at 8:11 AM Jason Baron <jbaron@...mai.com> wrote:
>
> The TCPI_OPT_SYN_DATA bit as part of tcpi_options currently reports whether
> or not data-in-SYN was ack'd on both the client and server side. We'd like
> to gather more information on the client-side in the failure case in order
> to indicate the reason for the failure. This can be useful for not only
> debugging TFO, but also for creating TFO socket policies. For example, if
> a middle box removes the TFO option or drops a data-in-SYN, we can
> can detect this case, and turn off TFO for these connections saving the
> extra retransmits.
>
> The newly added tcpi_fastopen_client_fail status is 2 bits and has the
> following 4 states:
>
> 1) TFO_STATUS_UNSPEC
>
> Catch-all state which includes when TFO is disabled via black hole
> detection, which is indicated via LINUX_MIB_TCPFASTOPENBLACKHOLE.
>
> 2) TFO_COOKIE_UNAVAILABLE
>
> If TFO_CLIENT_NO_COOKIE mode is off, this state indicates that no cookie
> is available in the cache.
>
> 3) TFO_DATA_NOT_ACKED
>
> Data was sent with SYN, we received a SYN/ACK but it did not cover the data
> portion. Cookie is not accepted by server because the cookie may be invalid
> or the server may be overloaded.
>
> 4) TFO_SYN_RETRANSMITTED
>
> Data was sent with SYN, we received a SYN/ACK which did not cover the data
> after at least 1 additional SYN was sent (without data). It may be the case
> that a middle-box is dropping data-in-SYN packets. Thus, it would be more
> efficient to not use TFO on this connection to avoid extra retransmits
> during connection establishment.
>
> These new fields do not cover all the cases where TFO may fail, but other
> failures, such as SYN/ACK + data being dropped, will result in the
> connection not becoming established. And a connection blackhole after
> session establishment shows up as a stalled connection.
>
> Signed-off-by: Jason Baron <jbaron@...mai.com>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: Neal Cardwell <ncardwell@...gle.com>
> Cc: Christoph Paasch <cpaasch@...le.com>
> Cc: Yuchung Cheng <ycheng@...gle.com>
> ---
Acked-by: Yuchung Cheng <ycheng@...gle.com>

Thanks

> v2:
>   -use tp->total_retrans instead of syn_drop for the non-tfo case
>   -improve state naming (Neal Cardwell)
>   -s/TFO_NO_COOKIE_SENT/TFO_COOKIE_UNAVAILABLE - exclude black-hole
>    from this state
>
>  net/ipv4/tcp_fastopen.c  |  5 ++++-
>  net/ipv4/tcp_input.c     |  4 ++++
>  5 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 99617e5..7790f28 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -223,7 +223,7 @@ struct tcp_sock {
>                 fastopen_connect:1, /* FASTOPEN_CONNECT sockopt */
>                 fastopen_no_cookie:1, /* Allow send/recv SYN+data without a cookie */
>                 is_sack_reneg:1,    /* in recovery from loss with SACK reneg? */
> -               unused:2;
> +               fastopen_client_fail:2; /* reason why fastopen failed */
>         u8      nonagle     : 4,/* Disable Nagle algorithm?             */
>                 thin_lto    : 1,/* Use linear timeouts for thin streams */
>                 recvmsg_inq : 1,/* Indicate # of bytes in queue upon recvmsg */
> diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
> index 81e6979..74af1f7 100644
> --- a/include/uapi/linux/tcp.h
> +++ b/include/uapi/linux/tcp.h
> @@ -155,6 +155,14 @@ enum {
>         TCP_QUEUES_NR,
>  };
>
> +/* why fastopen failed from client perspective */
> +enum tcp_fastopen_client_fail {
> +       TFO_STATUS_UNSPEC, /* catch-all */
> +       TFO_COOKIE_UNAVAILABLE, /* if not in TFO_CLIENT_NO_COOKIE mode */
> +       TFO_DATA_NOT_ACKED, /* SYN-ACK did not ack SYN data */
> +       TFO_SYN_RETRANSMITTED, /* SYN-ACK did not ack SYN data after timeout */
> +};
> +
>  /* for TCP_INFO socket option */
>  #define TCPI_OPT_TIMESTAMPS    1
>  #define TCPI_OPT_SACK          2
> @@ -211,7 +219,7 @@ struct tcp_info {
>         __u8    tcpi_backoff;
>         __u8    tcpi_options;
>         __u8    tcpi_snd_wscale : 4, tcpi_rcv_wscale : 4;
> -       __u8    tcpi_delivery_rate_app_limited:1;
> +       __u8    tcpi_delivery_rate_app_limited:1, tcpi_fastopen_client_fail:2;
>
>         __u32   tcpi_rto;
>         __u32   tcpi_ato;
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 9f41a76..bb5fc97 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2657,6 +2657,7 @@ int tcp_disconnect(struct sock *sk, int flags)
>         /* Clean up fastopen related fields */
>         tcp_free_fastopen_req(tp);
>         inet->defer_connect = 0;
> +       tp->fastopen_client_fail = 0;
>
>         WARN_ON(inet->inet_num && !icsk->icsk_bind_hash);
>
> @@ -3296,6 +3297,7 @@ void tcp_get_info(struct sock *sk, struct tcp_info *info)
>         info->tcpi_reord_seen = tp->reord_seen;
>         info->tcpi_rcv_ooopack = tp->rcv_ooopack;
>         info->tcpi_snd_wnd = tp->snd_wnd;
> +       info->tcpi_fastopen_client_fail = tp->fastopen_client_fail;
>         unlock_sock_fast(sk, slow);
>  }
>  EXPORT_SYMBOL_GPL(tcp_get_info);
> diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
> index 3fd4512..dabd2f1 100644
> --- a/net/ipv4/tcp_fastopen.c
> +++ b/net/ipv4/tcp_fastopen.c
> @@ -422,7 +422,10 @@ bool tcp_fastopen_cookie_check(struct sock *sk, u16 *mss,
>                 cookie->len = -1;
>                 return true;
>         }
> -       return cookie->len > 0;
> +       if (cookie->len > 0)
> +               return true;
> +       tcp_sk(sk)->fastopen_client_fail = TFO_COOKIE_UNAVAILABLE;
> +       return false;
>  }
>
>  /* This function checks if we want to defer sending SYN until the first
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 3578357a..d562774 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -5812,6 +5812,10 @@ static bool tcp_rcv_fastopen_synack(struct sock *sk, struct sk_buff *synack,
>         tcp_fastopen_cache_set(sk, mss, cookie, syn_drop, try_exp);
>
>         if (data) { /* Retransmit unacked data in SYN */
> +               if (tp->total_retrans)
> +                       tp->fastopen_client_fail = TFO_SYN_RETRANSMITTED;
> +               else
> +                       tp->fastopen_client_fail = TFO_DATA_NOT_ACKED;
>                 skb_rbtree_walk_from(data) {
>                         if (__tcp_retransmit_skb(sk, data, 1))
>                                 break;
> --
> 2.7.4
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ