> Should we update any documentation or NEWS?

The documentation I've looked at doesn't mention a default value so I don't
think it needs to change.  I can't imagine that any users will really care that
we don't send echo's on unix sockets so I'm reluctant to add it to the NEWS.

The following is an incremental incorporating your comments.

---
 lib/jsonrpc.c          |   11 ++++-------
 lib/stream.c           |   37 ++++++++++++++++---------------------
 lib/stream.h           |    3 +--
 ovsdb/jsonrpc-server.c |   15 +++------------
 python/ovs/jsonrpc.py  |    8 +++-----
 python/ovs/stream.py   |   31 +++++++++++++------------------
 6 files changed, 40 insertions(+), 65 deletions(-)

diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
index 0fc0995..e95db8a 100644
--- a/lib/jsonrpc.c
+++ b/lib/jsonrpc.c
@@ -762,13 +762,10 @@ jsonrpc_session_open(const char *name)
 
     if (!pstream_verify_name(name)) {
         reconnect_set_passive(s->reconnect, true, time_msec());
-        if (!pstream_needs_probes(name)) {
-            reconnect_set_probe_interval(s->reconnect, 0);
-        }
-    } else if (!stream_verify_name(name)) {
-        if (!stream_needs_probes(name)) {
-            reconnect_set_probe_interval(s->reconnect, 0);
-        }
+    }
+
+    if (!stream_or_pstream_needs_probes(name)) {
+        reconnect_set_probe_interval(s->reconnect, 0);
     }
 
     return s;
diff --git a/lib/stream.c b/lib/stream.c
index 8a546b4..eabace8 100644
--- a/lib/stream.c
+++ b/lib/stream.c
@@ -186,18 +186,6 @@ stream_verify_name(const char *name)
     return stream_lookup_class(name, &class);
 }
 
-/* True if the stream specified by 'name' needs periodic probes to verify
- * connectivty.  For streams which need probes, it can take a long time to
- * notice the connection was dropped. 'name' must be valid according to
- * stream_verify_name(). */
-bool
-stream_needs_probes(const char *name)
-{
-    const struct stream_class *class;
-    stream_lookup_class(name, &class);
-    return class->needs_probes;
-}
-
 /* Attempts to connect a stream to a remote peer.  'name' is a connection name
  * in the form "TYPE:ARGS", where TYPE is an active stream class's name and
  * ARGS are stream class-specific.
@@ -501,16 +489,23 @@ pstream_verify_name(const char *name)
     return pstream_lookup_class(name, &class);
 }
 
-/* True if the pstream specified by 'name' needs periodic probes to verify
- * connectivty.  For pstreams which need probes, it can take a long time to
- * notice the connection was dropped. 'name' must be valid according to
- * pstream_verify_name(). */
-bool
-pstream_needs_probes(const char *name)
+/* Returns 1 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
+stream_or_pstream_needs_probes(const char *name)
 {
-    const struct pstream_class *class;
-    pstream_lookup_class(name, &class);
-    return class->needs_probes;
+    const struct pstream_class *pclass;
+    const struct stream_class *class;
+
+    if (!stream_lookup_class(name, &class)) {
+        return class->needs_probes;
+    } else if (!pstream_lookup_class(name, &pclass)) {
+        return pclass->needs_probes;
+    } else {
+        return -1;
+    }
 }
 
 /* Attempts to start listening for remote stream connections.  'name' is a
diff --git a/lib/stream.h b/lib/stream.h
index fb60b22..bd39017 100644
--- a/lib/stream.h
+++ b/lib/stream.h
@@ -32,7 +32,6 @@ void stream_usage(const char *name, bool active, bool 
passive, bool bootstrap);
 
 /* Bidirectional byte streams. */
 int stream_verify_name(const char *name);
-bool stream_needs_probes(const char *name);
 int stream_open(const char *name, struct stream **, uint8_t dscp);
 int stream_open_block(int error, struct stream **);
 void stream_close(struct stream *);
@@ -60,7 +59,6 @@ void stream_send_wait(struct stream *);
 
 /* Passive streams: listeners for incoming stream connections. */
 int pstream_verify_name(const char *name);
-bool pstream_needs_probes(const char *name);
 int pstream_open(const char *name, struct pstream **, uint8_t dscp);
 const char *pstream_get_name(const struct pstream *);
 void pstream_close(struct pstream *);
