Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ping: Remove binding of ICMP socket #341

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tashanamatl
Copy link

When specifying an interface in the ping command, if trying to ping an
address that is not reachable then no errors would be received. This was
because the ICMP socket was being bound to the source interface but the
errors were coming through the loopback interface.
To fix this the ICMP socket should not be bound to the interface.
Instead IPv4 ping should copy IPv6's use of cmsg. A cmsg header will
contain information about the interface and will then be included in the
msg header when sending a ping. This means the outgoing ping uses the
correct interface but incoming messages from other interfaces can still
be received on the socket.

When specifying an interface in the ping command, if trying to ping an
address that is not reachable then no errors would be received. This was
because the ICMP socket was being bound to the source interface but the
errors were coming through the loopback interface.
To fix this the ICMP socket should not be bound to the interface.
Instead IPv4 ping should copy IPv6's use of cmsg. A cmsg header will
contain information about the interface and will then be included in the
msg header when sending a ping. This means the outgoing ping uses the
correct interface but incoming messages from other interfaces can still
be received on the socket.
@tashanamatl
Copy link
Author

This commit would fix #339

As the ICMP socket is no longer being bound to the interface, it will
never need to be unbound.
@pevik
Copy link
Contributor

pevik commented Feb 25, 2022

This commit don't even compile.

@pevik
Copy link
Contributor

pevik commented Mar 20, 2024

@tashanamatl Does the problem still exists in the latest release and in the current master? If yes, how to reproduce it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants