[TCP] tcp_write_timeout.c cleanup

[TCP] tcp_write_timeout.c cleanup

[Eric Dumazet]

[-- Attachment #1: Type: text/plain, Size: 248 bytes --]

Before submiting a patch to change a divide to a right shift, I felt
necessary to create a helper function tcp_mtu_probing() to reduce length of 
lines exceeding 100 chars in tcp_write_timeout().

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>


[-- Attachment #2: tcp_timer_cleanup.patch --]
[-- Type: text/plain, Size: 1838 bytes --]

diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index d8970ec..8f14808 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -114,13 +114,31 @@ static int tcp_orphan_retries(struct sock *sk, int alive)
 	return retries;
 }
 
+static void tcp_mtu_probing(struct inet_connection_sock *icsk, struct sock *sk)
+{
+	int mss;
+
+	/* Black hole detection */
+	if (sysctl_tcp_mtu_probing) {
+		if (!icsk->icsk_mtup.enabled) {
+			icsk->icsk_mtup.enabled = 1;
+			tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
+		} else {
+			struct tcp_sock *tp = tcp_sk(sk);
+			mss = tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_low)/2;
+			mss = min(sysctl_tcp_base_mss, mss);
+			mss = max(mss, 68 - tp->tcp_header_len);
+			icsk->icsk_mtup.search_low = tcp_mss_to_mtu(sk, mss);
+			tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
+		}
+	}
+}
+
 /* A write timeout has occurred. Process the after effects. */
 static int tcp_write_timeout(struct sock *sk)
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
-	struct tcp_sock *tp = tcp_sk(sk);
 	int retry_until;
-	int mss;
 
 	if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) {
 		if (icsk->icsk_retransmits)
@@ -129,18 +147,7 @@ static int tcp_write_timeout(struct sock *sk)
 	} else {
 		if (icsk->icsk_retransmits >= sysctl_tcp_retries1) {
 			/* Black hole detection */
-			if (sysctl_tcp_mtu_probing) {
-				if (!icsk->icsk_mtup.enabled) {
-					icsk->icsk_mtup.enabled = 1;
-					tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
-				} else {
-					mss = min(sysctl_tcp_base_mss,
-						  tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_low)/2);
-					mss = max(mss, 68 - tp->tcp_header_len);
-					icsk->icsk_mtup.search_low = tcp_mss_to_mtu(sk, mss);
-					tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
-				}
-			}
+			tcp_mtu_probing(icsk, sk);
 
 			dst_negative_advice(&sk->sk_dst_cache);
 		}

Re: [TCP] tcp_write_timeout.c cleanup

[David Miller]

From: Eric Dumazet <dada1@cosmosbay.com>
Date: Fri, 21 Dec 2007 06:56:17 +0100

> Before submiting a patch to change a divide to a right shift, I felt
> necessary to create a helper function tcp_mtu_probing() to reduce length of 
> lines exceeding 100 chars in tcp_write_timeout().
> 
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

Thanks for not mixing code cleanups with functional changes,
it makes my life so much easier and make me so much want to
review and apply your patches.

Re: [TCP] tcp_write_timeout.c cleanup

[Arnaldo Carvalho de Melo]

Em Fri, Dec 21, 2007 at 06:56:17AM +0100, Eric Dumazet escreveu:
> Before submiting a patch to change a divide to a right shift, I felt
> necessary to create a helper function tcp_mtu_probing() to reduce length of 
> lines exceeding 100 chars in tcp_write_timeout().
>
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
>

> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index d8970ec..8f14808 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -114,13 +114,31 @@ static int tcp_orphan_retries(struct sock *sk, int alive)
>  	return retries;
>  }
>  
> +static void tcp_mtu_probing(struct inet_connection_sock *icsk, struct sock *sk)
> +{
> +	int mss;
-       int mss;
		
> +
> +	/* Black hole detection */
> +	if (sysctl_tcp_mtu_probing) {
> +		if (!icsk->icsk_mtup.enabled) {
> +			icsk->icsk_mtup.enabled = 1;
> +			tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
> +		} else {
> +			struct tcp_sock *tp = tcp_sk(sk);
+			int mss;
> +			mss = tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_low)/2;
> +			mss = min(sysctl_tcp_base_mss, mss);
> +			mss = max(mss, 68 - tp->tcp_header_len);
> +			icsk->icsk_mtup.search_low = tcp_mss_to_mtu(sk, mss);
> +			tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
> +		}
> +	}
> +}

:-)

- Arnaldo

Re: [TCP] tcp_write_timeout.c cleanup

[David Miller]

From: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Fri, 21 Dec 2007 10:23:16 -0200

> Em Fri, Dec 21, 2007 at 06:56:17AM +0100, Eric Dumazet escreveu:
> > +static void tcp_mtu_probing(struct inet_connection_sock *icsk, struct sock *sk)
> > +{
> > +	int mss;
> -       int mss;
> 		
> > +
> > +	/* Black hole detection */
> > +	if (sysctl_tcp_mtu_probing) {
> > +		if (!icsk->icsk_mtup.enabled) {
> > +			icsk->icsk_mtup.enabled = 1;
> > +			tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
> > +		} else {
> > +			struct tcp_sock *tp = tcp_sk(sk);
> +			int mss;
> > +			mss = tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_low)/2;

I've checked in this, ummm... patch :-)

commit 323f3f2f31840f94e540ec5a0ce33593d05dd8d9
Author: David S. Miller <davem@sunset.davemloft.net>
Date:   Fri Dec 21 04:29:16 2007 -0800

    [TCP]: Move mss variable in tcp_mtu_probing()
    
    Down into the only scope where it is used.
    
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 8f14808..ea111e9 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -116,8 +116,6 @@ static int tcp_orphan_retries(struct sock *sk, int alive)
 
 static void tcp_mtu_probing(struct inet_connection_sock *icsk, struct sock *sk)
 {
-	int mss;
-
 	/* Black hole detection */
 	if (sysctl_tcp_mtu_probing) {
 		if (!icsk->icsk_mtup.enabled) {
@@ -125,6 +123,8 @@ static void tcp_mtu_probing(struct inet_connection_sock *icsk, struct sock *sk)
 			tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
 		} else {
 			struct tcp_sock *tp = tcp_sk(sk);
+			int mss;
+
 			mss = tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_low)/2;
 			mss = min(sysctl_tcp_base_mss, mss);
 			mss = max(mss, 68 - tp->tcp_header_len);

Re: [TCP] tcp_write_timeout.c cleanup

[Arnaldo Carvalho de Melo]

Em Fri, Dec 21, 2007 at 04:30:26AM -0800, David Miller escreveu:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date: Fri, 21 Dec 2007 10:23:16 -0200
> 
> > Em Fri, Dec 21, 2007 at 06:56:17AM +0100, Eric Dumazet escreveu:
> > > +static void tcp_mtu_probing(struct inet_connection_sock *icsk, struct sock *sk)
> > > +{
> > > +	int mss;
> > -       int mss;
> > 		
> > > +
> > > +	/* Black hole detection */
> > > +	if (sysctl_tcp_mtu_probing) {
> > > +		if (!icsk->icsk_mtup.enabled) {
> > > +			icsk->icsk_mtup.enabled = 1;
> > > +			tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
> > > +		} else {
> > > +			struct tcp_sock *tp = tcp_sk(sk);
> > +			int mss;
> > > +			mss = tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_low)/2;
> 
> I've checked in this, ummm... patch :-)

Thanks!

- Arnaldo