From: Thomas Markwalder Date: Wed, 26 Jun 2019 19:32:55 +0000 (-0400) Subject: [#691,!395] More review comments X-Git-Tag: Kea-1.6.0-beta2~187 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4a0b024bc6d83b26fe702d95ee7ce0c914b37d8e;p=thirdparty%2Fkea.git [#691,!395] More review comments src/hooks/dhcp/high_availability/ha_messages.mes Removed HA_SERVICE_CONNECT_INVALID_SOCKET message src/hooks/dhcp/high_availability/ha_service.cc HAService::clientConnectHandler() - now just avoids registering an invalid FD with no log and return(true) src/lib/http/client.cc Added commen in Connection::close() src/lib/http/tests/server_client_unittests.cc Removed invalid FD failure in test --- diff --git a/src/hooks/dhcp/high_availability/ha_messages.cc b/src/hooks/dhcp/high_availability/ha_messages.cc index 52fc6d53b5..708db8faca 100644 --- a/src/hooks/dhcp/high_availability/ha_messages.cc +++ b/src/hooks/dhcp/high_availability/ha_messages.cc @@ -1,4 +1,4 @@ -// File created from ../../../../src/hooks/dhcp/high_availability/ha_messages.mes on Wed Jun 26 2019 09:25 +// File created from ../../../../src/hooks/dhcp/high_availability/ha_messages.mes on Wed Jun 26 2019 15:13 #include #include @@ -60,7 +60,6 @@ extern const isc::log::MessageID HA_LOCAL_DHCP_DISABLE = "HA_LOCAL_DHCP_DISABLE" extern const isc::log::MessageID HA_LOCAL_DHCP_ENABLE = "HA_LOCAL_DHCP_ENABLE"; extern const isc::log::MessageID HA_MISSING_CONFIGURATION = "HA_MISSING_CONFIGURATION"; extern const isc::log::MessageID HA_SCOPES_HANDLER_FAILED = "HA_SCOPES_HANDLER_FAILED"; -extern const isc::log::MessageID HA_SERVICE_CONNECT_INVALID_SOCKET = "HA_SERVICE_CONNECT_INVALID_SOCKET"; extern const isc::log::MessageID HA_SERVICE_STARTED = "HA_SERVICE_STARTED"; extern const isc::log::MessageID HA_STATE_MACHINE_CONTINUED = "HA_STATE_MACHINE_CONTINUED"; extern const isc::log::MessageID HA_STATE_MACHINE_PAUSED = "HA_STATE_MACHINE_PAUSED"; @@ -130,7 +129,6 @@ const char* values[] = { "HA_LOCAL_DHCP_ENABLE", "local DHCP service is enabled while the %1 is in the %2 state", "HA_MISSING_CONFIGURATION", "high-availability parameter not specified for High Availability hooks library", "HA_SCOPES_HANDLER_FAILED", "ha-scopes command failed: %1", - "HA_SERVICE_CONNECT_INVALID_SOCKET", "Attempted to register an invalid socket, error code: %1", "HA_SERVICE_STARTED", "started high availability service in %1 mode as %2 server", "HA_STATE_MACHINE_CONTINUED", "state machine is un-paused", "HA_STATE_MACHINE_PAUSED", "state machine paused in state %1", diff --git a/src/hooks/dhcp/high_availability/ha_messages.h b/src/hooks/dhcp/high_availability/ha_messages.h index 14337ab4bb..bbe359fe03 100644 --- a/src/hooks/dhcp/high_availability/ha_messages.h +++ b/src/hooks/dhcp/high_availability/ha_messages.h @@ -1,4 +1,4 @@ -// File created from ../../../../src/hooks/dhcp/high_availability/ha_messages.mes on Wed Jun 26 2019 09:25 +// File created from ../../../../src/hooks/dhcp/high_availability/ha_messages.mes on Wed Jun 26 2019 15:13 #ifndef HA_MESSAGES_H #define HA_MESSAGES_H @@ -61,7 +61,6 @@ extern const isc::log::MessageID HA_LOCAL_DHCP_DISABLE; extern const isc::log::MessageID HA_LOCAL_DHCP_ENABLE; extern const isc::log::MessageID HA_MISSING_CONFIGURATION; extern const isc::log::MessageID HA_SCOPES_HANDLER_FAILED; -extern const isc::log::MessageID HA_SERVICE_CONNECT_INVALID_SOCKET; extern const isc::log::MessageID HA_SERVICE_STARTED; extern const isc::log::MessageID HA_STATE_MACHINE_CONTINUED; extern const isc::log::MessageID HA_STATE_MACHINE_PAUSED; diff --git a/src/hooks/dhcp/high_availability/ha_messages.mes b/src/hooks/dhcp/high_availability/ha_messages.mes index 83a9231378..85ba97bea4 100644 --- a/src/hooks/dhcp/high_availability/ha_messages.mes +++ b/src/hooks/dhcp/high_availability/ha_messages.mes @@ -116,11 +116,6 @@ failure. This informational message indicates that the High Availability hooks library has been unloaded successfully. -% HA_SERVICE_CONNECT_INVALID_SOCKET Attempted to register an invalid socket, error code: %1 -This is an error message that indicates that the server attempted to -register an invalid socket with the Interface Manager. This is an internal -server error and a bug report should be created. - % HA_DHCP4_START_SERVICE_FAILED failed to start DHCPv4 HA service in dhcp4_srv_configured callout: %1 This error message is issued when an attempt to start High Availability service for the DHCPv4 server failed in the dhcp4_srv_configured callout. This diff --git a/src/hooks/dhcp/high_availability/ha_service.cc b/src/hooks/dhcp/high_availability/ha_service.cc index f3188b3019..4589a8ad57 100644 --- a/src/hooks/dhcp/high_availability/ha_service.cc +++ b/src/hooks/dhcp/high_availability/ha_service.cc @@ -1621,19 +1621,17 @@ HAService::verifyAsyncResponse(const HttpResponsePtr& response) { bool HAService::clientConnectHandler(const boost::system::error_code& ec, int tcp_native_fd) { - if (!ec || (ec.value() == boost::asio::error::in_progress)) { - if (tcp_native_fd < 0) { - // This really should not be possible, but just in case. - LOG_ERROR(ha_logger, HA_SERVICE_CONNECT_INVALID_SOCKET) - .arg(ec.value()); - return (false); - } - - // Register the socket with Interface Manager. - // External socket callback is a NOP. Ready events handlers are run by an - // explicit call IOService ready in kea-dhcp code. We are registering - // the socket only to interrupt main-thread select(). + // If things look ok register the socket with Interface Manager. Note + // we don't register if the FD is < 0 to avoid an expection throw. + // It is unlikely that this will occur but we want to be liberal + // and avoid issues. + if ((!ec || (ec.value() == boost::asio::error::in_progress)) + && (tcp_native_fd >= 0)) { + // External socket callback is a NOP. Ready events handlers are + // run by an explicit call IOService ready in kea-dhcp code. + // We are registerin the socket only to interrupt main-thread + // select(). IfaceMgr::instance().addExternalSocket(tcp_native_fd, 0); } diff --git a/src/lib/http/client.cc b/src/lib/http/client.cc index a45236fb7e..50a581fcfb 100644 --- a/src/lib/http/client.cc +++ b/src/lib/http/client.cc @@ -594,6 +594,7 @@ Connection::doTransaction(const HttpRequestPtr& request, void Connection::close() { + // Pass in true to discard the callback. closeCallback(true); timer_.cancel(); diff --git a/src/lib/http/tests/server_client_unittests.cc b/src/lib/http/tests/server_client_unittests.cc index ad705fa39e..dd846eb131 100644 --- a/src/lib/http/tests/server_client_unittests.cc +++ b/src/lib/http/tests/server_client_unittests.cc @@ -1473,21 +1473,13 @@ public: /// @param tcp_native_fd socket descriptor to register bool connectHandler(const boost::system::error_code& ec, int tcp_native_fd) { ++connect_cnt_; - if (!ec || ec.value() == boost::asio::error::in_progress) { - if (tcp_native_fd >= 0) { - registered_fd_ = tcp_native_fd; - return (true); - } - - // Invalid fd?, this really should not be possible. EXPECT makes - // sure we log it. - EXPECT_TRUE (tcp_native_fd >= 0) << "no ec error but invalid fd?"; + if ((!ec || (ec.value() == boost::asio::error::in_progress)) + && (tcp_native_fd >= 0)) { + registered_fd_ = tcp_native_fd; + return (true); + } else if ((ec.value() == boost::asio::error::already_connected) + && (registered_fd_ != tcp_native_fd)) { return (false); - - } else if (ec.value() == boost::asio::error::already_connected) { - if (registered_fd_ != tcp_native_fd) { - return (false); - } } // ec indicates an error, return true, so that error can be handled