Otherwise failed non-blocking connection could be reported as
connected. This causes errors in all following operations with the
socket.
At least this is true on FreeBSD, where POLLHUP could be set without
POLLERR.
For example, stream_open_block() tests fails with the following error
reporting successful connection to the 'WRONG_PORT':
./ovsdb-idl.at:1817:
$PYTHON2 $srcdir/test-stream.py tcp:127.0.0.1:$WRONG_PORT
stdout:
./ovsdb-idl.at:1817: exit code was 0, expected 1
2399. ovsdb-idl.at:1817: FAILED (ovsdb-idl.at:1817)
Also added new tests to track this issue in C library:
'Check Stream open block - C - tcp'
'Check Stream open block - C - tcp6'
CC: Numan Siddique <[email protected]>
Fixes: c1aa16d191d2 ("ovs python: ovs.stream.open_block() returns success even
if the remote is unreachable")
Fixes: d6cedfd9d29d ("socket-util: Avoid using SO_ERROR.")
Signed-off-by: Ilya Maximets <[email protected]>
---
Version 2:
* Fixed C implementation of check_connection_completion().
* Added tests for C implementation.
lib/socket-util.c | 2 +-
python/ovs/socket_util.py | 2 +-
tests/.gitignore | 1 +
tests/automake.mk | 4 ++++
tests/ovsdb-idl.at | 17 +++++++++++++++
tests/test-stream.c | 46 +++++++++++++++++++++++++++++++++++++++
6 files changed, 70 insertions(+), 2 deletions(-)
create mode 100644 tests/test-stream.c
diff --git a/lib/socket-util.c b/lib/socket-util.c
index 8d18e043d..09daa3c90 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -285,7 +285,7 @@ check_connection_completion(int fd)
}
#endif
if (retval == 1) {
- if (pfd.revents & POLLERR) {
+ if (pfd.revents & (POLLERR | POLLHUP)) {
ssize_t n = send(fd, "", 1, 0);
if (n < 0) {
return sock_errno();
diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
index 8e582fe91..2596ddefd 100644
--- a/python/ovs/socket_util.py
+++ b/python/ovs/socket_util.py
@@ -178,7 +178,7 @@ def check_connection_completion(sock):
pfds = p.poll(0)
if len(pfds) == 1:
revents = pfds[0][1]
- if revents & ovs.poller.POLLERR:
+ if revents & ovs.poller.POLLERR or revents & ovs.poller.POLLHUP:
try:
# The following should raise an exception.
sock.send("\0".encode(), socket.MSG_DONTWAIT)
diff --git a/tests/.gitignore b/tests/.gitignore
index 7fb8deae8..8479f9bb0 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -51,6 +51,7 @@
/test-sha1
/test-skiplist
/test-stp
+/test-stream
/test-strtok_r
/test-timeval
/test-type-props
diff --git a/tests/automake.mk b/tests/automake.mk
index 6a7747cb5..92d56b29d 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -436,6 +436,10 @@ endif
tests_ovstest_LDADD = lib/libopenvswitch.la ovn/lib/libovn.la
+noinst_PROGRAMS += tests/test-stream
+tests_test_stream_SOURCES = tests/test-stream.c
+tests_test_stream_LDADD = lib/libopenvswitch.la
+
noinst_PROGRAMS += tests/test-strtok_r
tests_test_strtok_r_SOURCES = tests/test-strtok_r.c
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 142eee794..775bc0904 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -1785,6 +1785,23 @@ OVSDB_CHECK_IDL_COMPOUND_INDEX_WITH_REF([set, simple3
idl-compound-index-with-re
008: End test
]])
+m4_define([CHECK_STREAM_OPEN_BLOCK],
+ [AT_SETUP([Check Stream open block - C - $1])
+ AT_SKIP_IF([test "$1" = "tcp6" && test "$IS_WIN32" = "yes"])
+ AT_SKIP_IF([test "$1" = "tcp6" && test "$HAVE_IPV6" = "no"])
+ AT_KEYWORDS([Check Stream open block $1])
+ AT_CHECK([ovsdb_start_idltest "ptcp:0:$2"])
+ PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
+ WRONG_PORT=$(($TCP_PORT+1))
+ AT_CHECK([test-stream tcp:$2:$TCP_PORT], [0], [ignore])
+ AT_CHECK([test-stream tcp:$2:$WRONG_PORT], [1], [ignore], [ignore])
+ OVSDB_SERVER_SHUTDOWN
+ AT_CHECK([test-stream tcp:$2:$TCP_PORT], [1], [ignore], [ignore])
+ AT_CLEANUP])
+
+CHECK_STREAM_OPEN_BLOCK([tcp], [127.0.0.1])
+CHECK_STREAM_OPEN_BLOCK([tcp6], [[[::1]]])
+
m4_define([CHECK_STREAM_OPEN_BLOCK_PY],
[AT_SETUP([$1])
AT_SKIP_IF([test $2 = no])
diff --git a/tests/test-stream.c b/tests/test-stream.c
new file mode 100644
index 000000000..4816de02d
--- /dev/null
+++ b/tests/test-stream.c
@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2018 Ilya Maximets <[email protected]>
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <config.h>
+
+#include "fatal-signal.h"
+#include "openvswitch/vlog.h"
+#include "stream.h"
+#include "util.h"
+
+VLOG_DEFINE_THIS_MODULE(test_stream);
+
+int
+main(int argc, char *argv[])
+{
+ int error;
+ struct stream *stream;
+
+ fatal_ignore_sigpipe();
+ set_program_name(argv[0]);
+
+ if (argc < 2) {
+ ovs_fatal(0, "usage: %s REMOTE", argv[0]);
+ }
+
+ error = stream_open_block(stream_open(argv[1], &stream, DSCP_DEFAULT),
+ &stream);
+ if (error) {
+ VLOG_ERR("stream_open_block(%s) failure: %s",
+ argv[1], ovs_strerror(error));
+ }
+ return (error || !stream) ? 1 : 0;
+}
--
2.17.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
