netdev - Re: [PATCH net-next] tcp: fix tcp_should_expand_sndbuf() to use tcp_packets_in

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAA93jw7qV-SgjD4rZJGKeP-pt_4NBkP3eB1c+ewYUWfquUA3QQ@mail.gmail.com>
Date:	Fri, 20 Feb 2015 11:44:47 -0800
From:	Dave Taht <dave.taht@...il.com>
To:	Neal Cardwell <ncardwell@...gle.com>
Cc:	David Miller <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Yuchung Cheng <ycheng@...gle.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Nandita Dukkipati <nanditad@...gle.com>
Subject: Re: [PATCH net-next] tcp: fix tcp_should_expand_sndbuf() to use tcp_packets_in_flight()

On Fri, Feb 20, 2015 at 10:33 AM, Neal Cardwell <ncardwell@...gle.com> wrote:
> tcp_should_expand_sndbuf() does not expand the send buffer if we have
> filled the congestion window.
>
> However, it should use tcp_packets_in_flight() instead of
> tp->packets_out to make this check.

oy. is this a candidate for -stable?

> Testing has established that the difference matters a lot if there are
> many SACKed packets, causing a needless performance shortfall.

One of the things I have been meaning to do for a while is go back to
testing some of the original circumstances of bufferbloat,
to see to what extent all the enormous changes to linux tcp itself
since 2010 have made a difference. I vividly remember the sort of
dup ack/sack stuff, and tcp cubic losing it´s mind, we saw back in the
day on the simple tests here:

https://gettys.wordpress.com/2010/12/06/whose-house-is-of-glasse-must-not-throw-stones-at-another/

Running without fq_codel in place on my gateways is not something that
I do willingly. I am sad to report that the latest comcast services
running at nearly 200mbit down, still exhibit 1.2 seconds of latency
under load on the rrul test, *but* my primary targets for those sorts
of tests in the cloud have not kept up with the latest linuxes.
(hoping to deploy new stuff soon)

I had thought this guy´s dissertation on BBR was pretty promising, also:

http://cpsc.yale.edu/event/dissertation-defensemichael-nowlan

and keep wondering if that code has been more thoroughly evaluated and
tested somewhere.

> Signed-off-by: Neal Cardwell <ncardwell@...gle.com>
> Signed-off-by: Yuchung Cheng <ycheng@...gle.com>
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Signed-off-by: Nandita Dukkipati <nanditad@...gle.com>
> ---
>  net/ipv4/tcp_input.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 8fdd27b..fb4cf8b 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -4770,7 +4770,7 @@ static bool tcp_should_expand_sndbuf(const struct sock *sk)
>                 return false;
>
>         /* If we filled the congestion window, do not expand.  */
> -       if (tp->packets_out >= tp->snd_cwnd)
> +       if (tcp_packets_in_flight(tp) >= tp->snd_cwnd)
>                 return false;
>
>         return true;
> --
> 2.2.0.rc0.207.ga3a616c
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Dave Täht

thttp://www.bufferbloat.net/projects/bloat/wiki/Upcoming_Talks
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html