fix(mdns): send responses from UDP 5353 (GitHub Issue #1)

docs/architecture.mdx

@@ -587,13 +587,17 @@ scanner mints one and persists it.
 ### Sharing UDP 5353
 
 PAIR runs its own mDNS responder rather than depending on a system one, because
-Windows ships none. That responder must coexist with whatever else is on the
-port, including Bonjour, Avahi, and PAIR's own sibling processes. It therefore
-sets `SO_REUSEADDR` on the socket to share UDP 5353.
-
-It deliberately does **not** set `SO_REUSEPORT`. On Linux that load-balances
-incoming unicast datagrams across every socket sharing the port, which would let
-one process swallow mDNS replies meant for another.
+Windows ships none. Its receive socket and short-lived per-interface send
+sockets bind UDP 5353, as RFC 6762 requires for mDNS queries and responses.
+Binding each sender to the selected interface address also preserves reliable
+egress on multi-homed Windows hosts.
+
+Those sockets set `SO_REUSEADDR` so they coexist with Bonjour, Avahi, and other
+PAIR processes. A Darwin sender also sets `SO_REUSEPORT`, matching the BSD
+multicast sharing behavior needed to coexist with the system mDNS responder.
+Linux deliberately omits `SO_REUSEPORT`: there it can load-balance incoming
+unicast datagrams into a short-lived send socket and steal replies from the
+long-lived receiver.
 
 ### Node Enrichment
 

services/readme.md

@@ -55,7 +55,11 @@ This tree builds thirteen Go binaries. `nvpair-ui-broker` is the parent service
 
 Shared code lives in the local `shared/` Go module (imported as `nvpair-shared/…`, replaced via `replace nvpair-shared => ../shared`). It provides logging, wire types, JSON-RPC and IPC, discovery records, mDNS, network monitoring, stable node identity, application data paths, and cluster trust helpers.
 
-The mDNS responder is our own rather than the host's, because Windows ships none. It sets `SO_REUSEADDR` so it shares UDP 5353 with sibling PAIR processes and with a system responder — `avahi-daemon` on Linux, Bonjour where present — needing no configuration on either platform.
+The mDNS responder is our own rather than the host's, because Windows ships
+none. Its receive and per-interface send sockets bind UDP 5353 as RFC 6762
+requires. Socket reuse lets them coexist with sibling PAIR processes and with a
+system responder — `avahi-daemon` on Linux or Bonjour where present — without
+configuration.
 
 The broker feeds every accepted local or peer workload transition plus compact
 GPU telemetry to the scheduler. Queued and running work is counted by destination

services/shared/discovery/discovery.go

@@ -8,10 +8,11 @@
 //
 // The core is a scan-and-diff state machine: each scan browses a service type
 // over grandcat/zeroconf, re-sends the PTR query from a per-interface unicast
-// socket (the Windows send workaround — zeroconf sends from a multicast-bound
-// socket Windows refuses to transmit on), and reconciles the result against the
-// known-node map. Address/TXT comparison is order-insensitive so a multi-homed
-// node whose records come back reordered doesn't churn a spurious "updated".
+// UDP 5353 socket (the Windows send workaround — zeroconf sends from a
+// multicast-bound socket Windows refuses to transmit on), and reconciles the
+// result against the known-node map. Address/TXT comparison is order-insensitive
+// so a multi-homed node whose records come back reordered doesn't churn a
+// spurious "updated".
 //
 // The per-service variations are expressed as functional options rather than
 // forks:
@@ -44,9 +45,10 @@ import (
 	"sync"
 	"time"
 
+	"nvpair-shared/mdns"
+
 	"github.com/grandcat/zeroconf"
 	"github.com/miekg/dns"
-	"golang.org/x/net/ipv4"
 )
 
 // Event types emitted by Run.
@@ -607,8 +609,6 @@ func sendMulticastQuery(service, domain string) map[string]bool {
 		return outcomes
 	}
 
-	target := &net.UDPAddr{IP: net.IPv4(224, 0, 0, 251), Port: 5353}
-
 	ifaces, err := net.Interfaces()
 	if err != nil {
 		slog.Warn("mdns send: enumerate interfaces failed", "err", err)
@@ -630,44 +630,20 @@ func sendMulticastQuery(service, domain string) map[string]bool {
 		if err != nil {
 			continue
 		}
-		var src net.IP
-		for _, a := range addrs {
-			ipnet, ok := a.(*net.IPNet)
-			if !ok {
-				continue
-			}
-			if ip4 := ipnet.IP.To4(); ip4 != nil {
-				src = ip4
-				break
-			}
-		}
+		ifi := ifi
+		src, err := sendMulticastQueryOnInterface(buf, &ifi, addrs, mdns.SendFromInterface)
 		if src == nil {
 			continue
 		}
-
-		ifi := ifi
-		conn, err := net.ListenUDP("udp4", &net.UDPAddr{IP: src, Port: 0})
 		if err != nil {
-			slog.Debug("mdns send: bind failed", "iface", ifi.Name, "ip", src.String(), "err", err)
+			slog.Debug("mdns send: send failed", "iface", ifi.Name, "ip", src.String(), "err", err)
 			outcomes[ifi.Name] = false
-			failures = append(failures, fmt.Sprintf("%s bind: %v", ifi.Name, err))
+			failures = append(failures, fmt.Sprintf("%s send: %v", ifi.Name, err))
 			continue
 		}
-		pc := ipv4.NewPacketConn(conn)
-		if err := pc.SetMulticastInterface(&ifi); err != nil {
-			slog.Debug("mdns send: SetMulticastInterface failed", "iface", ifi.Name, "err", err)
-		}
-		_ = pc.SetMulticastTTL(255)
-		if _, err := conn.WriteToUDP(buf, target); err != nil {
-			slog.Debug("mdns send: write failed", "iface", ifi.Name, "ip", src.String(), "err", err)
-			outcomes[ifi.Name] = false
-			failures = append(failures, fmt.Sprintf("%s write: %v", ifi.Name, err))
-		} else {
-			slog.Debug("mdns send: query sent", "service", service, "iface", ifi.Name, "ip", src.String())
-			outcomes[ifi.Name] = true
-			sent++
-		}
-		_ = conn.Close()
+		slog.Debug("mdns send: query sent", "service", service, "iface", ifi.Name, "ip", src.String())
+		outcomes[ifi.Name] = true
+		sent++
 	}
 
 	if sent == 0 {
@@ -681,6 +657,25 @@ func sendMulticastQuery(service, domain string) map[string]bool {
 	return outcomes
 }
 
+func sendMulticastQueryOnInterface(
+	buf []byte,
+	ifi *net.Interface,
+	addrs []net.Addr,
+	send func([]byte, *net.Interface, net.IP, *net.UDPAddr) error,
+) (net.IP, error) {
+	for _, addr := range addrs {
+		ipnet, ok := addr.(*net.IPNet)
+		if !ok {
+			continue
+		}
+		if ip4 := ipnet.IP.To4(); ip4 != nil {
+			target := &net.UDPAddr{IP: net.IPv4(224, 0, 0, 251), Port: 5353}
+			return ip4, send(buf, ifi, ip4, target)
+		}
+	}
+	return nil, nil
+}
+
 // UUIDFromTXT returns the value of the "uuid=" TXT record, or "" if absent. It's
 // the stable per-host identity carried on the node-scanner daemon's single
 // _nvpair-node record, and was triplicated across the two proxies and the scanner

services/shared/discovery/discovery_test.go

@@ -5,7 +5,9 @@ package discovery
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"net"
 	"sync/atomic"
 	"testing"
 	"time"
@@ -446,6 +448,90 @@ func TestRunEmitsAndCloses(t *testing.T) {
 	}
 }
 
+func TestSendMulticastQueryOnInterfaceUsesFirstIPv4AndMDNSTarget(t *testing.T) {
+	ifi := &net.Interface{Index: 7, Name: "eth0"}
+	wantSource := net.IPv4(192, 0, 2, 10)
+	addrs := []net.Addr{
+		&net.IPNet{IP: net.ParseIP("2001:db8::10")},
+		&net.IPNet{IP: wantSource},
+		&net.IPNet{IP: net.IPv4(198, 51, 100, 20)},
+	}
+	payload := []byte("PTR query")
+
+	var gotPayload []byte
+	var gotInterface *net.Interface
+	var gotSource net.IP
+	var gotTarget *net.UDPAddr
+	source, err := sendMulticastQueryOnInterface(
+		payload,
+		ifi,
+		addrs,
+		func(buf []byte, sentIfi *net.Interface, src net.IP, target *net.UDPAddr) error {
+			gotPayload = append([]byte(nil), buf...)
+			gotInterface = sentIfi
+			gotSource = append(net.IP(nil), src...)
+			gotTarget = target
+			return nil
+		},
+	)
+	if err != nil {
+		t.Fatalf("sendMulticastQueryOnInterface: %v", err)
+	}
+	if !source.Equal(wantSource) || !gotSource.Equal(wantSource) {
+		t.Fatalf("source = %s / sent %s, want %s", source, gotSource, wantSource)
+	}
+	if gotInterface != ifi {
+		t.Errorf("interface = %v, want %v", gotInterface, ifi)
+	}
+	if string(gotPayload) != string(payload) {
+		t.Errorf("payload = %q, want %q", gotPayload, payload)
+	}
+	if gotTarget == nil || !gotTarget.IP.Equal(net.IPv4(224, 0, 0, 251)) || gotTarget.Port != 5353 {
+		t.Errorf("target = %v, want 224.0.0.251:5353", gotTarget)
+	}
+}
+
+func TestSendMulticastQueryOnInterfaceReturnsSenderFailure(t *testing.T) {
+	wantErr := errors.New("send refused")
+	wantSource := net.IPv4(192, 0, 2, 10)
+	source, err := sendMulticastQueryOnInterface(
+		[]byte("PTR query"),
+		&net.Interface{Index: 7, Name: "eth0"},
+		[]net.Addr{&net.IPNet{IP: wantSource}},
+		func([]byte, *net.Interface, net.IP, *net.UDPAddr) error {
+			return wantErr
+		},
+	)
+	if !source.Equal(wantSource) {
+		t.Fatalf("source = %s, want %s", source, wantSource)
+	}
+	if !errors.Is(err, wantErr) {
+		t.Fatalf("error = %v, want %v", err, wantErr)
+	}
+}
+
+func TestSendMulticastQueryOnInterfaceSkipsInterfacesWithoutIPv4(t *testing.T) {
+	called := false
+	source, err := sendMulticastQueryOnInterface(
+		[]byte("PTR query"),
+		&net.Interface{Index: 7, Name: "eth0"},
+		[]net.Addr{&net.IPNet{IP: net.ParseIP("2001:db8::10")}},
+		func([]byte, *net.Interface, net.IP, *net.UDPAddr) error {
+			called = true
+			return nil
+		},
+	)
+	if err != nil {
+		t.Fatalf("sendMulticastQueryOnInterface: %v", err)
+	}
+	if source != nil {
+		t.Fatalf("source = %s, want nil", source)
+	}
+	if called {
+		t.Fatal("sender called without an IPv4 address")
+	}
+}
+
 // TestSendFailuresNeedARunAndClearOnRecovery: this feeds address selection, so a
 // single blip must not move a host's canonical address, and one success must undo
 // the suppression immediately.

services/shared/mdns/responder.go

@@ -16,9 +16,9 @@
 // invisible to LAN peers.
 //
 // We keep zeroconf's receive trick (join the group on each multicast interface,
-// which works fine on Windows) but send every reply/announcement from a
-// per-interface unicast-bound socket with SetMulticastInterface set explicitly.
-// That path is well-supported on Windows.
+// which works fine on Windows) but send every reply/announcement from UDP 5353
+// on a per-interface unicast-bound socket with SetMulticastInterface set
+// explicitly. That path is both RFC-compliant and well-supported on Windows.
 //
 // This is the single implementation consolidated (the mDNS dedup) from the five
 // near-identical copies that lived in nvpair-advertiser,
@@ -48,7 +48,6 @@ import (
 )
 
 const (
-	mdnsPort = 5353
 	// recordTTL matches what zeroconf advertises for non-A records (3200s)
 	// for service-level records, but RFC 6762 §10 says A records SHOULD use
 	// a TTL of 120s to account for IP address changes. We use the shorter
@@ -571,9 +570,8 @@ func (r *Responder) sendUnicast(buf []byte, ifIndex int, to net.Addr) {
 }
 
 // sendOnInterface is the core of the Windows send workaround: it transmits buf
-// from a fresh unicast-bound socket on the given interface (setting the
-// multicast interface + TTL for group targets), never from the multicast-bound
-// receive socket that Windows refuses to send from.
+// from a fresh UDP 5353 socket bound to the given interface address, never from
+// the multicast-bound receive socket that Windows refuses to send from.
 func (r *Responder) sendOnInterface(buf []byte, ifIndex int, target *net.UDPAddr) error {
 	addrs, ok := r.ifaces()[ifIndex]
 	if !ok || len(addrs) == 0 {
@@ -584,19 +582,8 @@ func (r *Responder) sendOnInterface(buf []byte, ifIndex int, target *net.UDPAddr
 	if err != nil {
 		return err
 	}
-	conn, err := net.ListenUDP("udp4", &net.UDPAddr{IP: src, Port: 0})
-	if err != nil {
-		slog.Debug("mdns: bind failed", "iface", ifi.Name, "ip", src.String(), "err", err)
-		return err
-	}
-	defer conn.Close()
-	if target.IP.IsMulticast() {
-		pc := ipv4.NewPacketConn(conn)
-		_ = pc.SetMulticastInterface(ifi)
-		_ = pc.SetMulticastTTL(255)
-	}
-	if _, err := conn.WriteToUDP(buf, target); err != nil {
-		slog.Debug("mdns: write failed", "iface", ifi.Name, "ip", src.String(), "target", target.String(), "err", err)
+	if err := SendFromInterface(buf, ifi, src, target); err != nil {
+		slog.Debug("mdns: send failed", "iface", ifi.Name, "ip", src.String(), "target", target.String(), "err", err)
 		return err
 	}
 	return nil