[PATCH] tcp: fix SYN-data space mis-accounting

[PATCH] tcp: fix SYN-data space mis-accounting

[Yuchung Cheng]

In fast open the sender unncessarily reduces the space available
for data in SYN by 12 bytes.  This is because in the sender
incorrectly reserves space for TS option twice in tcp_send_syn_data():
tcp_mtu_to_mss() already accounts for TS option space. But it further
reserves MAX_TCP_OPTION_SPACE when computing the payload space.

Signed-off-by: Yuchung Cheng <ycheng@google.com>
---
 net/ipv4/tcp_output.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index fd0cea1..e2b4461 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1351,8 +1351,8 @@ int tcp_trim_head(struct sock *sk, struct sk_buff *skb, u32 len)
 	return 0;
 }
 
-/* Calculate MSS. Not accounting for SACKs here.  */
-int tcp_mtu_to_mss(struct sock *sk, int pmtu)
+/* Calculate MSS not accounting any TCP options.  */
+static inline int __tcp_mtu_to_mss(struct sock *sk, int pmtu)
 {
 	const struct tcp_sock *tp = tcp_sk(sk);
 	const struct inet_connection_sock *icsk = inet_csk(sk);
@@ -1381,13 +1381,17 @@ int tcp_mtu_to_mss(struct sock *sk, int pmtu)
 	/* Then reserve room for full set of TCP options and 8 bytes of data */
 	if (mss_now < 48)
 		mss_now = 48;
-
-	/* Now subtract TCP options size, not including SACKs */
-	mss_now -= tp->tcp_header_len - sizeof(struct tcphdr);
-
 	return mss_now;
 }
 
+/* Calculate MSS. Not accounting for SACKs here.  */
+int tcp_mtu_to_mss(struct sock *sk, int pmtu)
+{
+	/* Subtract TCP options size, not including SACKs */
+	return __tcp_mtu_to_mss(sk, pmtu) -
+	       (tcp_sk(sk)->tcp_header_len - sizeof(struct tcphdr));
+}
+
 /* Inverse of above */
 int tcp_mss_to_mtu(struct sock *sk, int mss)
 {
@@ -2930,7 +2934,7 @@ static int tcp_send_syn_data(struct sock *sk, struct sk_buff *syn)
 	 */
 	if (tp->rx_opt.user_mss && tp->rx_opt.user_mss < tp->rx_opt.mss_clamp)
 		tp->rx_opt.mss_clamp = tp->rx_opt.user_mss;
-	space = tcp_mtu_to_mss(sk, inet_csk(sk)->icsk_pmtu_cookie) -
+	space = __tcp_mtu_to_mss(sk, inet_csk(sk)->icsk_pmtu_cookie) -
 		MAX_TCP_OPTION_SPACE;
 
 	syn_data = skb_copy_expand(syn, skb_headroom(syn), space,
-- 
1.8.1.3

Re: [PATCH] tcp: fix SYN-data space mis-accounting

[Neal Cardwell]

On Fri, Feb 22, 2013 at 1:59 PM, Yuchung Cheng <ycheng@google.com> wrote:
> In fast open the sender unncessarily reduces the space available
> for data in SYN by 12 bytes.  This is because in the sender
> incorrectly reserves space for TS option twice in tcp_send_syn_data():
> tcp_mtu_to_mss() already accounts for TS option space. But it further
> reserves MAX_TCP_OPTION_SPACE when computing the payload space.
>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>

Acked-by: Neal Cardwell <ncardwell@google.com>

Re: [PATCH] tcp: fix SYN-data space mis-accounting

[Eric Dumazet]

On Fri, 2013-02-22 at 10:59 -0800, Yuchung Cheng wrote:
> In fast open the sender unncessarily reduces the space available
> for data in SYN by 12 bytes.  This is because in the sender
> incorrectly reserves space for TS option twice in tcp_send_syn_data():
> tcp_mtu_to_mss() already accounts for TS option space. But it further
> reserves MAX_TCP_OPTION_SPACE when computing the payload space.
> 
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> ---

Acked-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH] tcp: fix SYN-data space mis-accounting

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 22 Feb 2013 11:40:58 -0800

> On Fri, 2013-02-22 at 10:59 -0800, Yuchung Cheng wrote:
>> In fast open the sender unncessarily reduces the space available
>> for data in SYN by 12 bytes.  This is because in the sender
>> incorrectly reserves space for TS option twice in tcp_send_syn_data():
>> tcp_mtu_to_mss() already accounts for TS option space. But it further
>> reserves MAX_TCP_OPTION_SPACE when computing the payload space.
>> 
>> Signed-off-by: Yuchung Cheng <ycheng@google.com>
>> ---
> 
> Acked-by: Eric Dumazet <edumazet@google.com>

Applied, and queue up for -stable, thanks everyone.