netdev - Re: [PATCH net-next V2 1/1] tcp: Prevent needless syn-ack rexmt during TWHS

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:	Fri, 26 Oct 2012 17:07:26 -0700
From:	Vijay Subramanian <subramanian.vijay@...il.com>
To:	Julian Anastasov <ja@....bg>
Cc:	Eric Dumazet <eric.dumazet@...il.com>, netdev@...r.kernel.org,
	davem@...emloft.net, edumazet@...gle.com, ncardwell@...gle.com,
	Venkat Venkatsubra <venkat.x.venkatsubra@...cle.com>,
	Elliott Hughes <enh@...gle.com>
Subject: Re: [PATCH net-next V2 1/1] tcp: Prevent needless syn-ack rexmt
 during TWHS

Julian,
Thanks for the review.

>
>         We have 3 general cases:
>
> 1. HTTP-kind of protocol: client sends first, server without
> TCP_DEFER_ACCEPT
>
>         Server should retransmit but anyways client will
> send packet (ACK) that will move request_sock into child socket.

In  this case, is this retransmit from server and ack from client
necessary? I believe that when client finally sends fourth packet i.e.
data, server socket (currently request_sock) can move into accept
queue and into ESTABLISHED state. So this patch just removes
the syn-ack /ack transmissions in between.

>
> 2. HTTP-kind of protocol: client sends first, server with
> TCP_DEFER_ACCEPT
>
>         Server retransmits before 3WHS to get ACK from
> client. After ACK we keep request_sock because we do not
> want to wakeup listener without data. During TCP_DEFER_ACCEPT
> period nothing is retransmitted because we have ACK from
> client. After TCP_DEFER_ACCEPT period we start retransmissions
> because the server needs such request_socks to become
> child sockets after the TCP_DEFER_ACCEPT period and because
> received ACK is the only way to create child socket.
> Server wants to accept() them.

As I mentioned, if client is sending data first, then the data packet
from client will also cause the creation of child socket.
If server sends the synack again, if client is not ready with data, it
will just send an ack back. This is a needless synack/ack.
On the other hand if client is ready with data, it will send it and
server socket will create child socket if accept queue has space.

Even with DEFER_ACCEPT sockets, if a third ack comes in without data,
it is remembered in inet_rsk(req)->acked. So,
logic is the same. If inet_rsk(req)->acked==1, it means we have
received third ack and client is in ESTABLISHED state.
So why bother resending needless synacks?

> If TCP_DEFER_ACCEPT is
> above sysctl_tcp_synack_retries there are no such
> retransmissions because server does not want to accept()
> such request_socks without data. So, servers decide
> what they want with the TCP_DEFER_ACCEPT period value.
>
> 3. SMTP-kind of protocol: server sends first,
> TCP_DEFER_ACCEPT must not be used by server.
>         3WHS is completed, there is no 4th packet from
> client. It is the server side that needs to move request_sock
> to child socket, to accept the connection and to send
> welcome message. AFAIK, child socket is created only on
> ACK. Or I'm missing something? Now the question is:

You raise a good point. As far as I can see, it looks like the
request_sock will not become a full socket
if server is the one that has to send the data first. The problem it
seems is that we always have to wait
for the client to send something to create the full socket. Since
server side already knows that TWHS has finished,
maybe we can find a way to move a request_sock to accept_queue as
space opens up without sending synack.

> how request_sock is moved into child socket if ACK is
> dropped on syn_recv_sock failure and we do not send SYN-ACK
> retransmissions to trigger ACK from client? Client does not
> plan to send new packet in short time.

Agreed. Unless I am missing something too, for cases where server
socket sends data first, the patch will break things though for
defer-sockets
it seems ok. Maybe we need another approach?

Thanks for the feedback. I will wait for suggestions as to what to do
with the patch and have another look at the code.

Regards,
Vijay
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html