From: Rao Shoaib <rao.shoaib@oracle.com>
To: davem@davemloft.net, kuznet@ms2.inr.ac.ru
Cc: netdev@vger.kernel.org
Subject: [PATCH v1 net] TCP_USER_TIMEOUT and tcp_keepalive should conform to RFC5482
Date: Mon, 7 Aug 2017 11:16:14 -0700 [thread overview]
Message-ID: <20170807181614.GA16700@caduceus5> (raw)
Change from version 0: Rationale behind the change:
The man page for tcp(7) states
when used with the TCP keepalive (SO_KEEPALIVE) option, TCP_USER_TIMEOUT will
override keepalive to determine when to close a connection due to keepalive
failure.
This is ambigious at best. user expectation is most likely that the connection
will be reset after TCP_USER_TIMEOUT milliseconds of inactivity.
The code however waits for the keepalive to kick-in (default 2hrs) and than
after one failure resets the conenction.
What is the rationale for that ? The same effect can be obtained by simply
changing the value of tcp_keep_alive_probes.
Since the TCP_USER_TIMEOUT option was added based on RFC 5482 we need to follow
the RFC. Which states
4.2 TCP keep-Alives:
Some TCP implementations, such as those in BSD systems, use a
different abort policy for TCP keep-alives than for user data. Thus,
the TCP keep-alive mechanism might abort a connection that would
otherwise have survived the transient period without connectivity.
Therefore, if a connection that enables keep-alives is also using the
TCP User Timeout Option, then the keep-alive timer MUST be set to a
value larger than that of the adopted USER TIMEOUT.
This patch enforces the MUST and also dis-associates user timeout from keep
alive. A man page patch will be submitted separately.
Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
---
net/ipv4/tcp.c | 10 ++++++++--
net/ipv4/tcp_timer.c | 9 +--------
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 71ce33d..f2af44d 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2628,7 +2628,9 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
break;
case TCP_KEEPIDLE:
- if (val < 1 || val > MAX_TCP_KEEPIDLE)
+ /* Per RFC5482 keepalive_time must be > user_timeout */
+ if (val < 1 || val > MAX_TCP_KEEPIDLE ||
+ ((val * HZ) <= icsk->icsk_user_timeout))
err = -EINVAL;
else {
tp->keepalive_time = val * HZ;
@@ -2724,8 +2726,12 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
case TCP_USER_TIMEOUT:
/* Cap the max time in ms TCP will retry or probe the window
* before giving up and aborting (ETIMEDOUT) a connection.
+ * Per RFC5482 TCP user timeout must be < keepalive_time.
+ * If the default value changes later -- all bets are off.
*/
- if (val < 0)
+ if (val < 0 || (tp->keepalive_time &&
+ tp->keepalive_time <= msecs_to_jiffies(val)) ||
+ net->ipv4.sysctl_tcp_keepalive_time <= msecs_to_jiffies(val))
err = -EINVAL;
else
icsk->icsk_user_timeout = msecs_to_jiffies(val);
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index c0feeee..d39fe60 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -664,14 +664,7 @@ static void tcp_keepalive_timer (unsigned long data)
elapsed = keepalive_time_elapsed(tp);
if (elapsed >= keepalive_time_when(tp)) {
- /* If the TCP_USER_TIMEOUT option is enabled, use that
- * to determine when to timeout instead.
- */
- if ((icsk->icsk_user_timeout != 0 &&
- elapsed >= icsk->icsk_user_timeout &&
- icsk->icsk_probes_out > 0) ||
- (icsk->icsk_user_timeout == 0 &&
- icsk->icsk_probes_out >= keepalive_probes(tp))) {
+ if (icsk->icsk_probes_out >= keepalive_probes(tp)) {
tcp_send_active_reset(sk, GFP_ATOMIC);
tcp_write_err(sk);
goto out;
--
2.7.4
next reply other threads:[~2017-08-07 18:16 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-07 18:16 Rao Shoaib [this message]
2017-08-08 9:59 ` [PATCH v1 net] TCP_USER_TIMEOUT and tcp_keepalive should conform to RFC5482 Eric Dumazet
2017-08-08 17:25 ` Yuchung Cheng
2017-08-09 23:52 ` Jerry Chu
2017-08-10 0:20 ` Joe Smith
2017-08-10 0:30 ` David Miller
2017-08-10 0:47 ` Rao Shoaib
2017-08-10 0:52 ` David Miller
2017-08-10 3:32 ` Jerry Chu
2017-08-10 4:59 ` Jerry Chu
2017-08-10 21:05 ` Rao Shoaib
2017-08-11 2:32 ` Jerry Chu
2017-08-10 0:31 ` Rao Shoaib
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170807181614.GA16700@caduceus5 \
--to=rao.shoaib@oracle.com \
--cc=davem@davemloft.net \
--cc=kuznet@ms2.inr.ac.ru \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).