From patchwork Mon Jul 6 08:20:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Ivanov X-Patchwork-Id: 1323421 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=fraxinus.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=cambridgegreys.com Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4B0dn03klZz9sQt for ; Mon, 6 Jul 2020 18:20:52 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 13FE986CFC; Mon, 6 Jul 2020 08:20:51 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id n0-74JZiqnQF; Mon, 6 Jul 2020 08:20:46 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 0A22D86CF5; Mon, 6 Jul 2020 08:20:43 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D083DC08A9; Mon, 6 Jul 2020 08:20:42 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 09D5CC016F for ; Mon, 6 Jul 2020 08:20:41 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 051CD87FDD for ; Mon, 6 Jul 2020 08:20:41 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id DB9+WQUbq-p6 for ; Mon, 6 Jul 2020 08:20:39 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.7.6 Received: from www.kot-begemot.co.uk (ivanoab7.miniserver.com [37.128.132.42]) by whitealder.osuosl.org (Postfix) with ESMTPS id 04F4587F59 for ; Mon, 6 Jul 2020 08:20:38 +0000 (UTC) Received: from tun252.jain.kot-begemot.co.uk ([192.168.18.6] helo=jain.kot-begemot.co.uk) by www.kot-begemot.co.uk with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jsMMe-00068p-NY for dev@openvswitch.org; Mon, 06 Jul 2020 08:20:37 +0000 Received: from jain.kot-begemot.co.uk ([192.168.3.3]) by jain.kot-begemot.co.uk with esmtp (Exim 4.92) (envelope-from ) id 1jsMMb-0007Tb-Kf; Mon, 06 Jul 2020 09:20:35 +0100 From: anton.ivanov@cambridgegreys.com To: dev@openvswitch.org Date: Mon, 6 Jul 2020 09:20:10 +0100 Message-Id: <20200706082013.27446-3-anton.ivanov@cambridgegreys.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200706082013.27446-1-anton.ivanov@cambridgegreys.com> References: <20200706082013.27446-1-anton.ivanov@cambridgegreys.com> MIME-Version: 1.0 X-Clacks-Overhead: GNU Terry Pratchett Cc: Anton Ivanov Subject: [ovs-dev] [PATCH 2/6] Enable kernel probes and map stream probes onto them X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Anton Ivanov 1. Fix probe logic. The stream_or_pstream_needs_probes function returning a mix of integer and boolean. As a result probes were NOT turned off in a number of cases on unix domain sockets and other transports where there should be no probing. It now returns -1 (do not know), 0 (no probe needed), 1 (definitely needs probes). 2. Allow delegating probing to keepalive facilities in the stream layer if avaialable. 3. Provide TCP KEEPALIVE probing at stream layer on supported platforms for stream-ssl and stream-tcp. Signed-off-by: Anton Ivanov --- lib/jsonrpc.c | 36 +++++++++++++++++++++++++++++++- lib/socket-util.c | 48 +++++++++++++++++++++++++++++++++++++++++++ lib/socket-util.h | 8 ++++++++ lib/stream-fd.c | 9 ++++++++ lib/stream-provider.h | 6 ++++++ lib/stream-ssl.c | 8 ++++++++ lib/stream-tcp.c | 1 + lib/stream-unix.c | 6 ++++++ lib/stream-windows.c | 1 + lib/stream.c | 26 +++++++++++++++++------ lib/stream.h | 3 ++- 11 files changed, 144 insertions(+), 8 deletions(-) diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c index ed748dbde..830b9910f 100644 --- a/lib/jsonrpc.c +++ b/lib/jsonrpc.c @@ -787,6 +787,7 @@ struct jsonrpc_session { int last_error; unsigned int seqno; uint8_t dscp; + int probe_interval; }; static void @@ -839,6 +840,7 @@ jsonrpc_session_open_multiple(const struct svec *remotes, bool retry) s->seqno = 0; s->dscp = 0; s->last_error = 0; + s->probe_interval = reconnect_get_probe_interval(s->reconnect); const char *name = reconnect_get_name(s->reconnect); if (!pstream_verify_name(name)) { @@ -850,6 +852,7 @@ jsonrpc_session_open_multiple(const struct svec *remotes, bool retry) if (!stream_or_pstream_needs_probes(name)) { reconnect_set_probe_interval(s->reconnect, 0); + s->probe_interval = 0; } return s; @@ -879,6 +882,12 @@ jsonrpc_session_open_unreliably(struct jsonrpc *jsonrpc, uint8_t dscp) s->stream = NULL; s->pstream = NULL; s->seqno = 1; + s->probe_interval = reconnect_get_probe_interval(s->reconnect); + + if (!stream_or_pstream_needs_probes(reconnect_get_name(s->reconnect))) { + reconnect_set_probe_interval(s->reconnect, 0); + s->probe_interval = 0; + } return s; } @@ -934,6 +943,12 @@ jsonrpc_session_connect(struct jsonrpc_session *s) error = jsonrpc_stream_open(name, &s->stream, s->dscp); if (!error) { reconnect_connecting(s->reconnect, time_msec()); + if (stream_set_probe_interval(s->stream, s->probe_interval)) { + /* we have delegated probing to the stream layer */ + reconnect_set_probe_interval(s->reconnect, 0); + } else { + reconnect_set_probe_interval(s->reconnect, s->probe_interval); + } } else { s->last_error = error; } @@ -967,6 +982,12 @@ jsonrpc_session_run(struct jsonrpc_session *s) jsonrpc_session_disconnect(s); } reconnect_connected(s->reconnect, time_msec()); + if (stream_set_probe_interval(stream, s->probe_interval)) { + /* we have delegated probing to the stream layer */ + reconnect_set_probe_interval(s->reconnect, 0); + } else { + reconnect_set_probe_interval(s->reconnect, s->probe_interval); + } s->rpc = jsonrpc_open(stream); s->seqno++; } else if (error != EAGAIN) { @@ -1008,6 +1029,12 @@ jsonrpc_session_run(struct jsonrpc_session *s) if (!error) { reconnect_connected(s->reconnect, time_msec()); s->rpc = jsonrpc_open(s->stream); + if (stream_set_probe_interval(s->stream, s->probe_interval)) { + /* we have delegated probing to the stream layer */ + reconnect_set_probe_interval(s->reconnect, 0); + } else { + reconnect_set_probe_interval(s->reconnect, s->probe_interval); + } s->stream = NULL; s->seqno++; } else if (error != EAGAIN) { @@ -1231,7 +1258,14 @@ void jsonrpc_session_set_probe_interval(struct jsonrpc_session *s, int probe_interval) { - reconnect_set_probe_interval(s->reconnect, probe_interval); + s->probe_interval = probe_interval; + if (s->stream) { + if (stream_set_probe_interval(s->stream, probe_interval)) { + reconnect_set_probe_interval(s->reconnect, 0); + } else { + reconnect_set_probe_interval(s->reconnect, probe_interval); + } + } } /* Sets the DSCP value used for 's''s connection to 'dscp'. If this is diff --git a/lib/socket-util.c b/lib/socket-util.c index 4f1ffecf5..3aad89d23 100644 --- a/lib/socket-util.c +++ b/lib/socket-util.c @@ -114,6 +114,54 @@ setsockopt_tcp_nodelay(int fd) } } +#ifdef HAS_KERNEL_KEEPALIVES +bool tcp_set_probe_interval(int fd, int probe_interval) { + int on = 1; + int retval; + int value; + + on = 1; + retval = setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &on, sizeof on); + if (retval) { + retval = sock_errno(); + VLOG_DBG("setsockopt(SO_KEEPALIVE): %s", sock_strerror(retval)); + return false; + } + value = 2; + retval = setsockopt(fd, + IPPROTO_TCP, TCP_KEEPCNT, &value, sizeof value); + if (retval) { + retval = sock_errno(); + VLOG_DBG("setsockopt(TCP_KEEPCNT): %s", sock_strerror(retval)); + goto params_failed; + } + value = probe_interval; + retval = setsockopt(fd, + IPPROTO_TCP, TCP_KEEPIDLE, &value, sizeof value); + if (retval) { + retval = sock_errno(); + VLOG_DBG("setsockopt(TCP_KEEPIDLE): %s", sock_strerror(retval)); + goto params_failed; + } + value = probe_interval; + retval = setsockopt(fd, + IPPROTO_TCP, TCP_KEEPINTVL, &value, sizeof value); + if (retval) { + retval = sock_errno(); + VLOG_DBG("setsockopt(SO_KEEPALIVE): %s", sock_strerror(retval)); + goto params_failed; + } + return true; +params_failed: + on = 0; + retval = setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &on, sizeof on); + return false; +#else +bool tcp_set_probe_interval(int fd OVS_UNUSED, int probe_interval OVS_UNUSED) { + return false; +#endif +} + /* Sets the DSCP value of socket 'fd' to 'dscp', which must be 63 or less. * 'family' must indicate the socket's address family (AF_INET or AF_INET6, to * do anything useful). */ diff --git a/lib/socket-util.h b/lib/socket-util.h index 9ccb7d4cc..2439e2f78 100644 --- a/lib/socket-util.h +++ b/lib/socket-util.h @@ -27,6 +27,7 @@ #include "openvswitch/types.h" #include #include +#include struct ds; @@ -181,4 +182,11 @@ static inline int sock_errno(void) #endif } +#if defined (SO_KEEPALIVE) && defined (TCP_KEEPCNT) && \ + defined (TCP_KEEPIDLE) && defined (TCP_KEEPINTVL) +#define HAS_KERNEL_KEEPALIVES 1 +#endif + +bool tcp_set_probe_interval(int fd, int probe_interval); + #endif /* socket-util.h */ diff --git a/lib/stream-fd.c b/lib/stream-fd.c index 46ee7ae27..30622929b 100644 --- a/lib/stream-fd.c +++ b/lib/stream-fd.c @@ -162,6 +162,14 @@ fd_wait(struct stream *stream, enum stream_wait_type wait) } } +static bool fd_set_probe_interval(struct stream *stream, int probe_interval) { + struct stream_fd *sf = stream_fd_cast(stream); + + return tcp_set_probe_interval(sf->fd, probe_interval); +} + + + static const struct stream_class stream_fd_class = { "fd", /* name */ false, /* needs_probes */ @@ -173,6 +181,7 @@ static const struct stream_class stream_fd_class = { NULL, /* run */ NULL, /* run_wait */ fd_wait, /* wait */ + fd_set_probe_interval, /* set_probe_interval */ }; /* Passive file descriptor stream. */ diff --git a/lib/stream-provider.h b/lib/stream-provider.h index 75f4f059b..6c28cb50b 100644 --- a/lib/stream-provider.h +++ b/lib/stream-provider.h @@ -124,6 +124,12 @@ struct stream_class { /* Arranges for the poll loop to wake up when 'stream' is ready to take an * action of the given 'type'. */ void (*wait)(struct stream *stream, enum stream_wait_type type); + /* Sets low level keepalives if supported + * + * If successful returns true + * + */ + bool (*set_probe_interval)(struct stream *stream, int probe_interval); }; /* Passive listener for incoming stream connections. diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c index 078fcbc3a..575c55f5b 100644 --- a/lib/stream-ssl.c +++ b/lib/stream-ssl.c @@ -789,6 +789,13 @@ ssl_run(struct stream *stream) } } +static bool ssl_set_probe_interval(struct stream *stream, int probe_interval) { + struct ssl_stream *sslv = ssl_stream_cast(stream); + + return tcp_set_probe_interval(sslv->fd, probe_interval); +} + + static void ssl_run_wait(struct stream *stream) { @@ -861,6 +868,7 @@ const struct stream_class ssl_stream_class = { ssl_run, /* run */ ssl_run_wait, /* run_wait */ ssl_wait, /* wait */ + ssl_set_probe_interval, /* set_probe_interval */ }; /* Passive SSL. */ diff --git a/lib/stream-tcp.c b/lib/stream-tcp.c index e8dc2bfaa..67c912105 100644 --- a/lib/stream-tcp.c +++ b/lib/stream-tcp.c @@ -73,6 +73,7 @@ const struct stream_class tcp_stream_class = { NULL, /* run */ NULL, /* run_wait */ NULL, /* wait */ + NULL, }; /* Passive TCP. */ diff --git a/lib/stream-unix.c b/lib/stream-unix.c index d265efb83..4e96720ab 100644 --- a/lib/stream-unix.c +++ b/lib/stream-unix.c @@ -62,6 +62,11 @@ unix_open(const char *name, char *suffix, struct stream **streamp, AF_UNIX, streamp); } +static bool unix_set_probe_interval(struct stream *stream OVS_UNUSED, int probe_interval OVS_UNUSED) { + + return true; +} + const struct stream_class unix_stream_class = { "unix", /* name */ false, /* needs_probes */ @@ -73,6 +78,7 @@ const struct stream_class unix_stream_class = { NULL, /* run */ NULL, /* run_wait */ NULL, /* wait */ + unix_set_probe_interval, }; /* Passive UNIX socket. */ diff --git a/lib/stream-windows.c b/lib/stream-windows.c index 5c4c55e5d..836112f75 100644 --- a/lib/stream-windows.c +++ b/lib/stream-windows.c @@ -374,6 +374,7 @@ const struct stream_class windows_stream_class = { NULL, /* run */ NULL, /* run_wait */ windows_wait, /* wait */ + NULL, }; struct pwindows_pstream diff --git a/lib/stream.c b/lib/stream.c index e246b3773..9902c9ba4 100644 --- a/lib/stream.c +++ b/lib/stream.c @@ -430,6 +430,19 @@ stream_wait(struct stream *stream, enum stream_wait_type wait) (stream->class->wait)(stream, wait); } + +bool stream_set_probe_interval(struct stream *stream, int probe_interval) { + if (! stream->class->needs_probes) { + return true; + } + if (probe_interval && stream->class->set_probe_interval) { + return (stream->class->set_probe_interval)( + stream, probe_interval / 1000); + } + return false; +} + + void stream_connect_wait(struct stream *stream) { @@ -498,11 +511,13 @@ pstream_verify_name(const char *name) return pstream_lookup_class(name, &class); } -/* Returns 1 if the stream or pstream specified by 'name' needs periodic probes +/* Returns true if the stream or pstream specified by 'name' needs periodic probes * to verify connectivity. For [p]streams which need probes, it can take a - * long time to notice the connection has been dropped. Returns 0 if the - * stream or pstream does not need probes, and -1 if 'name' is not valid. */ -int + * long time to notice the connection has been dropped. Returns false if the + * stream or pstream does not need probes as well as when the name cannot + * be matched. */ + +bool stream_or_pstream_needs_probes(const char *name) { const struct pstream_class *pclass; @@ -512,9 +527,8 @@ stream_or_pstream_needs_probes(const char *name) return class->needs_probes; } else if (!pstream_lookup_class(name, &pclass)) { return pclass->needs_probes; - } else { - return -1; } + return false; } /* Attempts to start listening for remote stream connections. 'name' is a diff --git a/lib/stream.h b/lib/stream.h index 77bffa498..6765c7d15 100644 --- a/lib/stream.h +++ b/lib/stream.h @@ -40,6 +40,7 @@ const char *stream_get_name(const struct stream *); int stream_connect(struct stream *); int stream_recv(struct stream *, void *buffer, size_t n); int stream_send(struct stream *, const void *buffer, size_t n); +bool stream_set_probe_interval(struct stream *, int probe_interval); void stream_run(struct stream *); void stream_run_wait(struct stream *); @@ -80,7 +81,7 @@ int pstream_open_with_default_port(const char *name, bool stream_parse_target_with_default_port(const char *target, int default_port, struct sockaddr_storage *ss); -int stream_or_pstream_needs_probes(const char *name); +bool stream_or_pstream_needs_probes(const char *name); /* Error reporting. */