diff mbox

TCP_DEFER_ACCEPT is missing counter update

Message ID 4AD84D8B.5020103@gmail.com
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Dumazet Oct. 16, 2009, 10:40 a.m. UTC
Julian Anastasov a écrit :
> 	Hello,
> 
> On Fri, 16 Oct 2009, Willy Tarreau wrote:
> 
>>> 	This will need little change in inet_csk_reqsk_queue_prune()
>>> but it saves SYN-ACK traffic during deferring period in the
>>> common case when client sends ACK. If such compromise is
>>> acceptable I can prepare and test some patch.
>> I would personally like this a lot ! This will satisfy people who
>> expect it to establish at the end of the "TCP_DEFER_ACCEPT delay"
>> as can be interpreted from the man page, will reduce the number of
>> useless SYN-ACKs that annoy other people while still making no
>> visible change for anyone who would rely on the current behaviour.
> 
> 	OK, I don't have much time now, this is what I'm
> going to test later today and later can provide proper comments:
> 
> Signed-off-by: Julian Anastasov <ja@ssi.bg>

I tested both patches and they perform very well, thank you !

For the minimum 1 sec value, tcpdump looks like :
12:32:03.850456 IP 127.0.0.1.20000 > 127.0.0.1.2222: S 1879889239:1879889239(0) win 32792 <mss 16396,nop,nop,timestamp 952803 0,nop,wscale 6>
12:32:03.850463 IP 127.0.0.1.2222 > 127.0.0.1.20000: S 1890330616:1890330616(0) ack 1879889240 win 32768 <mss 16396,nop,nop,timestamp 952803 952803,nop,wscale 6>
12:32:03.850469 IP 127.0.0.1.20000 > 127.0.0.1.2222: . ack 1 win 513 <nop,nop,timestamp 952803 952803>

12:32:06.849989 IP 127.0.0.1.2222 > 127.0.0.1.20000: S 1890330616:1890330616(0) ack 1879889240 win 32768 <mss 16396,nop,nop,timestamp 955803 952803,nop,wscale 6>
12:32:06.849996 IP 127.0.0.1.20000 > 127.0.0.1.2222: . ack 1 win 513 <nop,nop,timestamp 955803 955803>

So listening application gets the accept() 3 seconds after initial SYN


# ss -emoian | grep SYN-RECV
SYN-RECV   0      0                 127.0.0.1:2222             127.0.0.1:20000  timer:(on,24sec,4) ino:0 sk:f6f0ec80

I wonder if tcp_diag should be extented a bit to reflect fact that the ACK was received from client
(ie forward the inet_rsk(req)->acked information to idiag_rqueue)




-> 
# ss -emoian | grep SYN-RECV
SYN-RECV   1      0                 127.0.0.1:2222             127.0.0.1:20000  timer:(on,24sec,4) ino:0 sk:f6f0ec80

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

Comments

Willy Tarreau Oct. 16, 2009, 7:27 p.m. UTC | #1
Hi,

On Fri, Oct 16, 2009 at 12:40:11PM +0200, Eric Dumazet wrote:
> Julian Anastasov a écrit :
> > 	Hello,
> > 
> > On Fri, 16 Oct 2009, Willy Tarreau wrote:
> > 
> >>> 	This will need little change in inet_csk_reqsk_queue_prune()
> >>> but it saves SYN-ACK traffic during deferring period in the
> >>> common case when client sends ACK. If such compromise is
> >>> acceptable I can prepare and test some patch.
> >> I would personally like this a lot ! This will satisfy people who
> >> expect it to establish at the end of the "TCP_DEFER_ACCEPT delay"
> >> as can be interpreted from the man page, will reduce the number of
> >> useless SYN-ACKs that annoy other people while still making no
> >> visible change for anyone who would rely on the current behaviour.
> > 
> > 	OK, I don't have much time now, this is what I'm
> > going to test later today and later can provide proper comments:
> > 
> > Signed-off-by: Julian Anastasov <ja@ssi.bg>
> 
> I tested both patches and they perform very well, thank you !
> 
> For the minimum 1 sec value, tcpdump looks like :
> 12:32:03.850456 IP 127.0.0.1.20000 > 127.0.0.1.2222: S 1879889239:1879889239(0) win 32792 <mss 16396,nop,nop,timestamp 952803 0,nop,wscale 6>
> 12:32:03.850463 IP 127.0.0.1.2222 > 127.0.0.1.20000: S 1890330616:1890330616(0) ack 1879889240 win 32768 <mss 16396,nop,nop,timestamp 952803 952803,nop,wscale 6>
> 12:32:03.850469 IP 127.0.0.1.20000 > 127.0.0.1.2222: . ack 1 win 513 <nop,nop,timestamp 952803 952803>
> 
> 12:32:06.849989 IP 127.0.0.1.2222 > 127.0.0.1.20000: S 1890330616:1890330616(0) ack 1879889240 win 32768 <mss 16396,nop,nop,timestamp 955803 952803,nop,wscale 6>
> 12:32:06.849996 IP 127.0.0.1.20000 > 127.0.0.1.2222: . ack 1 win 513 <nop,nop,timestamp 955803 955803>
> 
> So listening application gets the accept() 3 seconds after initial SYN

Excellent! Nice work guys.

> # ss -emoian | grep SYN-RECV
> SYN-RECV   0      0                 127.0.0.1:2222             127.0.0.1:20000  timer:(on,24sec,4) ino:0 sk:f6f0ec80
> 
> I wonder if tcp_diag should be extented a bit to reflect fact that the ACK was received from client
> (ie forward the inet_rsk(req)->acked information to idiag_rqueue)

I personally have no opinion on this point.

Thanks!
Willy

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

Patch

diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index cb73fde..c172bd4 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -589,7 +589,7 @@  static int inet_diag_fill_req(struct sk_buff *skb, struct sock *sk,
 	r->id.idiag_src[0] = ireq->loc_addr;
 	r->id.idiag_dst[0] = ireq->rmt_addr;
 	r->idiag_expires = jiffies_to_msecs(tmo);
- 	r->idiag_rqueue = 0;
+	r->idiag_rqueue = ireq->acked;
 	r->idiag_wqueue = 0;
 	r->idiag_uid = sock_i_uid(sk);
 	r->idiag_inode = 0;