From 49d66375b97ee7f08c34b7e2e7f66d729a5f606e Mon Sep 17 00:00:00 2001 From: Thomas Jarosch Date: Wed, 11 May 2011 17:04:44 +0200 Subject: [PATCH] Properly handle ICMP packet replies: The kernel sends us a copy of all incoming ICMP messages on every ICMP socket --- src/host/boostpinger.cpp | 98 +++++++++++++++++++++++++++----------------- src/host/pingscheduler.cpp | 5 ++- src/icmp/icmppacket.cpp | 16 +++++-- 3 files changed, 76 insertions(+), 43 deletions(-) diff --git a/src/host/boostpinger.cpp b/src/host/boostpinger.cpp index b23ff20..a979044 100644 --- a/src/host/boostpinger.cpp +++ b/src/host/boostpinger.cpp @@ -1,6 +1,5 @@ #include "host/boostpinger.h" -#include #include #include @@ -209,57 +208,80 @@ void BoostPinger::handle_ping_done() PingDoneCallback(ping_success); } +/** + * @brief Receive ICMP packets + * Note: Will receive -all- ICMP packets from the kernel. So if the packet doesn't match + * our criteria, we have to schedule another start_receive() requests. The timeout stays the same. + * + * In the future we might redesign the code to handle all ICMP packets + * from a single raw socket. + * + * @param bytes_transferred Number of bytes transferred + * @return void + **/ void BoostPinger::handle_receive_icmp_packet( const size_t &bytes_transferred ) { // 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 ); - istream is( &ReplyBuffer ); - if ( !is ) + try { - GlobalLogger.error() << "can't handle ReplyBuffer" << endl; - return; - } + istream is( &ReplyBuffer ); + if ( !is ) + { + GlobalLogger.error() << "can't handle ReplyBuffer" << endl; + return; + } - // Decode the reply packet. - IcmpPacket icmp_packet; - if (!(is >> icmp_packet)) - { - GlobalLogger.error() << "ignoring broken ICMP packet" << endl; - return; - } + // Decode the reply packet. + IcmpPacket icmp_packet; + if (!(is >> icmp_packet)) + { + GlobalLogger.notice() << "ignoring broken ICMP packet" << endl; + return; + } - // We can receive all ICMP packets received by the host, so we need to - // filter out only the echo replies that match the our identifier, - // expected sequence number, and destination host address (receive just the - // ICMP packets from the host we had ping). - if ( icmp_packet.match( - IcmpType_EchoReply, Identifier, SequenceNumber, - DestinationEndpoint.address()) - ) - { - ReceivedReply = true; + // We can receive all ICMP packets received by the host, so we need to + // filter out only the echo replies that match the our identifier, + // expected sequence number, and destination host address (receive just the + // ICMP packets from the host we had ping). + if ( icmp_packet.match( + IcmpType_EchoReply, Identifier, SequenceNumber, + DestinationEndpoint.address()) + ) + { + ReceivedReply = true; + print_echo_reply( icmp_packet, bytes_transferred ); - print_echo_reply( icmp_packet, bytes_transferred ); + set_ping_status( PingStatus_SuccessReply ); - set_ping_status( PingStatus_SuccessReply ); - } - else if ( icmp_packet.match( - IcmpType_DestinationUnreachable, Identifier, SequenceNumber, - DestinationEndpoint.address() - ) ) - { - ReceivedReply = true; - print_destination_unreachable( icmp_packet ); + IcmpPacketReceiveTimer.cancel(); + } + else if ( icmp_packet.match( + IcmpType_DestinationUnreachable, Identifier, SequenceNumber, + DestinationEndpoint.address() + ) ) + { + ReceivedReply = true; + print_destination_unreachable( icmp_packet ); + + set_ping_status( PingStatus_FailureDestinationUnreachable ); - set_ping_status( PingStatus_FailureDestinationUnreachable ); - } else + IcmpPacketReceiveTimer.cancel(); + } else + { + /* + GlobalLogger.notice() << "unknown ICMP reply (src IP: " << icmp_packet.get_ip_header().get_source_address() << ")" + << ". Starting another recieve till timeout." << endl; + */ + start_receive(); + } + } catch(...) { - GlobalLogger.notice() << "unknown ICMP reply" << endl; + GlobalLogger.notice() << "exception during ICMP parse. Starting another recieve till timeout." << endl; + start_receive(); } - - IcmpPacketReceiveTimer.cancel(); } void BoostPinger::print_echo_reply( diff --git a/src/host/pingscheduler.cpp b/src/host/pingscheduler.cpp index 91f6a7a..8065645 100644 --- a/src/host/pingscheduler.cpp +++ b/src/host/pingscheduler.cpp @@ -25,6 +25,9 @@ using I2n::Logger::GlobalLogger; // PingScheduler //----------------------------------------------------------------------------- +/// Ping reply timeout. Could be made a configuration variable +const int PingReplyTimeout = 30; + PingScheduler::PingScheduler( const string &network_interface, const string &destination_address, @@ -41,7 +44,7 @@ PingScheduler::PingScheduler( PingIntervalInSec( ping_interval_in_sec ), IpList( destination_address, nameserver ), HostAnalyzer( destination_address, ping_fail_percentage_limit, link_analyzer ), - Pinger(IoService, LocalNetworkInterfaceName, 5), + Pinger(IoService, LocalNetworkInterfaceName, PingReplyTimeout), Thread() { } diff --git a/src/icmp/icmppacket.cpp b/src/icmp/icmppacket.cpp index 2f75248..88746e1 100644 --- a/src/icmp/icmppacket.cpp +++ b/src/icmp/icmppacket.cpp @@ -1,4 +1,5 @@ #include "icmp/icmppacket.h" +#include using namespace std; using boost::asio::ip::address; @@ -50,10 +51,17 @@ bool IcmpPacket::match( const address &source_address ) const { - bool type_match = IcmpPayloadHeader.get_type() == type; - bool identifier_match = IcmpPayloadHeader.get_identifier() == identifier; - bool seq_num_match = IcmpPayloadHeader.get_sequence_number() == sequence_number; - bool address_match = IpHeader.get_source_address() == source_address; + bool type_match = IcmpPayloadHeader.get_type() == type ? true : false; + bool identifier_match = IcmpPayloadHeader.get_identifier() == identifier ? true: false; + bool seq_num_match = IcmpPayloadHeader.get_sequence_number() == sequence_number ? true : false; + bool address_match = IpHeader.get_source_address() == source_address ? true : false; + + /* + std::cout << "Packet type: " << IcmpPayloadHeader.get_type() << ", search_type: " << type << ", match: " << (type_match ? "YES" : "NO") << std::endl; + std::cout << "Packet identifier: " << IcmpPayloadHeader.get_identifier() << ", search_ident: " << identifier << ", match: " << (identifier_match ? "YES" : "NO") << std::endl; + std::cout << "Packet seq: " << IcmpPayloadHeader.get_sequence_number() << ", search_seq: " << sequence_number << ", match: " << (seq_num_match ? "YES" : "NO") << std::endl; + std::cout << "Packet addr: " << IpHeader.get_source_address() << ", search_addr: " << source_address << ", match: " << (address_match ? "YES" : "NO") << std::endl; +*/ return ( type_match && identifier_match && seq_num_match && address_match ); } -- 1.7.1