@@ -84,6 +82,7 @@ bool stream_parse_target_with_default_ports(const char 
*target,
                                            uint16_t default_tcp_port,
                                            uint16_t default_ssl_port,
                                            struct sockaddr_in *sin);
+int stream_or_pstream_needs_probes(const char *name);
 
 /* Error reporting. */
 
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 636efb3..88656b9 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -130,19 +130,10 @@ struct ovsdb_jsonrpc_options *
 ovsdb_jsonrpc_default_options(const char *target)
 {
     struct ovsdb_jsonrpc_options *options = xzalloc(sizeof *options);
-    options->probe_interval = RECONNECT_DEFAULT_PROBE_INTERVAL;
     options->max_backoff = RECONNECT_DEFAULT_MAX_BACKOFF;
-
-    if (!pstream_verify_name(target)) {
-        if (!pstream_needs_probes(target)) {
-            options->probe_interval = 0;
-        }
-    } else if (!stream_verify_name(target)) {
-        if (!stream_needs_probes(target)) {
-            options->probe_interval = 0;
-        }
-    }
-
+    options->probe_interval = (stream_or_pstream_needs_probes(target)
+                               ? RECONNECT_DEFAULT_PROBE_INTERVAL
+                               : 0);
     return options;
 }
 
diff --git a/python/ovs/jsonrpc.py b/python/ovs/jsonrpc.py
index 39ba628..b72af77 100644
--- a/python/ovs/jsonrpc.py
+++ b/python/ovs/jsonrpc.py
@@ -371,11 +371,9 @@ class Session(object):
 
         if ovs.stream.PassiveStream.is_valid_name(name):
             reconnect.set_passive(True, ovs.timeval.msec())
-            if not ovs.stream.PassiveStream.needs_probes(name):
-                reconnect.set_probe_interval(0)
-        elif ovs.stream.Stream.is_valid_name(name):
-            if not ovs.stream.Stream.needs_probes(name):
-                reconnect.set_probe_interval(0)
+
+        if ovs.stream.stream_or_pstream_needs_probes(name):
+            reconnect.set_probe_interval(0)
 
         return Session(reconnect, None)
 
diff --git a/python/ovs/stream.py b/python/ovs/stream.py
index bc5c83e..e2adaa5 100644
--- a/python/ovs/stream.py
+++ b/python/ovs/stream.py
@@ -24,6 +24,19 @@ import ovs.vlog
 vlog = ovs.vlog.Vlog("stream")
 
 
+def stream_or_pstream_needs_probes(name):
+    """ 1 if the stream or pstream specified by 'name' needs periodic probes to
+    verify connectivty.  For [p]streams which need probes, it can take a long
+    time to notice the connection was dropped.  Returns 0 if probes aren't
+    needed, and -1 if 'name' is invalid"""
+
+    if PassiveStream.is_valid_name(name) or Stream.is_valid_name(name):
+        # Only unix and punix are supported currently.
+        return 0
+    else:
+        return -1
+
+
 class Stream(object):
     """Bidirectional byte stream.  Currently only Unix domain sockets
     are implemented."""
@@ -45,15 +58,6 @@ class Stream(object):
         False."""
         return name.startswith("unix:")
 
-    @staticmethod
-    def needs_probes(name):
-        """ True if the stream specified by 'name' needs periodic probes to
-        verify connectivty.  For streams which need probes, it can take a long
-        time to notice the connection was dropped. 'name' must be valid
-        according to Stream.is_valid_name()."""
-        assert name.startswith("unix:")
-        return False
-
     def __init__(self, socket, name, status):
         self.socket = socket
         self.name = name
@@ -236,15 +240,6 @@ class PassiveStream(object):
         "punix:"), otherwise False."""
         return name.startswith("punix:")
 
-    @staticmethod
-    def needs_probes(name):
-        """True if the pstream specified by 'name' needs periodic probes to
-        verify connectivty.  For pstreams which need probes, it can take a long
-        time to notice the connection was dropped. 'name' must be valid
-        according to PassiveStream.pstream_is_valid_name()."""
-        assert name.statswith("punix:")
-        return False
-
     def __init__(self, sock, name, bind_path):
         self.name = name
         self.socket = sock
-- 
1.7.9.6

_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to