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

On Sat, 2012-10-27 at 16:23 +0300, Julian Anastasov wrote:

	Hello,

On Sat, 27 Oct 2012, Eric Dumazet wrote:

quoted

Author: Eric Dumazet [off-list ref]
Date:   Tue Oct 2 02:21:12 2012 -0700

net-tcp: better retrans tracking for defer-accept

For passive TCP connections using TCP_DEFER_ACCEPT facility,
we incorrectly increment req->retrans each time timeout triggers
while no SYNACK is sent.

Decouple req->retrans field into two fields :

num_retrans : number of retransmit
num_timeout : number of timeouts

(retrans was renamed to make sure we didnt miss an occurrence)

introduce inet_rtx_syn_ack() helper to increment num_retrans
only if ->rtx_syn_ack() succeeded.

	This is dangerous, the first of the cases is route
failure, what if we just added reject route for some attacker?
We will get error forever. May be it is difficult to decide
which error should change the counter. IMHO, such reliability
is not needed, we can be short of memory too.

We increase num_timeout regardless of success or failure sending a
SYNACK (can be a route failure, a memory allocation failure, a full
qdisc...)

So its not 'forever'. The decision to abort a SYN_RECV is based on
num_timeouts only, not anymore on 'number of restransmits'

num_retrans is only counting number of SYNACKS that were sent.

num_retrans <= num_timeouts

(Usually its the same, unless you have errors, or DEFER_ACCEPT
mini-sockets)

quoted

Use inet_rtx_syn_ack() from tcp_check_req() to increment num_retrans
when we re-send a SYNACK in answer to a SYN. Prior to this patch,
we were not counting these retransmits.

	Such change looks correct. Of course, it has
side effect on current TCP_DEFER_ACCEPT calculations but
it is a TCP_DEFER_ACCEPT implementation problem.

Better wait to see the patch, it changes nothing yet for
TCP_DEFER_ACCEPT

It only changes accounting problems, for more precise tracking of tcp
stack behavior.

TCP_DEFER_ACCEPT sockets have this strange accounting bug saying that
some packets were retransmitted, while its not true : We only were
waiting the user request.