Skip to content

Commit

Permalink
refs romana#23: fixed multi_ping retry
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Schaffner Brice METAS committed Apr 11, 2019
1 parent c6e4a29 commit 5926a28
Showing 1 changed file with 15 additions and 16 deletions.
31 changes: 15 additions & 16 deletions multiping/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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")

Expand All @@ -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:
Expand All @@ -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
Expand Down

0 comments on commit 5926a28

Please sign in to comment.