On Mon, Oct 21, 2019 at 7:14 PM Neal Cardwell <ncardw...@google.com> wrote: > > On Mon, Oct 21, 2019 at 5:11 PM Jason Baron <jba...@akamai.com> wrote: > > > > > > > > On 10/21/19 4:36 PM, Eric Dumazet wrote: > > > On Mon, Oct 21, 2019 at 12:53 PM Christoph Paasch <cpaa...@apple.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 but sendto would return -1 w/ EINPROGRESS in this case already so the application shouldn't expect TCPI_OPT_SYN_DATA?
> > 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? I would rather use only at most 2-bits for TFO errors to be parsimonious on (bloated) tcp sock. I don't mind if the next patch skip my idea of BH detection. my experience is reading host global stats for most applications are a hassle (or sometimes not even feasible). they mostly care about information of their own sockets. > > 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