-
Notifications
You must be signed in to change notification settings - Fork 23
Add support for detecting IPv6 scope IDs, plus some cleanups #97
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
Add support for detecting IPv6 scope IDs, plus some cleanups #97
Conversation
| } | ||
| } | ||
| } | ||
| let interface: Interface = Interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I felt like we could optimize the code by not constructing this until it's actually needed later on.
| if let Some(mac) = mac.clone() { | ||
| iface.mac_addr = Some(mac); | ||
| } | ||
| if let Some(ip) = ip { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be duplicated with the code above here. I tried to use the results from earlier and cut down on duplication.
| - name: Run cargo build | ||
| run: cargo build | ||
|
|
||
| - name: Run cargo test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't look to me like unit tests were actually getting run by the CI! Now they should be.
| run: cross check --target ${{ matrix.target }} | ||
| - name: Cross Compile Check | ||
| run: | | ||
| export CARGO_TARGET_DIR=target/build/${{ matrix.target }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was getting this error when running CI, and this line (or removing the cache) appeared to fix it.
|
Thank you for your contribution! I appreciate the effort you put into adding support for IPv6 scope IDs and the improvements to the code. The purpose of this PR is clear, and I agree with the direction. I won't have enough time for a thorough review today, but I will review it this weekend and proceed with merging. Thanks again for your work! |
Hello! I'm Jamie, I'm starting to work on a set of Python bindings for this library to replace the long-deprecated
netifaceslibrary (that lots of people are still stuck using, including me for my multicast_expert library). I've really liked what I've seen of this library so far, and it seems like a great tool to adapt for the Python world.There is, however, one missing feature that I need to add for my use case. In order to open an IPv6 multicast socket (on Linux at least), I need to know the scope ID of the interface. Scope IDs are a somewhat rarely used feature of IPv6, and honestly I haven't found much info about them other than the actual RFC that defined them. Basically, they are used to uniquely identify a link-local destination address so that the OS knows what interface it's supposed to be sent to.
For instance, suppose an application wants to send a packet to the address
fe80::1. This is a link local address, so it's actually valid on any of the network links the machine has. To resolve that ambiguity, the network address has to be suffixed with a scope ID (also called a zone index), likefe80::1%5. This tells the OS to send the packet out the interface associated with zone index 5. OSs may also support string scope IDs in addresses, likefe80::1%eth0, but this is just syntactic sugar for the same thing.So that applications can make use of scope IDs, this PR adds support for reporting the scope ID for each of the IPv6 addresses that an interface has. Thankfully the scope ID is already reported by getifaddrs and GetAdaptersAddresses, so all I had to do was pass it up the chain to the API. Unfortunately, unlike the corresponding Python class, the Rust Ipv6Addr class does not support storing the scope ID, so I have to make a separate vector for them.
I also took the chance to make some minor cleanups that seemed like they would be helpful, including:
Note: I still have some open questions about scope IDs, including:
Note 2: I'm still new to Rust so hopefully I didn't make a complete hash of this. Please let me know how this code could be improved!