netdev - Re: TCP and BBR: reproducibly low cwnd and bandwidth

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: <1518987867.55655.15.camel@gmail.com>
Date:   Sun, 18 Feb 2018 13:04:27 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Oleksandr Natalenko <oleksandr@...alenko.name>,
        Eric Dumazet <edumazet@...gle.com>
Cc:     Neal Cardwell <ncardwell@...gle.com>,
        "David S. Miller" <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>,
        Yuchung Cheng <ycheng@...gle.com>,
        Soheil Hassas Yeganeh <soheil@...gle.com>,
        Jerry Chu <hkchu@...gle.com>, Dave Taht <dave.taht@...il.com>
Subject: Re: TCP and BBR: reproducibly low cwnd and bandwidth

On Sat, 2018-02-17 at 10:52 -0800, Eric Dumazet wrote:
> 
> This must be some race condition in the code I added in TCP for self-
> pacing, when a sort timeout is programmed.
> 
> Disabling SG means TCP cooks 1-MSS packets.
> 
> I will take a look, probably after the (long) week-end : Tuesday.

I was able to take a look today, and I believe this is the time to
switch TCP to GSO being always on.

As a bonus, we get speed boost for cubic as well.

Todays high BDP and recent TCP improvements (rtx queue as rb-tree, sack
coalescing, TCP pacing...) all were developed/tested/maintained with
GSO/TSO being the norm.

Can you please test the following patch ?

Note that some cleanups can be done later in TCP stack, removing lots
of legacy stuff.

Also TCP internal-pacing could benefit from something similar to this
fq patch eventually, although there is no hurry.
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=fefa569a9d4bc4b7758c0fddd75bb0382c95da77  

Of course, you have to consider why SG was disabled on your device,
this looks very pessimistic.

Thanks !

 include/net/sock.h  |    1 +
 net/core/sock.c     |    2 +-
 net/ipv4/tcp_ipv4.c |    1 +
 net/ipv6/tcp_ipv6.c |    1 +
 4 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 169c92afcafa3d548f8238e91606b87c187559f4..df4ac691870ff9f779f1782ded58140eb4d3a961 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -417,6 +417,7 @@ struct sock {
 	struct page_frag	sk_frag;
 	netdev_features_t	sk_route_caps;
 	netdev_features_t	sk_route_nocaps;
+	netdev_features_t	sk_route_forced_caps;
 	int			sk_gso_type;
 	unsigned int		sk_gso_max_size;
 	gfp_t			sk_allocation;
diff --git a/net/core/sock.c b/net/core/sock.c
index c501499a04fe973e80e18655b306d762d348ff44..b084acb3b3b96791663b731788a392041148416c 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1773,7 +1773,7 @@ void sk_setup_caps(struct sock *sk, struct dst_entry *dst)
 	u32 max_segs = 1;
 
 	sk_dst_set(sk, dst);
-	sk->sk_route_caps = dst->dev->features;
+	sk->sk_route_caps = dst->dev->features | sk->sk_route_forced_caps;
 	if (sk->sk_route_caps & NETIF_F_GSO)
 		sk->sk_route_caps |= NETIF_F_GSO_SOFTWARE;
 	sk->sk_route_caps &= ~sk->sk_route_nocaps;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index f8ad397e285e9b8db0b04f8abc30a42f22294ef9..eaf1e30fc5af879442f5f33ed4bd69f89dff8cfb 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -233,6 +233,7 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 	}
 	/* OK, now commit destination to socket.  */
 	sk->sk_gso_type = SKB_GSO_TCPV4;
+	sk->sk_route_forced_caps = NETIF_F_GSO;
 	sk_setup_caps(sk, &rt->dst);
 	rt = NULL;
 
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 412139f4eccd96923daaea064cd9fb8be13f5916..4a461e8e2d654aa341d525a0df609a294c2040df 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -269,6 +269,7 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
 	inet->inet_rcv_saddr = LOOPBACK4_IPV6;
 
 	sk->sk_gso_type = SKB_GSO_TCPV6;
+	sk->sk_route_forced_caps = NETIF_F_GSO;
 	ip6_dst_store(sk, dst, NULL, NULL);
 
 	icsk->icsk_ext_hdr_len = 0;