From cfef5ae8f0e3954b829a60df702ffea4cd02d4d6 Mon Sep 17 00:00:00 2001 From: Ilya Maximets Date: Fri, 21 Dec 2018 16:09:50 +0300 Subject: [PATCH] socket-util: Report POLLHUP as an error while connection completion checking. 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 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 Signed-off-by: Ben Pfaff --- 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 + * + * 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 + +#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; +}