feat(mdns): advertise _jetkvm._tcp via Bonjour/DNS-SD#1454
Draft
mcuelenaere wants to merge 1 commit into
Draft
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
mcuelenaere
added a commit
to mcuelenaere/kvm
that referenced
this pull request
May 9, 2026
Cursorbot review on jetkvm#1454 flagged two issues: 1. hashicorp/mdns.NewServer always binds both 224.0.0.251:5353 and [ff02::fb]:5353 with no toggle, so MDNSMode=ipv4_only/ipv6_only was silently ignored — clients on the disabled family could still browse and discover a service whose A/AAAA they cannot resolve. Replace the call with a small custom responder that only binds the requested transports. Record generation still uses hashicorp/mdns.MDNSService via the existing jetkvmZone, and the query/response loop mirrors hashicorp/mdns to keep on-the-wire behavior identical otherwise. 2. DefaultAddressIPv4 / DefaultAddressIPv6 were leftover unused exports from the pion era. Removed. Also corrected misleading comments that claimed Stop() emits DNS-SD goodbye packets — neither hashicorp/mdns nor this responder do; we rely on TTL expiry like before. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 846b975. Configure here.
mcuelenaere
added a commit
to mcuelenaere/kvm
that referenced
this pull request
May 11, 2026
Cursorbot caught a real race I missed: SetOptions was acquiring the lock, updating fields, releasing the lock, then calling Restart() which re-acquires the same lock inside start(true). Between the unlock and the re-lock, a concurrent Stop() (e.g. SIGINT during a networkStateChanged) could shut the server down and set m.server = nil, after which the pending Restart() would build a fresh server — silently undoing the Stop(). Extract startLocked() as the body of start() assuming the caller already holds m.lock, and have SetOptions hold the lock continuously across the field update and the restart. Public Start() / Restart() still go through start() which acquires the lock as before. Cursor: jetkvm#1454 (comment) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
|
I'm moving this PR back to draft: an easier way forward should be to wait for pion/mdns#277 to get merged and then just build on top of that |
JetKVM devices now advertise themselves as the DNS-SD service type `_jetkvm._tcp` on the local network, so macOS/iOS clients can discover every device on the LAN via NWBrowser(for: .bonjour(type: "_jetkvm._tcp", domain: nil)), and `dns-sd -B _jetkvm._tcp` / `avahi-browse -r _jetkvm._tcp` work too. The advertised record carries: - instance: the device hostname (e.g. jetkvm-abc123) - host: <hostname>.local (existing A/AAAA resolution preserved) - port: 80, or 443 when TLS is enabled - TXT: version=<fw>, id=<deviceId>, setup=<true|false> Implementation uses pion/mdns/v2 (already in the tree transitively via pion/webrtc). pion takes the IPv4 and IPv6 multicast packet conns separately, so the existing MDNSMode=ipv4_only / ipv6_only config is honored by simply not binding the disabled family — no custom responder, no second mDNS library. We switch from the legacy Server() (A/AAAA only) to NewServer() so PTR/SRV/TXT are answered too. Lifecycle: - starts on the first networkStateChanged once the network is up - refreshes after device setup completes so the `setup` TXT flips - refreshes after a TLS mode change so the advertised port follows - Stop() closes the sockets on shutdown NOTE: DNS-SD TXT publication needs an exported TXTEntry API that is not yet in a tagged pion/mdns release; go.mod pins pion/mdns/v2 to a fork branch via `replace` until pion/mdns#277 merges and ships. This PR stays in draft until then. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mcuelenaere
force-pushed
the
claude/keen-hodgkin-0657ba
branch
from
7c807df to
a57e2e4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
JetKVM devices now advertise themselves on the local network as the DNS-SD service type
_jetkvm._tcp, so Bonjour browsers can discover every device on a LAN. Today the responder only answers A/AAAA forjetkvm.local, so PTR queries for the service type return nothing.Important
Draft — blocked on pion/mdns#277.
pion/mdns gained DNS-SD service advertising, but
ServiceInstance.Textused an unexported element type, so callers can't set TXT records. #277 exportsTXTEntry+NewTXTEntry/NewTXTFlag. Until it merges and ships,go.modpinspion/mdns/v2to a fork branch viareplace. Once released: drop thereplaceand bumppion/mdns/v2.What's advertised
jetkvm-abc123)jetkvm-abc123.local.— existing local-name resolution preservedversion=—builtAppVersionid=—GetDeviceID()setup=true|false— flips after the user finishesPOST /device/setupImplementation
Uses
pion/mdns/v2, which is already in the tree transitively viapion/webrtc. pion takes the IPv4 and IPv6 multicast packet conns as separate args, so the existingMDNSMode=ipv4_only/ipv6_onlyconfig keeps working — we just don't bind the disabled family. No second mDNS library, no custom responder. The responder is switched from the legacyServer()(A/AAAA only) toNewServer()so PTR/SRV/TXT are answered when a service is configured.The diff against
internal/mdnsis purely additive: the existing A/AAAA path, packet-conn setup, and locking are untouched; the change adds anMDNSServiceoption that maps to pion'sWithService.Lifecycle
networkStateChangedevent (existing behavior).handleSetupso thesetup=TXT flips promptly.setTLSStateso the advertised port follows TLS.SIGINT/SIGTERMcallsmDNS.Stop(), which closes the multicast sockets. Browsers age the entry out via record TTL (pion does not emit DNS-SD goodbye packets).Checklist
make test_e2elocally and passedgo vet ./...green via the buildkit container)Test plan (pending hardware, after the fork dep merges)
dns-sd -B _jetkvm._tcp(macOS) shows the device with the configured hostname, port, and TXT.avahi-browse -r _jetkvm._tcp(Linux) shows the same.MDNSMode=ipv4_only/ipv6_onlyadvertises on that family only;disabledadvertises nothing.POST /device/setupflips TXTsetupfromfalsetotrueon the next browse.dig @224.0.0.251 -p 5353 jetkvm.localstill resolves.🤖 Generated with Claude Code