Skip to content

Conversation

@macladson
Copy link
Member

Proposed Changes

This is an alternative to #8659 which just uses if-watch which we already depend on transitively through libp2p.

@macladson macladson requested a review from jxs as a code owner January 15, 2026 01:27
@macladson macladson added ready-for-review The code is ready for review code-quality Networking dependencies Pull requests that update a dependency file labels Jan 15, 2026
@macladson macladson requested a review from ackintosh January 15, 2026 01:28
Copy link
Member Author

@macladson macladson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think this doesn't work.
I think this uses if-addrs under the hood anyway:

https://github.com/libp2p/if-watch/blob/master/src/fallback.rs

pub fn is_ipv6_supported() -> bool {
// If IPv6 is supported
let Ok(std::net::IpAddr::V6(local_ip)) = local_ipv6() else {
let Ok(watcher) = IfWatcher::new() else {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is initialized empty and so we would have to force a resync somehow

@mergify
Copy link

mergify bot commented Jan 15, 2026

Some required checks have failed. Could you please take a look @macladson? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Jan 15, 2026
Copy link
Member

@ackintosh ackintosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @macladson! It seems preferable to use if-addrs directly, since that's what if-watch uses under the hood.

@macladson
Copy link
Member Author

Thanks @macladson! It seems preferable to use if-addrs directly, since that's what if-watch uses under the hood.

I actually misunderstood the feature paths, it only uses if-addrs for Windows, so this might still be worth doing if we can fix the PR

@ackintosh
Copy link
Member

ackintosh commented Jan 16, 2026

Let me clarify: if-watch uses if-addrs on Windows and macOS, but not on Linux. On Linux, if-watch uses rtnetlink instead, despite if-addrs supporting Linux.

I guess the reason is that if-watch wants to receive events for network interface up/down, whereas if-addrs only provides a snapshot.

In our case, we simply want to get a list of network interfaces, so using if-addrs directly seems straightforward.

@jxs
Copy link
Member

jxs commented Jan 17, 2026

Let me clarify: if-watch uses if-addrs on Windows and macOS, but not on Linux. On Linux, if-watch uses rtnetlink instead, despite if-addrs supporting Linux.

I guess the reason is that if-watch wants to receive events for network interface up/down, whereas if-addrs only provides a snapshot.

In our case, we simply want to get a list of network interfaces, so using if-addrs directly seems straightforward.

still, if we can avoid another dependency and just use if-watch as it's transitive would be nice, is it possible?


// Ensure all addresses are added.
loop {
match watcher.poll_if_event(&mut ctx) {
Copy link
Member Author

@macladson macladson Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the code was panicking since tokio::IfWatcher requires a runtime. Adding this either to the function itself or piping in an existing runtime from elsewhere feels way overkill to me.

I have switched to smol::IfWatcher which doesn't need an existing runtime, and I think it technically works now. The downside is, the smol feature added a bunch of extra deps so it totally defeats the purpose of this change.

So with that in mind, I am heavily leaning in favour of the original PR with if-addrs (#8659).

Unless anyone has a cleaner solution that I missed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-quality dependencies Pull requests that update a dependency file Networking waiting-on-author The reviewer has suggested changes and awaits thier implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants