* [PATCH net-next 2/4] tcp: TCP Fast Open Server - take SYNACK RTT after completing 3WHS
2012-09-22 14:18 [PATCH net-next 1/4] tcp: extract code to compute SYNACK RTT Neal Cardwell
@ 2012-09-22 14:18 ` Neal Cardwell
2012-09-22 19:47 ` David Miller
2012-09-22 14:18 ` [PATCH net-next 3/4] tcp: TCP Fast Open Server - note timestamps and retransmits for SYNACK RTT Neal Cardwell
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Neal Cardwell @ 2012-09-22 14:18 UTC (permalink / raw)
To: David Miller
Cc: netdev, Eric Dumazet, Yuchung Cheng, Jerry Chu, Neal Cardwell
When taking SYNACK RTT samples for servers using TCP Fast Open, fix
the code to ensure that we only call tcp_valid_rtt_meas() after we
receive the ACK that completes the 3-way handshake.
Previously we were always taking an RTT sample in
tcp_v4_syn_recv_sock(). However, for TCP Fast Open connections
tcp_v4_conn_req_fastopen() calls tcp_v4_syn_recv_sock() at the time we
receive the SYN. So for TFO we must wait until tcp_rcv_state_process()
to take the RTT sample.
To fix this, we wait until after TFO calls tcp_v4_syn_recv_sock()
before we set the snt_synack timestamp, since tcp_synack_rtt_meas()
already ensures that we only take a SYNACK RTT sample if snt_synack is
non-zero. To be careful, we only take a snt_synack timestamp when
a SYNACK transmit or retransmit succeeds.
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
include/net/tcp.h | 1 +
net/ipv4/tcp_input.c | 1 +
net/ipv4/tcp_ipv4.c | 12 +++++++++---
net/ipv6/tcp_ipv6.c | 2 +-
4 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index a718d0e..6feeccd 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1125,6 +1125,7 @@ static inline void tcp_openreq_init(struct request_sock *req,
req->cookie_ts = 0;
tcp_rsk(req)->rcv_isn = TCP_SKB_CB(skb)->seq;
tcp_rsk(req)->rcv_nxt = TCP_SKB_CB(skb)->seq + 1;
+ tcp_rsk(req)->snt_synack = 0;
req->mss = rx_opt->mss_clamp;
req->ts_recent = rx_opt->saw_tstamp ? rx_opt->rcv_tsval : 0;
ireq->tstamp_ok = rx_opt->tstamp_ok;
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index e2bec81..bb32668 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5983,6 +5983,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
* need req so release it.
*/
if (req) {
+ tcp_synack_rtt_meas(sk, req);
reqsk_fastopen_remove(sk, req, false);
} else {
/* Make sure socket is routed, for
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 1e66f7f..0a7e020 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -868,6 +868,8 @@ static int tcp_v4_send_synack(struct sock *sk, struct dst_entry *dst,
ireq->rmt_addr,
ireq->opt);
err = net_xmit_eval(err);
+ if (!tcp_rsk(req)->snt_synack && !err)
+ tcp_rsk(req)->snt_synack = tcp_time_stamp;
}
return err;
@@ -1382,6 +1384,7 @@ static int tcp_v4_conn_req_fastopen(struct sock *sk,
struct request_sock_queue *queue = &inet_csk(sk)->icsk_accept_queue;
const struct inet_request_sock *ireq = inet_rsk(req);
struct sock *child;
+ int err;
req->retrans = 0;
req->sk = NULL;
@@ -1393,8 +1396,11 @@ static int tcp_v4_conn_req_fastopen(struct sock *sk,
kfree_skb(skb_synack);
return -1;
}
- ip_build_and_send_pkt(skb_synack, sk, ireq->loc_addr,
- ireq->rmt_addr, ireq->opt);
+ err = ip_build_and_send_pkt(skb_synack, sk, ireq->loc_addr,
+ ireq->rmt_addr, ireq->opt);
+ err = net_xmit_eval(err);
+ if (!err)
+ tcp_rsk(req)->snt_synack = tcp_time_stamp;
/* XXX (TFO) - is it ok to ignore error and continue? */
spin_lock(&queue->fastopenq->lock);
@@ -1612,7 +1618,6 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
isn = tcp_v4_init_sequence(skb);
}
tcp_rsk(req)->snt_isn = isn;
- tcp_rsk(req)->snt_synack = tcp_time_stamp;
if (dst == NULL) {
dst = inet_csk_route_req(sk, &fl4, req);
@@ -1650,6 +1655,7 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
if (err || want_cookie)
goto drop_and_free;
+ tcp_rsk(req)->snt_synack = tcp_time_stamp;
tcp_rsk(req)->listener = NULL;
/* Add the request_sock to the SYN table */
inet_csk_reqsk_queue_hash_add(sk, req, TCP_TIMEOUT_INIT);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index cfeeeb7..d6212d6 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1169,7 +1169,6 @@ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
}
have_isn:
tcp_rsk(req)->snt_isn = isn;
- tcp_rsk(req)->snt_synack = tcp_time_stamp;
if (security_inet_conn_request(sk, skb, req))
goto drop_and_release;
@@ -1180,6 +1179,7 @@ have_isn:
want_cookie)
goto drop_and_free;
+ tcp_rsk(req)->snt_synack = tcp_time_stamp;
tcp_rsk(req)->listener = NULL;
inet6_csk_reqsk_queue_hash_add(sk, req, TCP_TIMEOUT_INIT);
return 0;
--
1.7.7.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next 3/4] tcp: TCP Fast Open Server - note timestamps and retransmits for SYNACK RTT
2012-09-22 14:18 [PATCH net-next 1/4] tcp: extract code to compute SYNACK RTT Neal Cardwell
2012-09-22 14:18 ` [PATCH net-next 2/4] tcp: TCP Fast Open Server - take SYNACK RTT after completing 3WHS Neal Cardwell
@ 2012-09-22 14:18 ` Neal Cardwell
2012-09-22 19:47 ` David Miller
2012-09-22 14:18 ` [PATCH net-next 4/4] tcp: TCP Fast Open Server - call tcp_validate_incoming() for all packets Neal Cardwell
2012-09-22 19:47 ` [PATCH net-next 1/4] tcp: extract code to compute SYNACK RTT David Miller
3 siblings, 1 reply; 8+ messages in thread
From: Neal Cardwell @ 2012-09-22 14:18 UTC (permalink / raw)
To: David Miller
Cc: netdev, Eric Dumazet, Yuchung Cheng, Jerry Chu, Neal Cardwell
Previously, when using TCP Fast Open a server would return from
tcp_check_req() before updating snt_synack based on TCP timestamp echo
replies and whether or not we've retransmitted the SYNACK. The result
was that (a) for TFO connections using timestamps we used an incorrect
baseline SYNACK send time (tcp_time_stamp of SYNACK send instead of
rcv_tsecr), and (b) for TFO connections that do not have TCP
timestamps but retransmit the SYNACK we took a SYNACK RTT sample when
we should not take a sample.
This fix merely moves the snt_synack update logic a bit earlier in the
function, so that connections using TCP Fast Open will properly do
these updates when the ACK for the SYNACK arrives.
Moving this snt_synack update logic means that with TCP_DEFER_ACCEPT
enabled we do a few instructions of wasted work on each bare ACK, but
that seems OK.
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
net/ipv4/tcp_minisocks.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 5792577..27536ba 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -692,6 +692,12 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
if (!(flg & TCP_FLAG_ACK))
return NULL;
+ /* Got ACK for our SYNACK, so update baseline for SYNACK RTT sample. */
+ if (tmp_opt.saw_tstamp && tmp_opt.rcv_tsecr)
+ tcp_rsk(req)->snt_synack = tmp_opt.rcv_tsecr;
+ else if (req->retrans) /* don't take RTT sample if retrans && ~TS */
+ tcp_rsk(req)->snt_synack = 0;
+
/* For Fast Open no more processing is needed (sk is the
* child socket).
*/
@@ -705,10 +711,6 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPDEFERACCEPTDROP);
return NULL;
}
- if (tmp_opt.saw_tstamp && tmp_opt.rcv_tsecr)
- tcp_rsk(req)->snt_synack = tmp_opt.rcv_tsecr;
- else if (req->retrans) /* don't take RTT sample if retrans && ~TS */
- tcp_rsk(req)->snt_synack = 0;
/* OK, ACK is valid, create big socket and
* feed this segment to it. It will repeat all
--
1.7.7.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next 4/4] tcp: TCP Fast Open Server - call tcp_validate_incoming() for all packets
2012-09-22 14:18 [PATCH net-next 1/4] tcp: extract code to compute SYNACK RTT Neal Cardwell
2012-09-22 14:18 ` [PATCH net-next 2/4] tcp: TCP Fast Open Server - take SYNACK RTT after completing 3WHS Neal Cardwell
2012-09-22 14:18 ` [PATCH net-next 3/4] tcp: TCP Fast Open Server - note timestamps and retransmits for SYNACK RTT Neal Cardwell
@ 2012-09-22 14:18 ` Neal Cardwell
2012-09-22 19:48 ` David Miller
2012-09-22 19:47 ` [PATCH net-next 1/4] tcp: extract code to compute SYNACK RTT David Miller
3 siblings, 1 reply; 8+ messages in thread
From: Neal Cardwell @ 2012-09-22 14:18 UTC (permalink / raw)
To: David Miller
Cc: netdev, Eric Dumazet, Yuchung Cheng, Jerry Chu, Neal Cardwell
A TCP Fast Open (TFO) passive connection must call both
tcp_check_req() and tcp_validate_incoming() for all incoming ACKs that
are attempting to complete the 3WHS.
This is needed to parallel all the action that happens for a non-TFO
connection, where for an ACK that is attempting to complete the 3WHS
we call both tcp_check_req() and tcp_validate_incoming().
For example, upon receiving the ACK that completes the 3WHS, we need
to call tcp_fast_parse_options() and update ts_recent based on the
incoming timestamp value in the ACK.
One symptom of the problem with the previous code was that for passive
TFO connections using TCP timestamps, the outgoing TS ecr values
ignored the incoming TS val value on the ACK that completed the 3WHS.
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
net/ipv4/tcp_input.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index bb32668..36e069a1 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5969,7 +5969,8 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
if (tcp_check_req(sk, skb, req, NULL, true) == NULL)
goto discard;
- } else if (!tcp_validate_incoming(sk, skb, th, 0))
+ }
+ if (!tcp_validate_incoming(sk, skb, th, 0))
return 0;
/* step 5: check the ACK field */
--
1.7.7.3
^ permalink raw reply related [flat|nested] 8+ messages in thread