fix: record an interface whose multicast join failed, ending an endless retry

[vdavid]

Interfaces that share an IPv4 address no longer retry the multicast join forever, or drop each other's membership.

Problem: IPv4 join, leave, and send identify an interface by address, but my_intfs keys interfaces by index. On macOS, OrbStack/Docker/UTM bridges (bridge1NN) end up with an address the socket already joined, so the join fails with AddrInUse, the interface never lands in my_intfs, and every IP check retries and logs again (every 5 s, for as long as the bridge lives).

Fix (based on @keepsimple1's draft):

• AddrInUse on an IPv4 join counts as success: the socket already holds that membership. Other join errors still retry like before.
• No leave for an address another interface still has (leaving by address would drop that interface's membership too).
• After removals, rejoin any address still in use, in case the removed interface held the shared membership (no-op otherwise).
• No IpDel / addr_auto cleanup for an address that's still on another interface.

Tests: two new ones add a second interface on 127.0.0.1, so no host setup needed. Both fail without the fix.

[keepsimple1]

Thank you for the PR! I think the root cause is that my_intfs keys interfaces by index, but the IPv4 join, leave and send-interface calls all identify the interface by address.

Inline comment with more details. I'll also try to see how we could fix the root cause.

[keepsimple1]

Recording the interface after a failed join fixes the loop, but it seems it could cause problems:

• Teardown drops the interface's membership as well on macOS since the membership is keyed with address.
• Every join error now stops retrying, even if it's a temp failure.
• Duplicate sends on two interfaces with the same address.

[keepsimple1]

Btw, another option is to simply change debug! to trace! level so that hopefully it won't show up in your logs if you'd like a stop-gap first. (While we work on a longer term fix)

[vdavid]

Thanks for the fast review, @keepsimple1! Right, I agree the address vs. index mismatch is the real root cause and that this is not the right fix.

Personally for me, no need for the temporary trace! change: I use my fork for now where this is fixed.

I have two ideas for the ways forward:

1. You do the root-cause fix, and I close this PR. Happy to test it on my Docker/UTM setup (I have the bridge1NN + vmenetN pairs that trigger it).
2. I rework this PR. A smaller change that handles your 3 points: when a join fails with AddrInUse, record the interface as an alias of the one that already holds that address. Aliases don't send, the membership is ref-counted per address so a teardown only leaves when the last user goes, and every other join error keeps retrying like today. Joining by index would be cleaner, but it's a bigger change.

Let me know your preference and how I can help.

[keepsimple1]

Let me know your preference and how I can help.

What you said make sense. I will choose option 1 , i.e. will do a root-cause fix (would need a couple of days), and will send over your way for review and verification once I have a PR. Thank you in advance for your help and working together on this!

[vdavid]

Sounds good, looking forward to it!

[keepsimple1]

Looks like, for option 1, we would need to upstream changes to join_multicast_v4 to use if_index (like IPv6) in socket-pktinfo. That is more involved and would assume we could get it merged quickly. And it also would deepen external dependencies, which I normally try to avoid, even for one we already depend on.

Looking at it again, your Option 2 could be a better option overall. I ran by Claude to have a draft diff attached below. One difference is: "aliases don't send" assumes packets arrive with the owner's interface index": With IPv4 joins keyed by address, the kernel attaches the membership to whichever interface it finds first for that address. That isn't necessarily the one mdns-sd joined first. So in this draft diff, the alias would stay in my_intfs for receiving and answering. It means we could send out duplicated packets from different interfaces, but probably okay. If you got a better approach, feel free to propose.

I don't want to post in another PR because it's based on your idea. Do you mind go ahead with your PR for option 2, and update the fix?

diff --git a/src/service_daemon.rs b/src/service_daemon.rs
index 5e50bcf..a2910c9 100644
--- a/src/service_daemon.rs
+++ b/src/service_daemon.rs
@@ -1215,6 +1215,22 @@ struct Zeroconf {
     test_down_interfaces: HashSet<String>,
 }

+/// Joins the IPv4 mDNS group on the interface that has `ip`.
+///
+/// The kernel picks the interface by address, so interfaces sharing an address
+/// (e.g. the `bridge1NN` and `vmenetN` pairs of VMs on macOS) share one membership.
+/// Joining it again fails with `AddrInUse`, which only means this socket already
+/// holds it, so that is treated as success.
+fn join_multicast_v4(my_sock: &PktInfoUdpSocket, ip: &Ipv4Addr) -> io::Result<()> {
+    match my_sock.join_multicast_v4(&GROUP_ADDR_V4, ip) {
+        Err(e) if e.kind() == io::ErrorKind::AddrInUse => {
+            debug!("multicast group V4 already joined on addr {ip}");
+            Ok(())
+        }
+        result => result,
+    }
+}
+
 /// Join the multicast group for the given interface.
 fn join_multicast_group(my_sock: &PktInfoUdpSocket, intf: &Interface) -> Result<()> {
     let intf_ip = &intf.ip();
@@ -1222,8 +1238,7 @@ fn join_multicast_group(my_sock: &PktInfoUdpSocket, intf: &Interface) -> Result<
         IpAddr::V4(ip) => {
             // Join mDNS group to receive packets.
             debug!("join multicast group V4 on {} addr {ip}", intf.name);
-            my_sock
-                .join_multicast_v4(&GROUP_ADDR_V4, ip)
+            join_multicast_v4(my_sock, ip)
                 .map_err(|e| e_fmt!("PKT join multicast group on addr {}: {}", intf_ip, e))?;
         }
         IpAddr::V6(ip) => {
@@ -1970,18 +1985,17 @@ impl Zeroconf {
             );
         }

-        for ip in deleted_ips {
-            self.del_ip(ip);
-        }
-
         for (if_index, last_ipv4, last_ipv6) in deleted_intfs {
             let Some(my_intf) = self.my_intfs.remove(&if_index) else {
                 continue;
             };

             if let Some(ipv4) = last_ipv4 {
-                debug!("leave multicast for {ipv4}");
-                if let Some(sock) = self.ipv4_sock.as_mut() {
+                if self.intfs_with_ip(&IpAddr::V4(ipv4)).next().is_some() {
+                    // Leaving by address would drop the membership the other interface uses.
+                    debug!("keep multicast for {ipv4}: still used by another interface");
+                } else if let Some(sock) = self.ipv4_sock.as_mut() {
+                    debug!("leave multicast for {ipv4}");
                     if let Err(e) = sock.pktinfo.leave_multicast_v4(&GROUP_ADDR_V4, &ipv4) {
                         debug!("leave multicast group for addr {ipv4}: {e}");
                     }
@@ -2010,6 +2024,26 @@ impl Zeroconf {
             self.resolve_updated_instances(&result.modified_instances);
         }

+        // Handled after the removals above, so that `my_intfs` shows which of
+        // these addresses are still on another interface.
+        deleted_ips.sort();
+        deleted_ips.dedup();
+        for ip in deleted_ips {
+            if self.intfs_with_ip(&ip).next().is_none() {
+                self.del_ip(ip);
+                continue;
+            }
+
+            // The address is still on another interface. The removed interface may
+            // have been the one holding the shared IPv4 membership, which the kernel
+            // drops with it, so join again (a no-op if the membership survived).
+            if let (IpAddr::V4(ipv4), Some(sock)) = (ip, self.ipv4_sock.as_ref()) {
+                if let Err(e) = join_multicast_v4(&sock.pktinfo, &ipv4) {
+                    debug!("check_ip_changes: rejoin multicast group on addr {ipv4}: {e}");
+                }
+            }
+        }
+
         // Add newly found interfaces only if in our selections.
         self.apply_intf_selections(my_ifaddrs);
     }
@@ -2024,6 +2058,10 @@ impl Zeroconf {
             intf.ip()
         );

+        // Another interface with the same address still serves it, and shares its
+        // IPv4 membership, e.g. the `bridge1NN` and `vmenetN` pairs of VMs on macOS.
+        let shared = self.intfs_with_ip(&intf.ip()).any(|i| i != if_index);
+
         let Some(my_intf) = self.my_intfs.get_mut(&if_index) else {
             debug!("del_interface_addr: interface {} not found", intf.name);
             return;
@@ -2036,7 +2074,9 @@ impl Zeroconf {

             match intf.addr.ip() {
                 IpAddr::V4(ipv4) => {
-                    if my_intf.next_ifaddr_v4().is_none() {
+                    if shared {
+                        debug!("keep multicast for {ipv4}: still used by another interface");
+                    } else if my_intf.next_ifaddr_v4().is_none() {
                         if let Some(sock) = self.ipv4_sock.as_mut() {
                             if let Err(e) = sock.pktinfo.leave_multicast_v4(&GROUP_ADDR_V4, &ipv4) {
                                 debug!("leave multicast group for addr {ipv4}: {e}");
@@ -2084,7 +2124,7 @@ impl Zeroconf {
             }
         }

-        if ip_removed {
+        if ip_removed && !shared {
             // Notify the monitors.
             self.notify_monitors(DaemonEvent::IpDel(intf.ip()));
             // Remove the interface from my services that enabled `addr_auto`.
@@ -2092,6 +2132,14 @@ impl Zeroconf {
         }
     }

+    /// Indexes of the interfaces in `my_intfs` that have `ip`.
+    fn intfs_with_ip<'a>(&'a self, ip: &'a IpAddr) -> impl Iterator<Item = u32> + 'a {
+        self.my_intfs
+            .values()
+            .filter(move |my_intf| my_intf.addrs.iter().any(|addr| addr.ip() == *ip))
+            .map(|my_intf| my_intf.index)
+    }
+
     /// Adds the address of `intf` and brings our services up on it.
     ///
     /// Note: this `interface` type only contains one address.
@@ -5438,6 +5486,141 @@ mod tests {
         })
     }

[vdavid]

Thanks! Sounds good, I'm on it!

[vdavid]

Thanks for the draft, it helped a lot!
I've pushed it with two tests (both red without the fix, green with it, on 1.71 and stable).

I also ran it on my Mac with OrbStack, churning Docker networks to recreate bridges, and experienced this:

• Before: 148 join retries in ~4 minutes.
• After: 0 retries, already joined instead, and discovery kept finding my NAS the whole time (~50 min in my app, 4 churn cycles).

One thing I learned, FYI: on my machine, the trigger wasn't two live interfaces sharing an address. It was bridges coming and going. Even a freshly started daemon got AddrInUse on its first join to a new bridge, and IPv6 joins (by index!) hit it too. The IPv6 failure only logs once, so no loop there. I couldn't verify whether the socket actually receives on such a bridge after already joined (netstat -g is system-wide, and mDNSResponder joins everything). It doesn't matter for my use case, but it might for someone's.

Can you re-review this?

[vdavid]

Sorry for the red Windows run! Windows reports a repeated join as WSAEINVAL instead of AddrInUse. I skipped the two new tests there, CI is green now.

[keepsimple1]

Thank you so much! I will review the diff this weekend.

[keepsimple1]

Thanks! I think this is a reasonable change that can address the issue at hand.