From 35530f1436364339d2ecbd7f4b1a12fd81034063 Mon Sep 17 00:00:00 2001 From: Schaffner Brice METAS Date: Thu, 11 Apr 2019 15:42:08 +0200 Subject: [PATCH 1/4] Add a delay option between each ICMP request to avoid burst issues which can create a lot of packet loss on some networks. --- multiping/__init__.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/multiping/__init__.py b/multiping/__init__.py index 970bb0c..5f166b4 100644 --- a/multiping/__init__.py +++ b/multiping/__init__.py @@ -71,7 +71,8 @@ class MultiPingSocketError(socket.gaierror): class MultiPing(object): - def __init__(self, dest_addrs, sock=None, ignore_lookup_errors=False): + def __init__(self, dest_addrs, sock=None, ignore_lookup_errors=False, + delay=0): """ Initialize a new multi ping object. This takes the configuration consisting of the list of destination addresses and an optional socket @@ -95,6 +96,7 @@ def __init__(self, dest_addrs, sock=None, ignore_lookup_errors=False): "65535 addresses at the same time.") self._ignore_lookup_errors = ignore_lookup_errors + self._delay = delay # delay between each ICMP request # Get the IP addresses for every specified target: We allow # specification of the ping targets by name, so a name lookup needs to @@ -301,6 +303,11 @@ def send(self): # of the current time stamp. This is returned to us in the # response and allows us to calculate the 'ping time'. self._send_ping(addr, payload=struct.pack("d", time.time())) + # Some system/network doesn't support the bombarding of ICMP + # request and lead to a lot of packet loss and retry, therefore + # introcude a small delay between each request. + if self._delay > 0: + time.sleep(self._delay) def _read_all_from_socket(self, timeout): """ @@ -457,7 +464,7 @@ def receive(self, timeout): return (results, no_results_so_far) -def multi_ping(dest_addrs, timeout, retry=0, ignore_lookup_errors=False): +def multi_ping(dest_addrs, timeout, retry=0, ignore_lookup_errors=False, delay=0): """ Combine send and receive measurement into single function. @@ -475,11 +482,15 @@ def multi_ping(dest_addrs, timeout, retry=0, ignore_lookup_errors=False): names or looking up their address information will silently be ignored. Those targets simply appear in the 'no_results' return list. + The 'delay' parameter can be used to introduced a small delay between + each requests. + """ retry = int(retry) if retry < 0: retry = 0 + timeout = float(timeout) if timeout < 0.1: raise MultiPingError("Timeout < 0.1 seconds not allowed") @@ -488,7 +499,8 @@ def multi_ping(dest_addrs, timeout, retry=0, ignore_lookup_errors=False): if retry_timeout < 0.1: raise MultiPingError("Time between ping retries < 0.1 seconds") - mp = MultiPing(dest_addrs, ignore_lookup_errors=ignore_lookup_errors) + mp = MultiPing(dest_addrs, ignore_lookup_errors=ignore_lookup_errors, + delay=delay) results = {} retry_count = 0 From c6e4a291c02ac53cc93b2b425a29ed049cbc6603 Mon Sep 17 00:00:00 2001 From: Schaffner Brice METAS Date: Thu, 11 Apr 2019 15:48:11 +0200 Subject: [PATCH 2/4] #24: Updated documentation with delay param --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 072c553..16d20d0 100644 --- a/README.md +++ b/README.md @@ -101,3 +101,8 @@ surpressed if the `silent_lookup_errors` parameter flag is set. Either as named parameter for the `multi_ping` function or when a `MultiPing` object is created. +To avoid burst issues with packet loss on some networks, the `delay` parameter +can be used with the `multi_ping` function or when a `MultiPing` object is +created. This delay in seconds will be applied between every ICMP request. +For milliseconds delay simply use floating number, e.g.: `0.001` for 1 ms. + From 5926a280b8ea8c21f21693c774fcd1fd52572cfe Mon Sep 17 00:00:00 2001 From: Schaffner Brice METAS Date: Thu, 11 Apr 2019 15:49:16 +0200 Subject: [PATCH 3/4] refs #23: fixed multi_ping retry Now the multi_ping retry functionality works correctly. The issue was that an ICMP retry packet was sent with a new ID, but the receive function was still checking packets using old IDs. Now the remainings_id is reinitialized with the new ids during each sends. This way the receive function always used the latest ids for packet matching. The source address of the packet has also been added to the packet matching to make the request/reply match more robust. --- multiping/__init__.py | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/multiping/__init__.py b/multiping/__init__.py index 5f166b4..fc597c0 100644 --- a/multiping/__init__.py +++ b/multiping/__init__.py @@ -231,7 +231,7 @@ def _send_ping(self, dest_addr, payload): # - ICMP code = 0 (unsigned byte) # - checksum = 0 (unsigned short) # - packet id (unsigned short) - # - sequence = 0 (unsigned short) This doesn't have to be 0. + # - sequence = pid (unsigned short) dummy_header = bytearray( struct.pack(_ICMP_HDR_PACK_FORMAT, icmp_echo_request, 0, 0, @@ -292,6 +292,10 @@ def send(self): # need to trim it down. self._last_used_id = int(time.time()) & 0xffff + # Reset the _id_to_addr, we are now retrying to send new request with + # new ID. Reply of a request that have been retried will be ignored. + self._id_to_addr = {} + # Send ICMPecho to all addresses... for addr in all_addrs: # Make a unique ID, wrapping around at 65535. @@ -309,6 +313,9 @@ def send(self): if self._delay > 0: time.sleep(self._delay) + # Keep track of the current request IDs to be used in the receive + self._remaining_ids = list(self._id_to_addr.keys()) + def _read_all_from_socket(self, timeout): """ Read all packets we currently can on the socket. @@ -331,9 +338,9 @@ def _read_all_from_socket(self, timeout): try: self._sock.settimeout(timeout) while True: - p = self._sock.recv(64) + p, src_addr = self._sock.recvfrom(128) # Store the packet and the current time - pkts.append((bytearray(p), time.time())) + pkts.append((src_addr, bytearray(p), time.time())) # Continue the loop to receive any additional packets that # may have arrived at this point. Changing the socket to # non-blocking (by setting the timeout to 0), so that we'll @@ -360,8 +367,8 @@ def _read_all_from_socket(self, timeout): try: self._sock6.settimeout(timeout) while True: - p = self._sock6.recv(128) - pkts.append((bytearray(p), time.time())) + p, src_addr = self._sock6.recvfrom(128) + pkts.append((src_addr, bytearray(p), time.time())) self._sock6.settimeout(0) except socket.timeout: pass @@ -391,15 +398,6 @@ def receive(self, timeout): self._receive_has_been_called = True - # Continue with any remaining IDs for which we hadn't received an - # answer, yet... - if self._remaining_ids is None: - # ... but if we don't have any stored yet, then we are just calling - # receive() for the first time afer a send. We initialize - # the list of expected IDs from all the IDs we created during the - # send(). - self._remaining_ids = list(self._id_to_addr.keys()) - if len(self._remaining_ids) == 0: raise MultiPingError("No responses pending") @@ -412,7 +410,7 @@ def receive(self, timeout): start_time = time.time() pkts = self._read_all_from_socket(remaining_time) - for pkt, resp_receive_time in pkts: + for src_addr, pkt, resp_receive_time in pkts: # Extract the ICMP ID of the response try: @@ -435,7 +433,8 @@ def receive(self, timeout): payload = pkt[_ICMP_PAYLOAD_OFFSET:] if pkt_ident == self.ident and \ - pkt_id in self._remaining_ids: + pkt_id in self._remaining_ids and \ + src_addr[0] == self._id_to_addr[pkt_id]: # The sending timestamp was encoded in the echo request # body and is now returned to us in the response. Note # that network byte order doesn't matter here, since we From 6094a7877fc12695986fd2855b92d2c31a8e5938 Mon Sep 17 00:00:00 2001 From: Schaffner Brice METAS Date: Thu, 11 Apr 2019 15:57:55 +0200 Subject: [PATCH 4/4] refs #23: Added number of retry in result Now the number of retry are added to the result dictionary. --- README.md | 6 +++--- multiping/__init__.py | 6 +++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 16d20d0..1ae2677 100644 --- a/README.md +++ b/README.md @@ -57,13 +57,13 @@ Here is an example of how to use MultiPing in your own code: responses, no_responses = mp.receive(1) The `receive()` function returns a tuple containing a results dictionary -(addresses and response times) as well as a list of addresses that did not +(addresses with response times and retry) as well as a list of addresses that did not respond in time. The results may be processed like this: ... - for addr, rtt in responses.items(): - print "%s responded in %f seconds" % (addr, rtt) + for addr, result in responses.items(): + print "%s responded in %f seconds (retry=%d)" % (addr, result['time'], result['retry']) if no_responses: print "These addresses did not respond: %s" % ", ".join(no_responses) diff --git a/multiping/__init__.py b/multiping/__init__.py index fc597c0..81efa98 100644 --- a/multiping/__init__.py +++ b/multiping/__init__.py @@ -147,6 +147,7 @@ def __init__(self, dest_addrs, sock=None, ignore_lookup_errors=False, self._unprocessed_targets.append(d) self._id_to_addr = {} + self._addr_retry = {} self._remaining_ids = None self._last_used_id = None self._time_stamp_size = struct.calcsize("d") @@ -281,6 +282,7 @@ def send(self): # Collect all the addresses for which we have not seen responses yet. if not self._receive_has_been_called: all_addrs = self._dest_addrs + self._addr_retry = {addr: -1 for addr in all_addrs} else: all_addrs = [a for (i, a) in list(self._id_to_addr.items()) if i in self._remaining_ids] @@ -298,6 +300,7 @@ def send(self): # Send ICMPecho to all addresses... for addr in all_addrs: + self._addr_retry[addr] += 1 # Make a unique ID, wrapping around at 65535. self._last_used_id = (self._last_used_id + 1) & 0xffff # Remember the address for each ID so we can produce meaningful @@ -443,7 +446,8 @@ def receive(self, timeout): req_sent_time = struct.unpack( "d", payload[:self._time_stamp_size])[0] results[self._id_to_addr[pkt_id]] = \ - resp_receive_time - req_sent_time + {'time': resp_receive_time - req_sent_time, + 'retry': self._addr_retry[src_addr[0]]} self._remaining_ids.remove(pkt_id) except IndexError: