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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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