SamuelYvon / netifaces-2

netifaces reborn
https://pypi.org/project/netifaces2/
MIT License
21 stars 7 forks source link

Add a way to check if an interface is up #32

Closed relativityspace-jsmith closed 1 month ago

relativityspace-jsmith commented 3 months ago

I realized I need another feature to make the multicast_expert tests work: the ability to check if an interface found by netifaces is currently up.

The reason for this is a bit complex. As I found out, netifaces has an odd behavior where it excludes some of the IPv4 addresses reported by GetAdaptersAddresses on Windows if those interfaces aren't currently up. As I found out using a debugger, this wasn't an intentional behavior, but rather a bug (sorta) caused by the fact that it skips an IPv4 address if it can't find a matching prefix (which it wants to use to compute the subnet mask and broadcast address). The prefix list doesn't seem to get populated for IPv4 if the interface is down, so this causes the address to be skipped.

Now, in netifaces2, we have no such limitation (in fact, we still can't even access the subnet mask on windows because of it being private in the get_adapters_addresses crate). So, netifaces2 is returning all of the IP addresses the OS gives it, which includes static IPs set for down network interfaces.

This actually caused a problem for the multicast_expert tests, because the tests were trying to identify an interface to use and they just picked a random IPv4 address that the machine has. This worked with netifaces-1, but not with netifaces-2, because it ended up trying to open a socket on a down interface.

Oddly, on Linux, we seem to still use the Windows-style behavior, and I do not see IPv4 addrs for down interfaces. I think this is an OS-level thing with the ifaddresses() function though.

To fix the issue, I have added a new function, netifaces.interface_is_up(). This simply returns a boolean representing whether a given interface is up and can pass traffic. Oddly enough, this was pretty straightforward on Windows (though I did take the time to restructure the code for searching interfaces a bit), but actually required a lot of new stuff added for the Unix implementation.

SamuelYvon commented 3 months ago

Hey!

Thanks again for your contribution. I am currently moving all the rust code away from this repository to have a standalone rust version. I am not going to open it up just yet, but if you want to, ping me and I can give you maintainer access.

I am mostly done with the base for the windows part, which does include the interface status (I am boiling it down to up/down however).

https://github.com/SamuelYvon/netifaces-2/discussions/31

SamuelYvon commented 3 months ago

@relativityspace-jsmith Do you have a problem if I cherry-pick some of your code for the linux side for the rust-rewrite? I'll attribute you in the doc comment but your name won't be in the commit history.

relativityspace-jsmith commented 3 months ago

Absolutely! Go ahead! And I'd be interested in being added to the Rust repo as well! btw, this branch is currently not building on mac, and I still need to look into why (should be able to tomorrow). Feel free to take a look too if you have a chance

SamuelYvon commented 3 months ago

Added you, will start pulling in the linux code and finishing the CI probably this weekend; We can use the discussions tab to communicate if you'd like

relativityspace-jsmith commented 3 months ago

OK I pushed an update. Turns out that forum post I was following was sorta incorrect, libc has actually added the struct needed for this ioctl so I can remove a lot of code.

I also fixed another bug in the original Rust code, which is that passing an invalid interface name on Linux to ifaddrs() returned no results without indicating any error (which seems a bit wrong).

SamuelYvon commented 3 months ago

OK I pushed an update. Turns out that forum post I was following was sorta incorrect, libc has actually added the struct needed for this ioctl so I can remove a lot of code.

I think the nix API already has the interface status but per address which is a little odd to me (so it may not be the interface status)

relativityspace-jsmith commented 3 months ago

Oh dang, you're right, I completely missed that. We can use IFF_RUNNING from the interface flags returned by Nix. However, the caveats I added to the readme about the interface flags will still apply.

The man page documentation makes the situation a bit clearer: https://linux.die.net/man/3/getifaddrs . It looks like the interface flags are actually returned once per interface from getifaddrs, so probably nix is duplicating them for each IP address. So it should be OK to use the flags returned there. However, I will say that the method I added with ioctl is a much more direct way to get the flags based on the interface name, so it might make more sense to keep using it.

SamuelYvon commented 3 months ago

If you look at how I'm refactoring into the new repository, we might be able to leverage both; depends on what the caller wants