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

Reachable ip #100

Closed
wants to merge 5 commits into from
Closed

Conversation

PritziElias
Copy link

I also ran into the issue with multiple interfaces (#25). Starting from #26 I tested it using a setup with multiple interfaces and found some additional scenarios, where I would suggest some further improvements.

  • When sending the message, it should be checked, if the address records included in the message can be reached from the destination IP-Address; If the address record can not be reached, the message should not be sent;
    I ran into the problem, that the non-reachable address records were removed from the message, but the message was sent on every available interface; Because of that, services were announced on interfaces where they can not be reached (although the address record was removed);
  • When sending the message, records that are not reachable from the subnet of the sender should be removed, if the desired destination IP is null;
    I ran into this problem when calling Announce(). Since there is no defined destination IP when calling announce, every address record was sent over every interface;
  • When checking for duplicate messages, the address of the remote endpoint should also be included in the check;
    I ran into this problem when I tried to query the same service from two different interfaces. The query is the same, but as the query comes from a different interface it should be answered nonetheless. The new answer will include new address records, as other addresses may be reachable from the interface of the new querier.
  • IsReachable() should return false when the mask is null, because that means no information about the local address could be gathered on the system; the selected local address is probably not present on the system
  • IPv6 link-local addresses should be reachable when their scope id matches; therefore a comparison of the Scope-ID has been added

I included these changes and tested everything on my multiple interface setup. Everything seems to work fine for me, but hopefully, someone can crosscheck. Since I am new to the open-source-community I am glad about feedback.
I appreciate your work and would find it very useful if this PR could be merged. It would simplify the work on multiple interface setups a lot.

Changes added, that ensure that every sent address record is reachable from the desired subnet. It has to be ensured that no listener on one of the subnets gets confused by messages claiming ip addresses, that it can not access.
Reinclude IPv6 address records to the discovery. The reachability of the IPv6 addresses is determined, by comparing the scope id of sender and receiver. IPv4 to IPv6 or vice versa are omitted, because the reachability can not be guaranteed. Testing with multiple IPv4 and IPv6 NICs looks successful.
Check if a connection from IPv4 to IPv6 (or vice versa) is possible, by checking every IP address on a given adapter.

Additionally, some test cases were changed, so that they use the local Loopback address instead of "127.1.1.1".
@PritziElias PritziElias closed this Dec 2, 2020
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.

1 participant