Prevent incorrect results when multiple pings are running at the same time

[bdraco]

Solves https://github.com/home-assistant/core/issues/39827

Since the socket would listen for any icmp traffic coming back, multiple pings that ran at the same time would receive responses from hosts that were unexpected so it would appear hosts were online when they were actually down.

[ValentinBELYN]

Hi @bdraco,

Thank you for contributing to the development of icmplib. I understand the problem but this is the normal behavior of the library.

As you say, ICMP sockets even pick up packets that are not intended for them. Consequently, this can skew the results. There are several solutions to avoid this and yours is correct.

When I developed the library, I wanted to develop a universal solution, valid for the ping function as well as for the multiping and traceroute functions, while respecting the RFCs. The library is therefore based on the identifier of the packets and their sequence number to differentiate two distinct "instances" of ping.

When you call the ping function, the identifier corresponds to the PID of your machine by default and therefore, if you call it several times in parallel, the only way to identify the packets will be based on their sequence number which is insufficient (because identifiers are the same). The solution is to use a unique identifier each time the ping function is called.

ping('1.1.1.1', id=get_unique_id())

This same principle is used by the ping command of Linux operating systems (it seems to me that Windows does it differently). The multiping function is also based on this mechanism and increments the identifier by 1 for each host to ping (https://github.com/ValentinBELYN/icmplib/blob/master/icmplib/ping.py#L223).

My mistake was maybe to define a fixed identifier by default for the ping function. But it is suitable for the majority of people and uses less resources.

I hope these explanations could help you.

[ValentinBELYN]

Here is an example of a function (thread-safe) allowing you to obtain a unique identifier:

from threading import Lock

lock = Lock
current_id = 0

def get_unique_id():
    global current_id

    with lock:
        current_id = (current_id + 1) & 0xffff

    return current_id

Each time you want to ping a host, call it as follows:

ping('1.1.1.1', id=get_unique_id())

This function will certainly be integrated in the next versions of icmplib.

[bdraco]

@ValentinBELYN Fantastic response! Thank you for the detailed explanation. Apologizes for assuming this was a bug 🙈

It looks like we get and unsigned short the id and then need to reset it once we reach 65535

[bdraco]

Closing this as the error is on my part.

[ValentinBELYN]

Don't worry! 😄 It's really cool to participate in this project.

Yes, since the ICMP packet ID field is 16-bit encoded (unsigned short), the limit is 65535 which is normally sufficient unless you have 65535 simultaneous pings! However, you don't necessarily have to reset the counter when it reaches 65535. icmplib does it automatically. I just added & 0xffff to avoid using unnecessary memory by handling too large numbers for the counter.

Don't hesitate if you have other questions in the future 😃