Thread (9 messages) 9 messages, 3 authors, 2017-08-01

Re: [PATCH net-next 4/6] tcp: remove header prediction

From: Neal Cardwell <ncardwell@google.com>
Date: 2017-08-01 17:24:27

On Sat, Jul 29, 2017 at 9:57 PM, Florian Westphal [off-list ref] wrote:
quoted hunk ↗ jump to hunk
@@ -5519,11 +5347,10 @@ void tcp_finish_connect(struct sock *sk, struct sk_buff *skb)
        if (sock_flag(sk, SOCK_KEEPOPEN))
                inet_csk_reset_keepalive_timer(sk, keepalive_time_when(tp));

-       if (!tp->rx_opt.snd_wscale)
-               __tcp_fast_path_on(tp, tp->snd_wnd);
-       else
-               tp->pred_flags = 0;
-
+       if (!sock_flag(sk, SOCK_DEAD)) {
+               sk->sk_state_change(sk);
+               sk_wake_async(sk, SOCK_WAKE_IO, POLL_OUT);
+       }
 }
This patch hunk seems like it introduces a minor bug. It seems like
after this change, the sk_state_change() and sk_wake_async() calls for
a completed active connection happen twice: once in this new spot
inside tcp_finish_connect() and once in the existing code in
tcp_rcv_synsent_state_process() immediately after it calls
tcp_finish_connect().

I would vote for removing this new code snippet and retaining the old
one, in case there are subtle interactions with
tcp_rcv_fastopen_synack(), which happens in between the new wake-up
location and the old wake-up location.

neal
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help