Skip to content
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

Future of mDNS discovery #28

Open
Stebalien opened this issue Mar 1, 2018 · 15 comments
Open

Future of mDNS discovery #28

Stebalien opened this issue Mar 1, 2018 · 15 comments

Comments

@Stebalien
Copy link
Member

After spending way too much time trying to figure out how to follow the semantics mDNS spec, I've come to the conclusion that, in light of our multiaddr system, it's probably not worth it. So, my proposal is to ignore the port/ip addresses and duplicate this information in the TXT field (flagrantly violating the spec).

Requirements

  • Needs to work with multiple transports (multiaddrs). Nether of the current implementations (js or go) supports this.
  • Needs to be compatible with all reasonably functional mDNS libraries.

Current State

js-ipfs

  • Uses ipfs.local while the spec requires _$service._$proto.local where $service can be anything (but the _ prefix is strongly suggested) and $proto must be tcp or udp (where udp is just a catch-all for everything else). This is a requirement of the SRV spec and we probably shouldn't be violating it.
  • Doesn't announce full multiaddrs (won't work with, e.g., QUIC).

go-ipfs

  • Uses _ipfs-discovery._udp.local. We're not announcing the discovery service, we're announcing the ipfs service.
  • Also doesn't announce full multiaddrs.
  • May hijack the hostname? Need to look into this more.

Proposal

First, we need to specify the Instance, ServiceName, Proto, and TLD fields (for mDNS).

The constraints are:

  • The TLD must be .local (for link-local mDNS, at least).
  • The proto should be either _udp or _tcp for legacy reasons. As _udp is treated as the "catch all" we should use that.
  • The service name (third to last) must begin with an underscore.
  • The instance name can be anything.

I propose:

  • Instance: Peer ID. Like js, go doesn't specify one.
  • ServiceName: _ipfs. This is inconsistent with both js and go but obeys the spec (starts with an underscore, names the actual service we care about). Also, using a different service name will provide us a clean break between new and old nodes.
  • Proto: _udp. We need something and _udp appears to be the catch all.

Second, we will need to at least specify a set of IPs and ports.

  • I'd just let the library pick whatever IPs it wants. We won't read them.
  • For the port, could we just put 0? We really don't care.

Third, we need to specify the multiaddrs somewhere. I'd like to reuse dnsaddr for this. That is, for each multiaddr, attach a TXT record of the form dnsaddr=/full/multiaddr.

Finally, we need to put a reasonable hostname in the A/AAAA and SRV records. We could continue putting in the local hostname but I'd kind of like to use QmId.ipfs.local (it's not the job of ipfs to broadcast the hostname to the network). Will that cause any problem)?

@whyrusleeping, @diasdavid, @richardschneider thoughts?

@daviddias
Copy link
Member

I like the proposal (as we sync'ed up) to use the TXT record as a store for dnsaddr=/full/multiaddr and ignore the ports and IP that come in the rest of the message. I do not see why this is a violation of the spec though.

@richardschneider
Copy link

I much prefer ipfs.local over _ipfs._upd.local, but I understand you legacy concerns.

Do not like the idea of port 0 in a SRV record. Why not 4001?

Or do we need to even publish a SRV record?

Why not just publish an A and TXT record.

@whyrusleeping
Copy link

Do not like the idea of port 0 in a SRV record. Why not 4001?

Because its not always 4001. It could be something else, or multiple something elses.

@Stebalien
Copy link
Member Author

I much prefer ipfs.local over _ipfs._upd.local, but I understand you legacy concerns.

I totally agree but... I'm worried that spec compliant libraries will simply ignore invalid records. SRV records require "names" to be of the form _service._proto.local where _proto is either _udp or _tcp. The mDNS spec itself laments at this requirement (the authors would rather have just used _service.local).

Or do we need to even publish a SRV record?

It's required by the mDNS spec, unfortunately.

Why not just publish an A and TXT record.

Really, we only care about the TXT record and the PTR record. Unfortunately, the go libraries, at least, choke when there are no A/AAAA records present.

I do not see why this is a violation of the spec though.

The spec very explicitly states that the TXT record should generally only be used for legacy systems and hints and that it absolutely MUST NOT duplicate any information from the SRV record (port/hostname).

@richardschneider
Copy link

@Stebalien I agree with your comments.

So a question for an ipfs service will have NAME = "_ipfs._upd.local". What about CLASS and TYPE?

Perhaps?

  • CLASS = IN (internet)
  • TYPE = PTR

@richardschneider
Copy link

@Stebalien more bike shedding

Since we are trying to replace ipfs with p2p, see protocols.csv, I think the service name should be _p2p._udp.local.

@Stebalien
Copy link
Member Author

@richardschneider Ideally, we'd announce both.

@daviddias
Copy link
Member

@richardschneider it would be genius if you could compile the results of this issue into a small spec to libp2p/specs that all implementations can follow.

@richardschneider
Copy link

It's been on my todo list to write some notes on MDNS, I will add it to libp2p/specs (real soon).

TXT record dnsaddr=/full/address is a problem. Strings must be less than 128 bytes, An ipv6 multiaddress (dnsaddr=/ip6/2406:e001:1467:1:ecec:9ca6:XXXX:XXXX/tcp/4009/ipfs/QmTp1fwYnJxc5rUEVM2xowcDiyR61J7yvmBRUQzT4g8MHs) almost reaches this limit. If the peer-id hashing algorithm changes to sha2-512 then the byte limit is exceeded.

I suggest we go back to generating A/AAAA records and have the peer synthesize the multiaddres.

@richardschneider
Copy link

@diasdavid can you grant me access to 'https://github.com/libp2p/specs.git/'. I have a spec ready for review (minus working examples).

@daviddias
Copy link
Member

@richardschneider you got it :)

@Kubuxu
Copy link
Member

Kubuxu commented Oct 29, 2018

An important factor, we either have to bind 5353 for mDNS using SO_REUSEPORT to allow system mDNS daemon to work.

Also, it would be nice if we used system specific servers/resolvers but it could be problematic.

@Stebalien
Copy link
Member Author

An important factor, we either have to bind 5353 for mDNS using SO_REUSEPORT to allow system mDNS daemon to work.

Unfortunately, that won't work as only one application will receive the packets. Really, we need to register with the OS (Avahi, windows, etc.).

@tomaka
Copy link
Member

tomaka commented Oct 29, 2018

Unfortunately, that won't work as only one application will receive the packets.

I'm not sure that's true. My Rust prototype (libp2p/rust-libp2p#590) "just works". I can start it multiple times in addition to sniffing applications, without any issue.

@Stebalien
Copy link
Member Author

Apparently, you're right. Multicast packets are delivered to all listeners on a port. Although, apparently, we need to be careful about sending responses to specific IP addresses ("QU responses").

https://tools.ietf.org/html/rfc6762#section-15.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants