-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(mdns): filter addresses to reduce packet size #3434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Filter mDNS advertised addresses to help stay within 1500-byte MTU: - Skip browser transports (WebTransport, WebRTC, WebSocket) - Skip circuit relay addresses - Skip non-.local DNS names (require unicast DNS) - Allow IP addresses and .local DNS names (RFC 6762) Fixes: #3415
| switch first.Protocol().Code { | ||
| case ma.P_IP4, ma.P_IP6: | ||
| // Direct IP addresses are always suitable for LAN discovery | ||
| case ma.P_DNS, ma.P_DNS4, ma.P_DNS6, ma.P_DNSADDR: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does DNSADDR make sense here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added it mostly for completeness, but yes, technically someone could have TXT record on _dnsaddr.foo.local and use it for signaling peerid similar to how one can ipfs swarm connect /dnsaddr/bootstrap.libp2p.io without knowing peerid upfront (or connecting to more than one peer)
p2p/discovery/mdns/mdns.go
Outdated
| // WebRTC, WebSocket) because these are not useful for direct LAN discovery. | ||
| // | ||
| // Filtering reduces mDNS packet size, helping stay within the 1500-byte MTU | ||
| // limit per RFC 6762. See: https://github.com/libp2p/go-libp2p/issues/3415 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A slight correction in this comment. The RFC states the packet "MUST NOT exceed 9000 bytes". Below 1500 bytes is recommended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 1aabf13
per review feedback: RFC 6762 states packets MUST NOT exceed 9000 bytes, with 1500 bytes being recommended
MarcoPolo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ty!
This PR filters mDNS advertised addresses to help stay within 1500-byte MTU:
.localDNS names (require unicast DNS).localDNS names (RFC 6762)This PR aims to help IPFS (Kubo) users with local discovery problems on Windows and macOS, as noted in #3415