From 96c4e7a450708663d92bd3bf5c8a4cea610b6dea Mon Sep 17 00:00:00 2001 From: Christian Herdtweck Date: Fri, 22 May 2015 15:28:47 +0200 Subject: [PATCH] give HostStatus analyzer more info: details on ping success/failure and ping duration preparing HostStatus to decide other ping timeouts/frequencies/numbers and DNS timeout --- src/CMakeLists.txt | 1 + src/host/hoststatus.cpp | 16 ++++++--- src/host/hoststatus.h | 4 ++- src/host/pinger.h | 4 ++- src/host/pingscheduler.cpp | 42 ++++++++++-------------- src/host/pingscheduler.h | 4 +- src/host/pingstatus.cpp | 46 ++++++++++++++++++++++++++ src/host/pingstatus.h | 11 ++++++- src/icmp/icmppinger.cpp | 20 ++++++----- src/icmp/icmppinger.h | 4 +- src/tcp/tcppinger.cpp | 5 +-- src/tcp/tcppinger.h | 4 +- test/CMakeLists.test_hoststatus.txt | 1 + test/test_hoststatus.cpp | 61 ++++++++++++++++++----------------- 14 files changed, 143 insertions(+), 80 deletions(-) create mode 100644 src/host/pingstatus.cpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 10b4f12..2b6b7e3 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -69,6 +69,7 @@ set(SOURCES dns/dnsresolver.cpp dns/dnsmaster.cpp host/hoststatus.cpp + host/pingstatus.cpp host/loglevel.cpp host/logoutput.cpp host/messagepayload.cpp diff --git a/src/host/hoststatus.cpp b/src/host/hoststatus.cpp index d620e58..b5d5bdb 100644 --- a/src/host/hoststatus.cpp +++ b/src/host/hoststatus.cpp @@ -86,14 +86,20 @@ bool HostStatus::exceeded_ping_failed_limit() const } /** - * Adds a ping status (success or failure). - * @param ping_success + * Tells the status analyzer how the last ping went + * + * @param result: status of ping specifying success/failure and reason of fail + * @param ping_duration_us duration of ping in micro seconds */ -void HostStatus::update_ping_statistics( bool ping_success ) +void HostStatus::update_ping_statistics( const PingStatus &result, + const long ping_duration_us ) { + float ping_duration_ms = static_cast(ping_duration_us) / 1000.; + GlobalLogger.debug() << "Stat(" << HostAddress << "): " << PingsFailedCount << " fail/" << PingsPerformedCount << " pings/" - << ResolvedIpCount << " IPs: add ping with success=" << ping_success; + << ResolvedIpCount << " IPs: add ping with result " + << to_string(result) << " which took " << ping_duration_ms << " ms"; BOOST_ASSERT( 1 <= ResolvedIpCount ); BOOST_ASSERT( 0 <= PingsPerformedCount ); @@ -101,7 +107,7 @@ void HostStatus::update_ping_statistics( bool ping_success ) increase_ping_performed_count(); - if ( !ping_success ) + if ( result != PingStatus_SuccessReply ) { increase_ping_failed_count(); } diff --git a/src/host/hoststatus.h b/src/host/hoststatus.h index c5f558d..a167afe 100644 --- a/src/host/hoststatus.h +++ b/src/host/hoststatus.h @@ -22,6 +22,7 @@ on this file might be covered by the GNU General Public License. #include +#include "host/pingstatus.h" #include "link/linkstatus.h" //----------------------------------------------------------------------------- @@ -45,7 +46,8 @@ public: void set_resolved_ip_count( const int resolved_ip_count ); bool exceeded_ping_failed_limit() const; - void update_ping_statistics( bool ping_success ); + void update_ping_statistics( const PingStatus &ping_success, + const long ping_duration_us ); private: bool tried_all_resolved_ip() const; diff --git a/src/host/pinger.h b/src/host/pinger.h index 47d3e21..fa3a28e 100644 --- a/src/host/pinger.h +++ b/src/host/pinger.h @@ -32,6 +32,8 @@ #include #include +#include "host/pingstatus.h" + //----------------------------------------------------------------------------- // IoServiceItem //----------------------------------------------------------------------------- @@ -57,7 +59,7 @@ public: virtual void ping( const boost::asio::ip::address &ip, const uint16_t destination_port, - boost::function ping_done_callback + boost::function ping_done_callback ) = 0; virtual void stop_pinging() = 0; diff --git a/src/host/pingscheduler.cpp b/src/host/pingscheduler.cpp index abeb02d..0bf1777 100644 --- a/src/host/pingscheduler.cpp +++ b/src/host/pingscheduler.cpp @@ -207,7 +207,7 @@ void PingScheduler::ping_when_ready() // skip the ping and directly call ping_done_handler HostAnalyzer.set_resolved_ip_count(1); // must have been 0 --> failed // ping would create failed assumption (nPings > nIPs) - ping_done_handler(false); + ping_done_handler(PingStatus_FailureNoIP); HostAnalyzer.set_resolved_ip_count(0); // set back } else @@ -228,6 +228,7 @@ void PingScheduler::ping_when_ready() Ping->ping( ip.get_ip(), DestinationPort, boost::bind(&PingScheduler::ping_done_handler, this, _1) ); + TimeSentLastPing = microsec_clock::universal_time(); } } @@ -241,25 +242,30 @@ void PingScheduler::ping_when_ready() * statistics, ping interval and elapsed time; * schedules a call to ping, thereby closing the loop */ -void PingScheduler::ping_done_handler( const bool ping_success ) +void PingScheduler::ping_done_handler( const PingStatus &result ) { - GlobalLogger.info() << LogPrefix << "Ping done with success = " - << ping_success; + PingStatus edited_result = result; + if (result == PingStatus_SuccessReply && ContinueOnOutdatedIps) + { + edited_result = PingStatus_SuccessOutdatedIP; + + // reset ContinueOnOutdatedIps + ContinueOnOutdatedIps = false; + update_log_prefix(); + } + + GlobalLogger.info() << LogPrefix << "Ping done with result " + << to_string(edited_result); // post-processing // You must call these 3 methods exactly in this order // TODO Fix this method, once it has a semantic dependency with the // update_ping_statistics method, because it depends on the PingAnalyzer // statistics to update the exceeded_ping_failed_limit - HostAnalyzer.update_ping_statistics( ping_success ); + ptime now = microsec_clock::universal_time(); + HostAnalyzer.update_ping_statistics( edited_result, + (now - TimeSentLastPing).total_microseconds()); update_ping_interval(); - update_ping_elapsed_time(); - - if (ping_success) - { // reset ContinueOnOutdatedIps - ContinueOnOutdatedIps = false; - update_log_prefix(); - } // get next protocol, possibly start resolving IPs update_ping_protocol(); @@ -289,18 +295,6 @@ void PingScheduler::update_ping_interval() } } -void PingScheduler::update_ping_elapsed_time() -{ - ptime now = microsec_clock::universal_time(); - time_resolution_traits_adapted64_impl::int_type elapsed_time_in_sec = - (now - TimeSentLastPing).total_seconds(); - GlobalLogger.debug() << LogPrefix << "- Time elapsed since last ping: " - << elapsed_time_in_sec << "s"; - - TimeSentLastPing = microsec_clock::universal_time(); -} - - //------------------------------------------------------------------------------ // Ping Protocol Rotation //------------------------------------------------------------------------------ diff --git a/src/host/pingscheduler.h b/src/host/pingscheduler.h index 106f69c..37d94a1 100644 --- a/src/host/pingscheduler.h +++ b/src/host/pingscheduler.h @@ -30,6 +30,7 @@ on this file might be covered by the GNU General Public License. #include "link/linkstatus.h" #include "host/hoststatus.h" +#include "host/pingstatus.h" #include "host/pinger.h" #include "host/pinginterval.h" #include "host/pingprotocol.h" @@ -70,9 +71,8 @@ private: // void ping(const boost::system::error_code &error); - void ping_done_handler(const bool ping_success); + void ping_done_handler(const PingStatus &result); void update_ping_interval(); - void update_ping_elapsed_time(); void init_ping_protocol(); void update_ping_protocol(); diff --git a/src/host/pingstatus.cpp b/src/host/pingstatus.cpp new file mode 100644 index 0000000..1b255aa --- /dev/null +++ b/src/host/pingstatus.cpp @@ -0,0 +1,46 @@ +/* + The software in this package is distributed under the GNU General + Public License version 2 (with a special exception described below). + + A copy of GNU General Public License (GPL) is included in this distribution, + in the file COPYING.GPL. + + As a special exception, if other files instantiate templates or use macros + or inline functions from this file, or you compile this file and link it + with other works to produce a work based on this file, this file + does not by itself cause the resulting work to be covered + by the GNU General Public License. + + However the source code for this file must still be made available + in accordance with section (3) of the GNU General Public License. + + This exception does not invalidate any other reasons why a work based + on this file might be covered by the GNU General Public License. + + Christian Herdtweck, Intra2net AG 2015 + */ + +#include "host/pingstatus.h" + +std::string to_string( const PingStatus &status ) +{ + switch (status) + { + case PingStatus_NotSent: return "PingNotSent"; break; + case PingStatus_SuccessReply: return "PingSucceeded"; break; + case PingStatus_FailureOther: return "PingFailed"; break; + case PingStatus_FailureTimeout: return "PingTimedOut"; break; + case PingStatus_FailureDestinationUnreachable: + return "PingFailed(DestUnreachable)"; + break; + case PingStatus_FailureNoIP: return "PingFailed(NoIP)"; break; + case PingStatus_SuccessOutdatedIP: return "PingSucceeded(OutdatedIP)"; + break; + case PingStatus_FailureAsyncCancel: return "PingFailed(AsyncCancel)"; + break; + case PingStatus_FailureAsyncError: return "PingFailed(AsyncError)"; + break; + default: return "PingStatusUnknown"; break; + } +} + diff --git a/src/host/pingstatus.h b/src/host/pingstatus.h index 30f412a..f62dc88 100644 --- a/src/host/pingstatus.h +++ b/src/host/pingstatus.h @@ -21,12 +21,21 @@ on this file might be covered by the GNU General Public License. #ifndef PING_STATUS_H #define PING_STATUS_H +#include + enum PingStatus { PingStatus_NotSent, PingStatus_SuccessReply, + PingStatus_FailureOther, PingStatus_FailureTimeout, - PingStatus_FailureDestinationUnreachable + PingStatus_FailureDestinationUnreachable, + PingStatus_FailureNoIP, + PingStatus_SuccessOutdatedIP, + PingStatus_FailureAsyncCancel, + PingStatus_FailureAsyncError }; +std::string to_string( const PingStatus &status ); + #endif // PING_STATUS_H diff --git a/src/icmp/icmppinger.cpp b/src/icmp/icmppinger.cpp index 93a071d..12832d6 100644 --- a/src/icmp/icmppinger.cpp +++ b/src/icmp/icmppinger.cpp @@ -127,7 +127,7 @@ IcmpPinger::~IcmpPinger() void IcmpPinger::ping( const address &destination_ip, const uint16_t /*destination_port*/, // the ICMP protocol does not use ports - function ping_done_callback + function ping_done_callback ) { PingDoneCallback = ping_done_callback; @@ -238,7 +238,7 @@ void IcmpPinger::schedule_timeout_echo_reply() /** * @brief Gets called when the ping is finished: Either on timeout or on ping reply * - * @return void + * @return void (but calls PingDoneCallback) **/ void IcmpPinger::handle_timeout(const boost::system::error_code& error) { @@ -247,32 +247,34 @@ void IcmpPinger::handle_timeout(const boost::system::error_code& error) if ( error == boost::asio::error::operation_aborted ) { if (! ReplyReceived) + { GlobalLogger.notice() << LogPrefix << "Timer waiting for ICMP echo reply was cancelled!" << endl; + set_ping_status( PingStatus_FailureAsyncCancel ); + } // otherwise probably called by IcmpPacketReceiveTimer.cancel in // handle_receive_icmp_packet! } else + { GlobalLogger.notice() << LogPrefix << "Error " << error << " waiting for ICMP echo reply!" << endl; + set_ping_status( PingStatus_FailureAsyncError ); + } // Still continue with rest of function, so PingStatus is updated and Callback executed // when timer was cancelled } - - // Check ReplyReceived as the timer handler - // is also called by Timer.cancel(); - if ( !ReplyReceived ) - { + else if ( !ReplyReceived ) + { // Check ReplyReceived since the timer handler is also called by Timer.cancel(); GlobalLogger.info() << LogPrefix << "Request timed out" << endl; set_ping_status( PingStatus_FailureTimeout ); } // Call ping-done handler - bool ping_success = (PingerStatus == PingStatus_SuccessReply); - PingDoneCallback( ping_success ); + PingDoneCallback( PingerStatus ); } diff --git a/src/icmp/icmppinger.h b/src/icmp/icmppinger.h index f6ce523..e9fb98a 100644 --- a/src/icmp/icmppinger.h +++ b/src/icmp/icmppinger.h @@ -127,7 +127,7 @@ public: virtual void ping( const boost::asio::ip::address &destination_ip, const uint16_t destination_port, - boost::function ping_done_callback + boost::function ping_done_callback ); virtual void stop_pinging(); @@ -176,7 +176,7 @@ private: /// The status of the pinger PingStatus PingerStatus; /// Callback to notify when the ping is done (got reply/timeout) - boost::function< void(bool) > PingDoneCallback; + boost::function< void(PingStatus) > PingDoneCallback; /// prefix to logging output lines std::string LogPrefix; }; diff --git a/src/tcp/tcppinger.cpp b/src/tcp/tcppinger.cpp index 1cd857d..8bf6a7a 100644 --- a/src/tcp/tcppinger.cpp +++ b/src/tcp/tcppinger.cpp @@ -115,7 +115,7 @@ TcpPinger::~TcpPinger() void TcpPinger::ping( const address &destination_ip, const uint16_t destination_port, - function ping_done_callback + function ping_done_callback ) { BOOST_ASSERT( ( 0 < destination_port ) && ( destination_port < numeric_limits::max() ) ); @@ -252,8 +252,7 @@ void TcpPinger::handle_ping_done() } // Call ping-done handler - bool ping_success = (PingerStatus == PingStatus_SuccessReply); - PingDoneCallback( ping_success ); + PingDoneCallback( PingerStatus ); } void TcpPinger::start_receive() diff --git a/src/tcp/tcppinger.h b/src/tcp/tcppinger.h index 19effee..05e9586 100644 --- a/src/tcp/tcppinger.h +++ b/src/tcp/tcppinger.h @@ -49,7 +49,7 @@ public: virtual void ping( const boost::asio::ip::address &destination_ip, const uint16_t destination_port, - boost::function ping_done_callback + boost::function ping_done_callback ); virtual void stop_pinging(); @@ -117,7 +117,7 @@ private: /// The status of the pinger PingStatus PingerStatus; /// Callback to notify when the ping is done (got reply/timeout) - boost::function< void(bool) > PingDoneCallback; + boost::function< void(PingStatus) > PingDoneCallback; }; #endif // TCP_PINGER_H diff --git a/test/CMakeLists.test_hoststatus.txt b/test/CMakeLists.test_hoststatus.txt index 097a23e..bf1bc29 100644 --- a/test/CMakeLists.test_hoststatus.txt +++ b/test/CMakeLists.test_hoststatus.txt @@ -3,6 +3,7 @@ add_executable(test_hoststatus test_hoststatus.cpp ${CMAKE_SOURCE_DIR}/src/boost_assert_handler.cpp ${CMAKE_SOURCE_DIR}/src/host/hoststatus.cpp + ${CMAKE_SOURCE_DIR}/src/host/pingstatus.cpp ) # linker: link the program against the libraries diff --git a/test/test_hoststatus.cpp b/test/test_hoststatus.cpp index a97f92c..752822f 100644 --- a/test/test_hoststatus.cpp +++ b/test/test_hoststatus.cpp @@ -29,6 +29,7 @@ on this file might be covered by the GNU General Public License. #include "mock_linkstatus.h" #include "host/hoststatus.h" +#include "host/pingstatus.h" BOOST_AUTO_TEST_SUITE( TestHostStatus ) @@ -41,34 +42,34 @@ BOOST_AUTO_TEST_CASE( fail_percentage_10 ) HostStatus host_status( "localhost", ping_fail_percentage_limit, link_status ); host_status.set_resolved_ip_count( resolved_ip_count ); - host_status.update_ping_statistics( true ); + host_status.update_ping_statistics( PingStatus_SuccessReply, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true ); - host_status.update_ping_statistics( true ); + host_status.update_ping_statistics( PingStatus_SuccessReply, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true ); - host_status.update_ping_statistics( true ); + host_status.update_ping_statistics( PingStatus_SuccessReply, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true ); - host_status.update_ping_statistics( true ); + host_status.update_ping_statistics( PingStatus_SuccessReply, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true ); - host_status.update_ping_statistics( true ); + host_status.update_ping_statistics( PingStatus_SuccessReply, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true ); - host_status.update_ping_statistics( true ); + host_status.update_ping_statistics( PingStatus_SuccessReply, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true ); - host_status.update_ping_statistics( true ); + host_status.update_ping_statistics( PingStatus_SuccessReply, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true ); - host_status.update_ping_statistics( true ); + host_status.update_ping_statistics( PingStatus_SuccessReply, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true ); } @@ -81,34 +82,34 @@ BOOST_AUTO_TEST_CASE( fail_percentage_50 ) HostStatus host_status( "localhost", ping_fail_percentage_limit, link_status ); host_status.set_resolved_ip_count( resolved_ip_count ); - host_status.update_ping_statistics( true ); + host_status.update_ping_statistics( PingStatus_SuccessReply, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( true ); + host_status.update_ping_statistics( PingStatus_SuccessReply, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( true ); + host_status.update_ping_statistics( PingStatus_SuccessReply, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true ); - host_status.update_ping_statistics( true ); + host_status.update_ping_statistics( PingStatus_SuccessReply, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true ); } @@ -121,34 +122,34 @@ BOOST_AUTO_TEST_CASE( fail_percentage_80 ) HostStatus host_status( "localhost", ping_fail_percentage_limit, link_status ); host_status.set_resolved_ip_count( resolved_ip_count ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false ); - host_status.update_ping_statistics( false ); + host_status.update_ping_statistics( PingStatus_FailureOther, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true ); - host_status.update_ping_statistics( true ); + host_status.update_ping_statistics( PingStatus_SuccessReply, 1 ); BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true ); } -- 1.7.1