* [PATCH net-next] tcp:elapsed variable calculated twice while keepalive working
@ 2013-08-06 12:38 Tingwei Liu
2013-08-09 18:12 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Tingwei Liu @ 2013-08-06 12:38 UTC (permalink / raw)
To: netdev, davem, kuznet, eric.dumazet; +Cc: Tingwei Liu
When tcp keepalive working elapsed calculated twice while the first time is not needed!
CC: Eric Dumazet <eric.dumazet@gmail.com>
CC: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Signed-off-by: Tingwei Liu <tingw.liu@gmail.com>
---
net/ipv4/tcp_timer.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 4b85e6f..03091d9 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -591,11 +591,11 @@ static void tcp_keepalive_timer (unsigned long data)
if (!sock_flag(sk, SOCK_KEEPOPEN) || sk->sk_state == TCP_CLOSE)
goto out;
- elapsed = keepalive_time_when(tp);
-
/* It is alive without keepalive 8) */
- if (tp->packets_out || tcp_send_head(sk))
+ if (tp->packets_out || tcp_send_head(sk)) {
+ elapsed = keepalive_time_when(tp);
goto resched;
+ }
elapsed = keepalive_time_elapsed(tp);
--
1.6.0.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tcp:elapsed variable calculated twice while keepalive working
2013-08-06 12:38 [PATCH net-next] tcp:elapsed variable calculated twice while keepalive working Tingwei Liu
@ 2013-08-09 18:12 ` David Miller
2013-08-09 21:06 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2013-08-09 18:12 UTC (permalink / raw)
To: tingw.liu; +Cc: netdev, kuznet, eric.dumazet
From: Tingwei Liu <tingw.liu@gmail.com>
Date: Tue, 6 Aug 2013 20:38:58 +0800
> When tcp keepalive working elapsed calculated twice while the first time is not needed!
>
> CC: Eric Dumazet <eric.dumazet@gmail.com>
> CC: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> Signed-off-by: Tingwei Liu <tingw.liu@gmail.com>
Please put a space after "tcp:" in your Subject line prefixes, it looks
awful the way you've done it here.
> @@ -591,11 +591,11 @@ static void tcp_keepalive_timer (unsigned long data)
> if (!sock_flag(sk, SOCK_KEEPOPEN) || sk->sk_state == TCP_CLOSE)
> goto out;
>
> - elapsed = keepalive_time_when(tp);
> -
> /* It is alive without keepalive 8) */
> - if (tp->packets_out || tcp_send_head(sk))
> + if (tp->packets_out || tcp_send_head(sk)) {
> + elapsed = keepalive_time_when(tp);
> goto resched;
> + }
>
> elapsed = keepalive_time_elapsed(tp);
This is overkill, just delete the second assignment. Your version makes
the code look less elegant and also makes it harder to audit.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tcp:elapsed variable calculated twice while keepalive working
2013-08-09 18:12 ` David Miller
@ 2013-08-09 21:06 ` Eric Dumazet
0 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2013-08-09 21:06 UTC (permalink / raw)
To: David Miller; +Cc: tingw.liu, netdev, kuznet
On Fri, 2013-08-09 at 11:12 -0700, David Miller wrote:
> From: Tingwei Liu <tingw.liu@gmail.com>
> Date: Tue, 6 Aug 2013 20:38:58 +0800
>
> > When tcp keepalive working elapsed calculated twice while the first time is not needed!
> >
> > CC: Eric Dumazet <eric.dumazet@gmail.com>
> > CC: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> > Signed-off-by: Tingwei Liu <tingw.liu@gmail.com>
>
>
> Please put a space after "tcp:" in your Subject line prefixes, it looks
> awful the way you've done it here.
>
> > @@ -591,11 +591,11 @@ static void tcp_keepalive_timer (unsigned long data)
> > if (!sock_flag(sk, SOCK_KEEPOPEN) || sk->sk_state == TCP_CLOSE)
> > goto out;
> >
> > - elapsed = keepalive_time_when(tp);
> > -
> > /* It is alive without keepalive 8) */
> > - if (tp->packets_out || tcp_send_head(sk))
> > + if (tp->packets_out || tcp_send_head(sk)) {
> > + elapsed = keepalive_time_when(tp);
> > goto resched;
> > + }
> >
> > elapsed = keepalive_time_elapsed(tp);
>
> This is overkill, just delete the second assignment. Your version makes
> the code look less elegant and also makes it harder to audit.
> --
As a matter of fact,
keepalive_time_when(tp) and keepalive_time_elapsed(tp) are different ;)
This is very slow path in TCP stack, so it should not matter a lot.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tcp:elapsed variable calculated twice while keepalive working
2013-08-06 12:23 ` Eric Dumazet
@ 2013-08-06 12:29 ` tingwei liu
0 siblings, 0 replies; 6+ messages in thread
From: tingwei liu @ 2013-08-06 12:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Alexey Kuznetsov, davem
On Tue, Aug 6, 2013 at 8:23 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2013-08-06 at 20:05 +0800, Tingwei Liu wrote:
>> ---
>> net/ipv4/tcp_timer.c | 6 +++---
>> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> I suggest reading Documentation/SubmittingPatches, because you forgot
> the "Signed-off-by:" tag, and you did the same on your last patch
> submission.
Thanks very much! I will resend the patch in a few minutes.
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tcp:elapsed variable calculated twice while keepalive working
2013-08-06 12:05 Tingwei Liu
@ 2013-08-06 12:23 ` Eric Dumazet
2013-08-06 12:29 ` tingwei liu
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2013-08-06 12:23 UTC (permalink / raw)
To: Tingwei Liu; +Cc: netdev, kuznet, davem
On Tue, 2013-08-06 at 20:05 +0800, Tingwei Liu wrote:
> ---
> net/ipv4/tcp_timer.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
I suggest reading Documentation/SubmittingPatches, because you forgot
the "Signed-off-by:" tag, and you did the same on your last patch
submission.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next] tcp:elapsed variable calculated twice while keepalive working
@ 2013-08-06 12:05 Tingwei Liu
2013-08-06 12:23 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Tingwei Liu @ 2013-08-06 12:05 UTC (permalink / raw)
To: netdev, kuznet, davem; +Cc: Tingwei Liu
---
net/ipv4/tcp_timer.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 4b85e6f..03091d9 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -591,11 +591,11 @@ static void tcp_keepalive_timer (unsigned long data)
if (!sock_flag(sk, SOCK_KEEPOPEN) || sk->sk_state == TCP_CLOSE)
goto out;
- elapsed = keepalive_time_when(tp);
-
/* It is alive without keepalive 8) */
- if (tp->packets_out || tcp_send_head(sk))
+ if (tp->packets_out || tcp_send_head(sk)) {
+ elapsed = keepalive_time_when(tp);
goto resched;
+ }
elapsed = keepalive_time_elapsed(tp);
--
1.6.0.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-08-09 21:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-06 12:38 [PATCH net-next] tcp:elapsed variable calculated twice while keepalive working Tingwei Liu
2013-08-09 18:12 ` David Miller
2013-08-09 21:06 ` Eric Dumazet
-- strict thread matches above, loose matches on Subject: below --
2013-08-06 12:05 Tingwei Liu
2013-08-06 12:23 ` Eric Dumazet
2013-08-06 12:29 ` tingwei liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.