From: Christian Herdtweck Date: Tue, 23 Dec 2014 13:29:42 +0000 (+0100) Subject: hotfix for problem with 0-packages X-Git-Url: http://developer.intra2net.com/git/?a=commitdiff_plain;h=d9bbc1d7b1db48ded2a6d21c34e7b4e4352ecea5;p=pingcheck hotfix for problem with 0-packages * added flag ReceiveHandlerInPlace to icmppinger * to be sure, also check if reply was received already in handle_receive_icmp_packet * in handle_receive_icmp_packet also check error code * in icmp package read, print strings for failure codes * prepend IP to pinger log output so can trace which thread did what --- diff --git a/src/icmp/icmppacket.cpp b/src/icmp/icmppacket.cpp index 50a616d..c012ec9 100644 --- a/src/icmp/icmppacket.cpp +++ b/src/icmp/icmppacket.cpp @@ -39,13 +39,22 @@ IcmpPacket::~IcmpPacket() { } -/** - * - * - */ -{ +std::string IcmpPacket::return_code_to_string( const IcmpPacket::ReadReturnCode &code ) +{ + switch (code) + { + case IcmpPacket::ReadReturnCode_OK: return "ok"; break; + case IcmpPacket::ReadReturnCode_UNSPECIFIED: return "return code not set yet!"; break; + case IcmpPacket::ReadReturnCode_FAIL: return "failed for unspecified reason!"; break; + case IcmpPacket::ReadReturnCode_NOT_ENOUGH_DATA: return "not enough data!"; break; + case IcmpPacket::ReadReturnCode_BAD_STREAM: return "bad stream!"; break; + case IcmpPacket::ReadReturnCode_INVALID_SIZE: return "invalid data size!"; break; + case IcmpPacket::ReadReturnCode_UNKNOWN_PROTOCOL: return "unknown protocol, expect ICMPv4/6!"; break; + } + // if nothing matched + return "unknown return code!"; } /** diff --git a/src/icmp/icmppacket.h b/src/icmp/icmppacket.h index e37686e..139957e 100644 --- a/src/icmp/icmppacket.h +++ b/src/icmp/icmppacket.h @@ -45,12 +45,16 @@ public: enum ReadReturnCode { ReadReturnCode_OK = 0, + ReadReturnCode_UNSPECIFIED, ReadReturnCode_FAIL, ReadReturnCode_NOT_ENOUGH_DATA, ReadReturnCode_BAD_STREAM, - ReadReturnCode_INVALID_SIZE + ReadReturnCode_INVALID_SIZE, + ReadReturnCode_UNKNOWN_PROTOCOL }; + static std::string return_code_to_string( const ReadReturnCode &code ); + virtual ReadReturnCode read( std::istream &is ) = 0; virtual bool write( std::ostream &os ) const = 0; diff --git a/src/icmp/icmppacketfactory.cpp b/src/icmp/icmppacketfactory.cpp index 4828747..6fe8f8c 100644 --- a/src/icmp/icmppacketfactory.cpp +++ b/src/icmp/icmppacketfactory.cpp @@ -52,8 +52,6 @@ IcmpPacketItem IcmpPacketFactory::create_icmp_packet( istream &is ) { - BOOST_ASSERT( (icmp::v4() == protocol) || (icmp::v6() == protocol) ); - IcmpPacketItem icmp_packet; if ( icmp::v4() == protocol ) @@ -66,17 +64,28 @@ IcmpPacketItem IcmpPacketFactory::create_icmp_packet( } else { - BOOST_ASSERT( !"Invalid ICMP Packet Type." ); //lint !e506 + GlobalLogger.warning() << "ICMP packet creation failed: " + << "Unknown protocol, expect ICMP v4 or v6!" << endl; + icmp_packet.reset(); // --> (!icmp_packet) is true + return icmp_packet; } - if ( !icmp_packet->read( is ) ) + IcmpPacket::ReadReturnCode return_code = icmp_packet->read( is ); + if ( return_code != IcmpPacket::ReadReturnCode_OK ) { - GlobalLogger.info() << "ICMP packet creation failed" << endl; + GlobalLogger.warning() << "ICMP packet creation failed: " + << IcmpPacket::return_code_to_string(return_code) << endl; icmp_packet.reset(); // --> (!icmp_packet) is true // --> icmp_pinger will not try to continue working with this packet // TODO why good packets are reports as bad? The read() has problems! } + else if ( !is.good() ) + { + GlobalLogger.warning() << "ICMP packet creation failed: " + << "Stream not good at end of creation!" << endl; + icmp_packet.reset(); // --> (!icmp_packet) is true + } return icmp_packet; } diff --git a/src/icmp/icmppinger.cpp b/src/icmp/icmppinger.cpp index 6309c09..c1076a7 100644 --- a/src/icmp/icmppinger.cpp +++ b/src/icmp/icmppinger.cpp @@ -69,12 +69,15 @@ IcmpPinger::IcmpPinger( ReplyReceived( false ), EchoReplyTimeoutInSec( echo_reply_timeout_in_sec ), PingerStatus( PingStatus_NotSent ), - PingDoneCallback() + PingDoneCallback(), + ReceiveHandlerInPlace( false ) { if ( !NetInterface.bind() ) { - GlobalLogger.error() << "Error: could not bind the socket with the local interface. " - << ::strerror( errno ) << endl; + GlobalLogger.error() + << DestinationEndpoint.address().to_string() + << ": could not bind the socket with the local interface. " + << ::strerror( errno ) << endl; } // Create "unique" identifier @@ -146,7 +149,9 @@ void IcmpPinger::send_echo_request( const IcmpPacketItem icmp_packet ) ostream os( &request_buffer ); if ( !icmp_packet->write( os ) ) { - GlobalLogger.error() << "Error: fail writing ping data." << endl; + GlobalLogger.error() + << DestinationEndpoint.address().to_string() + << ": fail writing ping data." << endl; } TimeSent = microsec_clock::universal_time(); @@ -162,12 +167,16 @@ void IcmpPinger::send_echo_request( const IcmpPacketItem icmp_packet ) size_t bytes_sent = Socket.send_to( data, DestinationEndpoint ); if ( bytes_sent != buffer_size( data ) ) { - GlobalLogger.error() << "Error: fail sending ping data." << endl; + GlobalLogger.error() + << DestinationEndpoint.address().to_string() + << ": fail sending ping data." << endl; } } catch ( const exception &ex ) { - GlobalLogger.error() << "Error: fail sending ping data. " << ex.what() << endl; + GlobalLogger.error() + << DestinationEndpoint.address().to_string() + << ": fail sending ping data. " << ex.what() << endl; } schedule_timeout_echo_reply(); @@ -196,7 +205,9 @@ void IcmpPinger::handle_ping_done() // is also called by Timer.cancel(); if ( !ReplyReceived ) { - GlobalLogger.info() << "Request timed out" << endl; + GlobalLogger.info() + << DestinationEndpoint.address().to_string() + << ": Request timed out" << endl; set_ping_status( PingStatus_FailureTimeout ); } @@ -208,16 +219,20 @@ void IcmpPinger::handle_ping_done() void IcmpPinger::start_receive() { - // Discard any data already in the buffer. - if (ReplyBuffer.size() > 0) - GlobalLogger.debug() << "consuming unused " << ReplyBuffer.size() << "bytes!" << endl; - ReplyBuffer.consume( ReplyBuffer.size() ); + if ( ReceiveHandlerInPlace ) + { + GlobalLogger.info() + << DestinationEndpoint.address().to_string() + << ": Receive Handler in place, do not schedule another one" << endl; + return; + } // Waiting for a reply, We prepare the buffer to receive up to SOCKET_BUFFER_SIZE bytes Socket.async_receive( ReplyBuffer.prepare( SOCKET_BUFFER_SIZE ), - boost::bind( &IcmpPinger::handle_receive_icmp_packet, this, _2 ) + boost::bind( &IcmpPinger::handle_receive_icmp_packet, this, _1, _2 ) ); + ReceiveHandlerInPlace = true; } /** @@ -231,8 +246,28 @@ void IcmpPinger::start_receive() * @param bytes_transferred Number of bytes transferred. * @return void **/ -void IcmpPinger::handle_receive_icmp_packet( const size_t &bytes_transferred ) +void IcmpPinger::handle_receive_icmp_packet( const boost::system::error_code &error, + const size_t &bytes_transferred ) { + + ReceiveHandlerInPlace = false; + + if ( error ) + { + GlobalLogger.warning() + << DestinationEndpoint.address().to_string() + << ": Received error " << error << " in ICMP packet handler; end handler."; + return; + } + if ( ReplyReceived ) + { + GlobalLogger.info() + << DestinationEndpoint.address().to_string() + << ": Got another call to handler, probably due to earlier timeout. " + << "Ignore it." << endl; + return; + } + // The actual number of bytes received is committed to the buffer so that we // can extract it using a std::istream object. ReplyBuffer.commit( bytes_transferred ); @@ -242,7 +277,9 @@ void IcmpPinger::handle_receive_icmp_packet( const size_t &bytes_transferred ) istream is( &ReplyBuffer ); if ( !is ) { - GlobalLogger.error() << "Error: can't handle ReplyBuffer" << endl; + GlobalLogger.error() + << DestinationEndpoint.address().to_string() + << ": Can't handle ReplyBuffer" << endl; return; } @@ -250,7 +287,9 @@ void IcmpPinger::handle_receive_icmp_packet( const size_t &bytes_transferred ) IcmpPacketItem icmp_packet = IcmpPacketFactory::create_icmp_packet( Protocol, is ); if ( !icmp_packet ) { - GlobalLogger.notice() << "Warning: ignoring broken ICMP packet" << endl; + GlobalLogger.warning() + << DestinationEndpoint.address().to_string() + << ": Ignoring broken ICMP packet" << endl; return; } @@ -263,6 +302,10 @@ void IcmpPinger::handle_receive_icmp_packet( const size_t &bytes_transferred ) Identifier, SequenceNumber, DestinationEndpoint.address() ) ) { + GlobalLogger.info() + << DestinationEndpoint.address().to_string() + << ": Received reply" << endl; + ReplyReceived = true; icmp_packet->print_echo_reply( bytes_transferred, TimeSent ); @@ -275,6 +318,10 @@ void IcmpPinger::handle_receive_icmp_packet( const size_t &bytes_transferred ) Identifier, SequenceNumber, DestinationEndpoint.address() ) ) { + GlobalLogger.info() + << DestinationEndpoint.address().to_string() + << ": Received destination unreachable" << endl; + ReplyReceived = true; icmp_packet->print_destination_unreachable(); @@ -286,12 +333,17 @@ void IcmpPinger::handle_receive_icmp_packet( const size_t &bytes_transferred ) // Unknown ICMP reply, start another receive till timeout else { + GlobalLogger.info() + << DestinationEndpoint.address().to_string() + << ": Received packet that does not match" << endl; start_receive(); } } catch ( ... ) { - GlobalLogger.notice() << "Warning: exception during ICMP parse. " + GlobalLogger.notice() + << DestinationEndpoint.address().to_string() + << ": exception during ICMP parse. " << "Starting another receive till timeout." << endl; start_receive(); } diff --git a/src/icmp/icmppinger.h b/src/icmp/icmppinger.h index fc85a90..5aa0547 100644 --- a/src/icmp/icmppinger.h +++ b/src/icmp/icmppinger.h @@ -54,7 +54,8 @@ private: void handle_ping_done(); void start_receive(); - void handle_receive_icmp_packet( const std::size_t &bytes_transferred ); + void handle_receive_icmp_packet( const boost::system::error_code &error, + const std::size_t &bytes_transferred ); void set_ping_status( PingStatus ping_status ); @@ -88,6 +89,8 @@ private: PingStatus PingerStatus; /// Callback to notify when the ping is done (got reply/timeout) boost::function< void(bool) > PingDoneCallback; + /// flag that have registered a handler, so do not accumulate handlers with timeouts + bool ReceiveHandlerInPlace; }; #endif // ICMP_PINGER_H