netdev - [PATCH net-next] tcp: inherit listener congestion control for passive cnx

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: <1322650961.2403.13.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Date:	Wed, 30 Nov 2011 12:02:41 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Rick Jones <raj@...dy.cup.hp.com>,
	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org, Yuchung Cheng <ycheng@...gle.com>,
	Stephen Hemminger <shemminger@...tta.com>
Subject: [PATCH net-next] tcp: inherit listener congestion control for
 passive cnx

Le mercredi 30 novembre 2011 à 06:40 +0100, Eric Dumazet a écrit :
> Le mercredi 30 novembre 2011 à 06:21 +0100, Eric Dumazet a écrit :
> 
> > So my suggestion would be to use this two lines patch instead :
> > 
> > diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> > index 945efff..6b066e2 100644
> > --- a/net/ipv4/tcp_minisocks.c
> > +++ b/net/ipv4/tcp_minisocks.c
> > @@ -495,8 +495,6 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
> >  		newtp->frto_counter = 0;
> >  		newtp->frto_highmark = 0;
> >  
> > -		newicsk->icsk_ca_ops = &tcp_init_congestion_ops;
> > -
> >  		tcp_set_ca_state(newsk, TCP_CA_Open);
> >  		tcp_init_xmit_timers(newsk);
> >  		skb_queue_head_init(&newtp->out_of_order_queue);
> > 
> 
> Please test this change and if its OK, resubmit your patch, with
> appropriate Documentation change, as pointed out by Yuchung Cheng 

Hmm, after taking more time on this, we have to take care of module
refcounting and proper cleanup. I hope you dont mind I submit my final
version of this patch.

Thanks !

[PATCH net-next] tcp: inherit listener congestion control for passive cnx

Rick Jones reported that TCP_CONGESTION sockopt performed on a listener
was ignored for its children sockets : right after accept() the
congestion control for new socket is the system default one.

This seems an oversight of the initial design (quoted from Stephen)

Based on prior investigation and patch from Rick.

Reported-by: Rick Jones <rick.jones2@...com>
Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
CC: Stephen Hemminger <shemminger@...tta.com>
CC: Yuchung Cheng <ycheng@...gle.com>
---
 Documentation/networking/ip-sysctl.txt |    3 +++
 net/ipv4/tcp_ipv4.c                    |    1 +
 net/ipv4/tcp_minisocks.c               |    4 +++-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index b886706..cb2b1c6 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -175,6 +175,9 @@ tcp_congestion_control - STRING
 	connections. The algorithm "reno" is always available, but
 	additional choices may be available based on kernel configuration.
 	Default is set as part of kernel configuration.
+	For passive connections, the listener congestion control choice
+	is inherited.
+	[see setsockopt(listenfd, SOL_TCP, TCP_CONGESTION, "name" ...) ]
 
 tcp_cookie_size - INTEGER
 	Default size of TCP Cookie Transactions (TCPCT) option, that may be
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index a9db4b1..c4b8b09 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1511,6 +1511,7 @@ exit:
 	return NULL;
 put_and_exit:
 	tcp_clear_xmit_timers(newsk);
+	tcp_cleanup_congestion_control(newsk);
 	bh_unlock_sock(newsk);
 	sock_put(newsk);
 	goto exit;
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 945efff..9dc146e 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -495,7 +495,9 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
 		newtp->frto_counter = 0;
 		newtp->frto_highmark = 0;
 
-		newicsk->icsk_ca_ops = &tcp_init_congestion_ops;
+		if (newicsk->icsk_ca_ops != &tcp_init_congestion_ops &&
+		    !try_module_get(newicsk->icsk_ca_ops->owner))
+			newicsk->icsk_ca_ops = &tcp_init_congestion_ops;
 
 		tcp_set_ca_state(newsk, TCP_CA_Open);
 		tcp_init_xmit_timers(newsk);


--
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