netdev - Re: [net-next] tcp: add TCP_INFO status for failed client TFO

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADVnQykxoK4JarMN_5Lps8YobTtoJvVstehbCZQ4P3hAGUQ+Uw@mail.gmail.com>
Date:   Mon, 21 Oct 2019 22:13:52 -0400
From:   Neal Cardwell <ncardwell@...gle.com>
To:     Jason Baron <jbaron@...mai.com>
Cc:     Eric Dumazet <edumazet@...gle.com>,
        Christoph Paasch <cpaasch@...le.com>,
        Yuchung Cheng <ycheng@...gle.com>,
        David Miller <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>
Subject: Re: [net-next] tcp: add TCP_INFO status for failed client TFO

On Mon, Oct 21, 2019 at 5:11 PM Jason Baron <jbaron@...mai.com> wrote:
>
>
>
> On 10/21/19 4:36 PM, Eric Dumazet wrote:
> > On Mon, Oct 21, 2019 at 12:53 PM Christoph Paasch <cpaasch@...le.com> wrote:
> >>
> >
> >> Actually, longterm I hope we would be able to get rid of the
> >> blackhole-detection and fallback heuristics. In a far distant future where
> >> these middleboxes have been weeded out ;-)
> >>
> >> So, do we really want to eternalize this as part of the API in tcp_info ?
> >>
> >
> > A getsockopt() option won't be available for netlink diag (ss command).
> >
> > So it all depends on plans to use this FASTOPEN information ?
> >
>
> The original proposal I had 4 states of interest:
>
> First, we are interested in knowing when a socket has TFO set but
> actually requires a retransmit of a non-TFO syn to become established.
> In this case, we'd be better off not using TFO.
>
> A second case is when the server immediately rejects the DATA and just
> acks the syn (but not the data). Again in that case, we don't want to be
> sending syn+data.
>
> The third case was whether or not we sent a cookie. Perhaps, the server
> doesn't have TFO enabled in which case, it really doesn't make make
> sense to enable TFO in the first place. Or if one also controls the
> server its helpful in understanding if the server is mis-configured. So
> that was the motivation I had for the original four states that I
> proposed (final state was a catch-all state).
>
> Yuchung suggested dividing the 3rd case into 2 for - no cookie sent
> because of blackhole or no cookie sent because its not in cache. And
> then dropping the second state because we already have the
> TCPI_OPT_SYN_DATA bit. However, the TCPI_OPT_SYN_DATA may not be set
> because we may fallback in tcp_send_syn_data() due to send failure. So
> I'm inclined to say that the second state is valuable. And since
> blackhole is already a global thing via MIB, I didn't see a strong need
> for it. But it sounded like Yuchung had an interest in it, and I'd
> obviously like a set of states that is generally useful.

I have not kept up with all the proposals in this thread, but would it
work to include all of the cases proposed in this thread? It seems
like there are enough bits available in holes in tcp_sock and tcp_info
to have up to 7 bits of information saved in tcp_sock and exported to
tcp_info. That seems like more than enough?

The pahole tool shows the following small holes in tcp_sock:

        u8                         compressed_ack;       /*  1530     1 */

        /* XXX 1 byte hole, try to pack */

        u32                        chrono_start;         /*  1532     4 */
...
        u8                         bpf_sock_ops_cb_flags; /*  2076     1 */

        /* XXX 3 bytes hole, try to pack */

        u32                        rcv_ooopack;          /*  2080     4 */

...and the following hole in tcp_info:
        __u8                       tcpi_delivery_rate_app_limited:1;
/*     7: 7  1 */

        /* XXX 7 bits hole, try to pack */

        __u32                      tcpi_rto;             /*     8     4 */

cheers,
neal