ValentinBELYN / icmplib

Easily forge ICMP packets and make your own ping and traceroute.
GNU Lesser General Public License v3.0
266 stars 46 forks source link

The regular time.time() function does not work well in Windows, rtt = 0 #61

Open skychel opened 1 year ago

skychel commented 1 year ago

Hi Valentine, thanks for the great library. I use it with pleasure.

And I think I found a small problem. When using the library in Windows OS, if the rtt is less than 5-10 ms, the regular time.time()function does not cope with the task and round-trip times (rtt)is displayed as zero.

Such short delays are typical for a local network and ping, for example, the nearest default gateways. I checked the observation in Win10 Pro 22H2 (Python 3.9-11) and Win7 Pro (Python 3.8). And the problem was consistently reproduced.

It looks like this:

"C:\Program Files\Python38\python.exe" C:/Users/Admin/PycharmProjects/ICMPLIB/verbose_ping.py
PING 10.117.3.1: 56 data bytes

  18 bytes from 10.117.3.1: icmp_seq=0 time=0 ms
  18 bytes from 10.117.3.1: icmp_seq=1 time=0 ms
  18 bytes from 10.117.3.1: icmp_seq=2 time=0 ms
  18 bytes from 10.117.3.1: icmp_seq=3 time=0 ms
  18 bytes from 10.117.3.1: icmp_seq=4 time=0 ms

Completed.

Process finished with exit code 0

In principle, it is known that time.time() does not work well in Windows (there are not enough decimal places), so on other icmp implementations I used time.clock()before instead of time.time(). But, it seems to me, since some time, the function time.clock has been removed, after having been deprecated since Python 3.3 or 3.4.

It seems to me that the way out of this situation is to use the time.perf_counter() function in Windows. It has a good resolution and works reliably.

In fact, we need to check that we are in Windows and in the module sockets.py use perf_counter() instead of time().

I tweaked your code a bit for verification and it really helped.

"C:\Program Files\Python38\python.exe" C:/Users/Admin/PycharmProjects/ICMPLIB/verbose_ping.py
PING 10.117.3.1: 56 data bytes

  18 bytes from 10.117.3.1: icmp_seq=0 time=1.111 ms
  18 bytes from 10.117.3.1: icmp_seq=1 time=1.08 ms
  18 bytes from 10.117.3.1: icmp_seq=2 time=0.932 ms
  18 bytes from 10.117.3.1: icmp_seq=3 time=0.919 ms
  18 bytes from 10.117.3.1: icmp_seq=4 time=1.939 ms

Completed.

Process finished with exit code 0
ValentinBELYN commented 1 year ago

Hi @skychel,

Thank you for your detailed answer! It is indeed a good idea to use time.perf_counter() instead of time.time() on Windows, maybe even on other OS as well.

That being said, since this will change the return value of the models time property, it will take a few releases before this is fixed.

TerjeMjelde commented 11 months ago

Implementing this should be easy. I experienced the same problem, and just replaced every call to time() with a call to perf_counter() and it works perfectly. Also, there's no need to differentiate between Linux and Windows here. I am running this modification on both systems without any problems-