diff mbox

[net-next] tcp: fix rcv_ssthresh regression for medium-sized initial packets

Message ID 1351006545-12556-1-git-send-email-ncardwell@google.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Neal Cardwell Oct. 23, 2012, 3:35 p.m. UTC
The recent skb truesize fixes caused a slight regression in that
moderately-sized initial incoming packets (eg ~1000 bytes) can now
lead to smaller rcv_ssthresh increments than in 2.6.

The mitigation b49960a05e32121d ("tcp: change tcp_adv_win_scale and
tcp_rmem[2]") compensates for the truesize fixes by fixing the
ultimate value to which receive window converges. But this commit did
not mitigate the impact for the initial cwnd growth phase for
connetions with moderately-sized initial incoming packets. Such
connections still hit the __tcp_grow_window() code path, where the
increment to rcv_sssthresh was by rcv_mss, and not the generally
larger advmss.

We should not handicap this initial growth rate of the receive
window. That way short connections are not penalized in cases where
they wouldn't lead to receive buffer memory consumption issues.

Signed-off-by: Neal Cardwell <ncardwell@google.com>
CC: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp_input.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Eric Dumazet Oct. 23, 2012, 3:57 p.m. UTC | #1
On Tue, 2012-10-23 at 11:35 -0400, Neal Cardwell wrote:
> The recent skb truesize fixes caused a slight regression in that
> moderately-sized initial incoming packets (eg ~1000 bytes) can now
> lead to smaller rcv_ssthresh increments than in 2.6.
> 
> The mitigation b49960a05e32121d ("tcp: change tcp_adv_win_scale and
> tcp_rmem[2]") compensates for the truesize fixes by fixing the
> ultimate value to which receive window converges. But this commit did
> not mitigate the impact for the initial cwnd growth phase for
> connetions with moderately-sized initial incoming packets. Such
> connections still hit the __tcp_grow_window() code path, where the
> increment to rcv_sssthresh was by rcv_mss, and not the generally
> larger advmss.
> 
> We should not handicap this initial growth rate of the receive
> window. That way short connections are not penalized in cases where
> they wouldn't lead to receive buffer memory consumption issues.
> 
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> CC: Eric Dumazet <edumazet@google.com>
> ---
>  net/ipv4/tcp_input.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 60cf836..e5981a3 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -316,7 +316,7 @@ static int __tcp_grow_window(const struct sock *sk, const struct sk_buff *skb)
>  
>  	while (tp->rcv_ssthresh <= window) {
>  		if (truesize <= skb->len)
> -			return 2 * inet_csk(sk)->icsk_ack.rcv_mss;
> +			return 2 * tp->advmss;
>  
>  		truesize >>= 1;
>  		window >>= 1;

But this defeats whole point of having a small initial window and
increase it if packets are 'good citizens' ?

Are you sure we wont drop packets later because we hit sk->sk_rcvbuf
limit ?

This brings back the receiver initial window we discussed lately.

Because if we had an initial window of 32KB or 64KB instead of 10*1460,
I am pretty sure we would not have to change this code ?

Thanks


--
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/tcp_input.c b/net/ipv4/tcp_input.c
index 60cf836..e5981a3 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -316,7 +316,7 @@  static int __tcp_grow_window(const struct sock *sk, const struct sk_buff *skb)
 
 	while (tp->rcv_ssthresh <= window) {
 		if (truesize <= skb->len)
-			return 2 * inet_csk(sk)->icsk_ack.rcv_mss;
+			return 2 * tp->advmss;
 
 		truesize >>= 1;
 		window >>= 1;