From 3c957e1d05b6598ce679a4b6c94b1e33f0914afb Mon Sep 17 00:00:00 2001 From: Micah Albers Date: Wed, 2 Sep 2020 11:41:41 -0500 Subject: [PATCH 1/7] add a thread safe note comment about seed ID in executor for expert users --- pythonping/executor.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pythonping/executor.py b/pythonping/executor.py index 7bca61c..4668352 100644 --- a/pythonping/executor.py +++ b/pythonping/executor.py @@ -241,6 +241,7 @@ def __init__(self, target, payload_provider, timeout, socket_options=(), seed_id self.timeout = timeout self.responses = ResponseList(verbose=verbose, output=output) self.seed_id = seed_id + # note that to make Communicator instances thread safe, the seed ID must be unique per thread if self.seed_id is None: self.seed_id = os.getpid() & 0xFFFF From e3cf32840e32dddbe9f7e87ae96527f5af43c897 Mon Sep 17 00:00:00 2001 From: Micah Albers Date: Wed, 2 Sep 2020 11:44:46 -0500 Subject: [PATCH 2/7] make network.py thread safe as it was intermittenly failing rarely --- pythonping/network.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/pythonping/network.py b/pythonping/network.py index 0c25e2f..b30b627 100644 --- a/pythonping/network.py +++ b/pythonping/network.py @@ -23,7 +23,14 @@ def __init__(self, destination, protocol, source=None, options=(), buffer_size=2 self.destination = socket.gethostbyname(destination) except socket.gaierror as e: raise RuntimeError('Cannot resolve address "' + destination + '", try verify your DNS or host file') - self.protocol = socket.getprotobyname(protocol) + + # Implementing a version of socket.getprotobyname for this library since built-in is not thread safe + # for python 3.5, 3.6, and 3.7: + # https://bugs.python.org/issue30482 + # This bug was causing failures as it would occasionally return a 0 (incorrect) instead of a 1 (correct) + # for the 'icmp' string, causing a OSError for "Protocol not supported" in multi-threaded usage: + # https://github.com/alessandromaggio/pythonping/issues/40 + self.protocol = Socket.getprotobyname(protocol) self.buffer_size = buffer_size if source is not None: raise NotImplementedError('PythonPing currently does not support specification of source IP') @@ -31,6 +38,12 @@ def __init__(self, destination, protocol, source=None, options=(), buffer_size=2 if options: self.socket.setsockopt(*options) + @staticmethod + def getprotobyname(name): + lookup = {"icmp": socket.IPPROTO_ICMP, "tcp": socket.IPPROTO_TCP, "udp": socket.IPPROTO_UDP, + "ip": socket.IPPROTO_IP, "raw": socket.IPPROTO_RAW} + return lookup[name.lower()] + def send(self, packet): """Sends a raw packet on the stream From 85d6fbbf224a2f2c8ed6617eaa6f03fa6771793a Mon Sep 17 00:00:00 2001 From: Micah Albers Date: Sat, 5 Sep 2020 10:27:02 -0500 Subject: [PATCH 3/7] make a single static class level declaration for network proto dictionary to be more efficient and faster for lookup --- pythonping/network.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/pythonping/network.py b/pythonping/network.py index b30b627..2845fe4 100644 --- a/pythonping/network.py +++ b/pythonping/network.py @@ -5,6 +5,8 @@ class Socket: DONT_FRAGMENT = (socket.SOL_IP, 10, 1) # Option value for raw socket + PROTO_LOOKUP = {"icmp": socket.IPPROTO_ICMP, "tcp": socket.IPPROTO_TCP, "udp": socket.IPPROTO_UDP, + "ip": socket.IPPROTO_IP, "raw": socket.IPPROTO_RAW} def __init__(self, destination, protocol, source=None, options=(), buffer_size=2048): """Creates a network socket to exchange messages @@ -24,12 +26,6 @@ def __init__(self, destination, protocol, source=None, options=(), buffer_size=2 except socket.gaierror as e: raise RuntimeError('Cannot resolve address "' + destination + '", try verify your DNS or host file') - # Implementing a version of socket.getprotobyname for this library since built-in is not thread safe - # for python 3.5, 3.6, and 3.7: - # https://bugs.python.org/issue30482 - # This bug was causing failures as it would occasionally return a 0 (incorrect) instead of a 1 (correct) - # for the 'icmp' string, causing a OSError for "Protocol not supported" in multi-threaded usage: - # https://github.com/alessandromaggio/pythonping/issues/40 self.protocol = Socket.getprotobyname(protocol) self.buffer_size = buffer_size if source is not None: @@ -38,11 +34,19 @@ def __init__(self, destination, protocol, source=None, options=(), buffer_size=2 if options: self.socket.setsockopt(*options) + # Implementing a version of socket.getprotobyname for this library since built-in is not thread safe + # for python 3.5, 3.6, and 3.7: + # https://bugs.python.org/issue30482 + # This bug was causing failures as it would occasionally return a 0 (incorrect) instead of a 1 (correct) + # for the 'icmp' string, causing a OSError for "Protocol not supported" in multi-threaded usage: + # https://github.com/alessandromaggio/pythonping/issues/40 @staticmethod def getprotobyname(name): - lookup = {"icmp": socket.IPPROTO_ICMP, "tcp": socket.IPPROTO_TCP, "udp": socket.IPPROTO_UDP, - "ip": socket.IPPROTO_IP, "raw": socket.IPPROTO_RAW} - return lookup[name.lower()] + try: + return Socket.PROTO_LOOKUP[name.lower()] + except KeyError: + raise KeyError("'" + str(name) + "' is not in the list of supported proto types: " + + str(list(Socket.PROTO_LOOKUP.keys()))) def send(self, packet): """Sends a raw packet on the stream From 56bf53143a1521485bc7119f1319086c86a08fef Mon Sep 17 00:00:00 2001 From: Alessandro Maggio Date: Sat, 12 Sep 2020 15:29:53 +0200 Subject: [PATCH 4/7] refactor: Increase version patch number --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 8345f51..f626f67 100644 --- a/setup.py +++ b/setup.py @@ -4,7 +4,7 @@ long_description = file.read() setup(name='pythonping', - version='1.0.13', + version='1.0.14', description='A simple way to ping in Python', url='https://github.com/alessandromaggio/pythonping', author='Alessandro Maggio', From b7fdd373756b7ef720ae9922267dab23d9095275 Mon Sep 17 00:00:00 2001 From: Sam Gomena Date: Thu, 5 Nov 2020 13:50:41 -0800 Subject: [PATCH 5/7] feat: Add naive calculation for determining packet loss --- pythonping/executor.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pythonping/executor.py b/pythonping/executor.py index 4668352..5490de9 100644 --- a/pythonping/executor.py +++ b/pythonping/executor.py @@ -169,6 +169,10 @@ def success(self, option=SuccessOn.One): result = False not in success_list return result + def packet_loss(self): + success_list = [resp.success for resp in self._responses] + return (success_list.count(False) / len(success_list)) * 100 + @property def rtt_min_ms(self): return represent_seconds_in_ms(self.rtt_min) From dfc6777d3820c2de9550d90fd08dbc92f891268e Mon Sep 17 00:00:00 2001 From: Sam Gomena Date: Fri, 6 Nov 2020 16:38:44 -0800 Subject: [PATCH 6/7] fix: Calculate packet loss as a running average --- pythonping/executor.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pythonping/executor.py b/pythonping/executor.py index 5490de9..d777a62 100644 --- a/pythonping/executor.py +++ b/pythonping/executor.py @@ -148,6 +148,7 @@ def __init__(self, initial_set=[], verbose=False, output=sys.stdout): self.rtt_avg = 0 self.rtt_min = 0 self.rtt_max = 0 + self.packets_lost = 0 for response in initial_set: self.append(response) @@ -169,9 +170,9 @@ def success(self, option=SuccessOn.One): result = False not in success_list return result + @property def packet_loss(self): - success_list = [resp.success for resp in self._responses] - return (success_list.count(False) / len(success_list)) * 100 + return self.packets_lost @property def rtt_min_ms(self): @@ -201,6 +202,9 @@ def append(self, value): self.rtt_max = value.time_elapsed if value.time_elapsed < self.rtt_min: self.rtt_min = value.time_elapsed + + self.packets_lost = self.packets_lost + (0 if value.success else 1 - self.packets_lost) / len(self) + if self.verbose: print(value, file=self.output) From b46bcefbc4319b84f58ce76404a03b29f0a50b65 Mon Sep 17 00:00:00 2001 From: Sam Gomena Date: Sun, 8 Nov 2020 13:24:44 -0800 Subject: [PATCH 7/7] feat: Add tests for packet loss --- test/test_executor.py | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/test_executor.py b/test/test_executor.py index 1fb4364..46970ad 100644 --- a/test/test_executor.py +++ b/test/test_executor.py @@ -247,6 +247,47 @@ def test_success_half_success(self): 'Unable to calculate success on all with half responses successful' ) + def test_no_packets_lost(self): + rs = executor.ResponseList([ + SuccessfulResponseMock(None, 1), + SuccessfulResponseMock(None, 1), + SuccessfulResponseMock(None, 1), + SuccessfulResponseMock(None, 1) + ]) + + self.assertEqual( + rs.packet_loss, + 0.0, + "Unable to calculate packet loss correctly when all resposes successful" + ) + + def test_all_packets_lost(self): + rs = executor.ResponseList([ + FailingResponseMock(None, 1), + FailingResponseMock(None, 1), + FailingResponseMock(None, 1), + FailingResponseMock(None, 1) + ]) + + self.assertEqual( + rs.packet_loss, + 1.0, + "Unable to calculate packet loss correctly when all resposes failed" + ) + + def test_some_packets_lost(self): + rs = executor.ResponseList([ + SuccessfulResponseMock(None, 1), + SuccessfulResponseMock(None, 1), + FailingResponseMock(None, 1), + FailingResponseMock(None, 1) + ]) + + self.assertEqual( + rs.packet_loss, + 0.5, + "Unable to calculate packet loss correctly when some of the responses failed" + ) class CommunicatorTestCase(unittest.TestCase): """Tests for Communicator"""