I'm creating my own little toolbox for pinging IP interfaces (for the test, only in localhost). In the future I'm thinking of piping the output to a text file. But I'd like to focus on improving the performance for now.
For now, scanning a class A IP range is pretty fast, but for a B range (65025 addresses) it's very long. Taking in excess of 5 minutes. Here is my code:
#!/usr/bin/python3
import threading
import os
# Define a function for the thread
def pingBox( threadName, host):
response = os.system("ping -c 1 "+host)
print('response for pinging %s is %s' %(threadName, response))
if response == 0:
print ('%s : is up' %host)
else:
print ('%s : is down' %host)
# Create one thread by ping
def createThread(host):
try:
tlist = []
tlist.append(
threading.Thread(
target=pingBox,
kwargs={
"threadName": "Thread:{0}".format(host),
"host":host
}
))
for t in tlist:
t.start()
for t in tlist:
t.join()
except Exception as e:
print(e)
def rangeSelector(ipRange):
if ipRange== 'A' or ipRange== 'a':
for i in range (0,255):
createThread('127.0.0.'+str(i))
elif ipRange== 'B' or ipRange== 'b':
for i in range (0,255):
for j in range(0,255):
createThread('127.0.'+str(i)+'.'+str(j))
else:
pass
if __name__ == "__main__":
ipRange=input('Choose range A|B :')
rangeSelector(ipRange)
2 Answers 2
Towards better design, functionality and performance
Start with good names: that means following Python naming conventions and give a meaningful names to your identifiers/functions/classes:
rangeSelector
---> ping_network_range
createThread
---> start_threads
pingBox
---> check_host
As @Peilonrayz already mentioned range(256)
is the valid range for your case.
The former createThread
function tried to create a single thread with start
ing and join
ing it at one step. But that undermines the benefit of using threading for parallelizing computations. The crucial mechanics is that all treads are initiated/started at once, then, we join
them (awaiting for their finishing) at next separate phase.
On constructing a threading.Thread
no need to pass custom thread name through kwargs={"threadName": ...}
- the Thread
constructor already accepts name
argument for the thread name which is then accessed within target function via threading.current_thread().name
.
The former pingBox
function used os.system
function (executes the command (a string) in a subshell) which is definitely not a good choice.
The more robust, powerful and flexible choice is subprocess
module:
The
subprocess
module allows you to spawn new processes, connect to their input/output/error pipes, and obtain their return codes. This module intends to replace several older modules and functions:os.system
,os.spawn*
...
The recommended approach to invoking subprocesses is to use therun()
function for all use cases it can handle. For more advanced use cases, the underlyingPopen
interface can be used directly.
Some good, user-friendly description - on this SO link.
Furthermore, the ping
command (OS network tool) can be speed up itself through adjusting specific options. The most significant ones in such context are:
-n
(No attempt will be made to lookup symbolic names for host addresses. Allows to disable DNS lookup to speed up queries)-W <number>
(Time to wait for a response, in seconds. The option affects only timeout in absence of any responses, otherwise ping waits for two RTTs)-i interval
(Waitinterval
seconds between sending each packet. The default is to wait for one second between each packet normally, or not to wait in flood mode. Only super-user may set interval to values less than 0.2 seconds)
The difference would be more noticeable on sending more than one packet (-c
option).
In your case I'd apply -n
and -q
options. With -q
option allows to quite the output since we'll get the returned code which itself indicates whether a particular host is reachable across an IP network. subprocess.run
allows to access the returncode
explicitly.
The 2 nested for
loops within the former rangeSelector
function is flexibly replaced with itertools.product
routine to compose an efficient generator expression yielding formatted IP addresses:
from itertools import product
...
start_threads(f'127.0.{i}.{j}' for i, j in product(range(256), range(256)))
Designing command-line interface
As your program is planned to be used as part of pipeline, instead of hanging on interactive, blocking call of input('Choose range A|B :')
- the more flexible and powerful way is using argparse
module that allows building an extended and flexible command-line interfaces with variety of options of different types and actions.
For ex. the allowed network classes names can be supplied through choices
:
...
parser = ArgumentParser(description='Ping network addresses by network class')
parser.add_argument('-c', '--nclass', choices=('A', 'B'), required=True, help='Choose class A or B')
The final optimized implementation:
import threading
from argparse import ArgumentParser
from itertools import product
import subprocess
def check_host(host: str):
return_code = subprocess.run(["ping", "-c", "1", "-n", "-q", host],
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL).returncode
print(f'response for pinging {threading.current_thread().name} is {return_code}')
status = 'up' if return_code == 0 else 'down'
print(f'{host} : is {status}')
def start_threads(addr_range):
for addr in addr_range:
t = threading.Thread(target=check_host, args=(addr,),
name=f'Thread:{addr}')
t.start()
yield t
def ping_network_range(net_class: str):
net_class = net_class.upper()
if net_class == 'A':
threads = list(start_threads(f'127.0.0.{i}' for i in range(256)))
elif net_class == 'B':
threads = list(start_threads(f'127.0.{i}.{j}'
for i, j in product(range(256), range(256))))
else:
raise ValueError(f'Wrong network class name {net_class}')
for t in threads:
t.join()
if __name__ == "__main__":
parser = ArgumentParser(description='Ping network addresses by network class')
parser.add_argument('-c', '--nclass', choices=('A', 'B'),
required=True, help='Choose class A or B')
args = parser.parse_args()
ping_network_range(args.nclass)
Sample usage (with timing, under Unix time
command):
time python3 ping_network_range.py -c B > test.txt
real 2m4,165s
user 2m17,660s
sys 4m35,790s
Sample tail contents of resulting test.txt
file:
$ tail test.txt
response for pinging Thread:127.0.255.250 is 0
127.0.255.250 : is up
response for pinging Thread:127.0.255.252 is 0
127.0.255.252 : is up
response for pinging Thread:127.0.255.253 is 0
127.0.255.253 : is up
response for pinging Thread:127.0.255.255 is 0
127.0.255.255 : is up
response for pinging Thread:127.0.255.254 is 0
127.0.255.254 : is up
-
1\$\begingroup\$ Nice answer, contains a lot that mine doesn't. \$\endgroup\$2019年12月18日 18:25:37 +00:00Commented Dec 18, 2019 at 18:25
- You should run a linter on your program, you have quite a few PEP 8 issues. A lot are from non-standard whitespace usage, and using
cammelCase
rather thansnake_case
. Conforming to one standard - PEP 8 - allows others to more easily address your code. This is as all code looks the same and so can be easily read. - At one time
%
formatting was deprecated in the Python docs. Given that f-strings are available in Python now, I would recommend converting to eitherstr.format
or f-strings to increase readability with modern Python best practices. - You can change
pingBox
to use a ternary to DRY your code. You should be able to see that in
createThread
tlist
is only going to have 1 item. On creation you're going to start the thread and then wait for it to finish running.The problem is that you're
Thread.join
ing before you've started all the other threads. This is simple to fix, you just need to build the threads before joining them.You can use
str.lower
to simplify yourrangeSelector
if statements.- If your range starts at 0, then you don't need to specify 0.
- I think you have a bug, 255 is a valid address. You currently are ignoring it tho. You need to specify 256 in the
range
. - You can use a generator comprehension to make all the IPs you want, that you need to pass to
create_threads
.
#!/usr/bin/python3
import threading
import os
def ping_box(thread_name, host):
response = os.system("ping -c 1 " + host)
print(f'Response for pinging {thread_name} is {response}')
status = 'down' if response else 'up'
print(f'{host}: is {stetus}')
def create_threads(ips):
for ip in ips:
thread = threading.Thread(
target=ping_box,
kwargs={
"thread_name": f"Thread:{ip}",
"host": ip
}
)
thread.start()
yield thread
def range_selector(ip_range):
if ip_range.lower() == 'a':
ips = (
f'127.0.0.{i}'
for i in range(256)
)
elif ip_range.lower() == 'b':
ips = (
f'127.0.{i}.{j}'
for i in range(256)
for j in range(256)
)
else:
ips = ()
for thread in list(create_threads(ips)):
thread.join()
if __name__ == "__main__":
range_selector(input('Choose range A|B :'))
Please note the list
around create_threads
. This is to avoid the laziness of generator functions, which is not something we want.
-
\$\begingroup\$ I'm trying to understand your code, but it seems there is an error :
Exception in thread Thread-2361: Traceback (most recent call last): File "/usr/lib/python3.6/threading.py", line 916, in _bootstrap_inner self.run() File "/usr/lib/python3.6/threading.py", line 864, in run self._target(*self._args, **self._kwargs) TypeError: ping_box() got an unexpected keyword argument 'threadName'
\$\endgroup\$Warok– Warok2019年12月18日 15:15:39 +00:00Commented Dec 18, 2019 at 15:15 -
\$\begingroup\$ @Warok Oh yeah there's an error there. You should be able to figure it out. Note,
threadName
vs.thread_name
. \$\endgroup\$2019年12月18日 15:40:14 +00:00Commented Dec 18, 2019 at 15:40 -
\$\begingroup\$ I knew you did it on purpose ;) Thanks for the help and the improvements \$\endgroup\$Warok– Warok2019年12月18日 15:42:06 +00:00Commented Dec 18, 2019 at 15:42
-
\$\begingroup\$ @Warok Nope, just a small typo. :( No problem, :) \$\endgroup\$2019年12月18日 16:12:15 +00:00Commented Dec 18, 2019 at 16:12
Explore related questions
See similar questions with these tags.
127.0.0.0/24
is assigned to localhost addresses (which respond very fast to ping on normal machines), but the rest of127.0.0.0/16
is not (and likely won't respond at all), don't you? \$\endgroup\$