From 7f7b97b5012dfa5783968c2a700f4eb969bdb7f6 Mon Sep 17 00:00:00 2001 From: Raghav Gulati Date: Mon, 26 Feb 2018 21:50:43 -0800 Subject: [PATCH 1/8] discovery: swap mdns lib for grandcat/zeroconf This commit makes the minimal amount of changes to switch us from whyrusleeping/mdns to grandcat/zeroconf. Of note, rather than asking the host.Host which addrs we're available to listen on, we push this to the zeroconf library (which runs across available ifaces). --- p2p/discovery/mdns.go | 87 +++++++++++++++---------------------------- 1 file changed, 30 insertions(+), 57 deletions(-) diff --git a/p2p/discovery/mdns.go b/p2p/discovery/mdns.go index a7ec72ab12..4d594b85cf 100644 --- a/p2p/discovery/mdns.go +++ b/p2p/discovery/mdns.go @@ -2,21 +2,21 @@ package discovery import ( "context" - "errors" "io" "io/ioutil" golog "log" "net" + "strings" "sync" "time" + mdns "github.com/grandcat/zeroconf" logging "github.com/ipfs/go-log" "github.com/libp2p/go-libp2p-host" "github.com/libp2p/go-libp2p-peer" pstore "github.com/libp2p/go-libp2p-peerstore" ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr-net" - "github.com/whyrusleeping/mdns" ) var log = logging.Logger("mdns") @@ -34,72 +34,36 @@ type Notifee interface { } type mdnsService struct { - server *mdns.Server - service *mdns.MDNSService - host host.Host - tag string + server *mdns.Server + host host.Host + tag string lk sync.Mutex notifees []Notifee interval time.Duration } -func getDialableListenAddrs(ph host.Host) ([]*net.TCPAddr, error) { - var out []*net.TCPAddr - for _, addr := range ph.Addrs() { - na, err := manet.ToNetAddr(addr) - if err != nil { - continue - } - tcp, ok := na.(*net.TCPAddr) - if ok { - out = append(out, tcp) - } - } - if len(out) == 0 { - return nil, errors.New("failed to find good external addr from peerhost") - } - return out, nil -} - func NewMdnsService(ctx context.Context, peerhost host.Host, interval time.Duration, serviceTag string) (Service, error) { // TODO: dont let mdns use logging... golog.SetOutput(ioutil.Discard) - var ipaddrs []net.IP port := 4001 - - addrs, err := getDialableListenAddrs(peerhost) - if err != nil { - log.Warning(err) - } else { - port = addrs[0].Port - for _, a := range addrs { - ipaddrs = append(ipaddrs, a.IP) - } - } - myid := peerhost.ID().Pretty() info := []string{myid} if serviceTag == "" { serviceTag = ServiceTag } - service, err := mdns.NewMDNSService(myid, serviceTag, "", "", port, ipaddrs, info) - if err != nil { - return nil, err - } // Create the mDNS server, defer shutdown - server, err := mdns.NewServer(&mdns.Config{Zone: service}) + server, err := mdns.Register(myid, serviceTag, "", port, info, nil) if err != nil { return nil, err } s := &mdnsService{ server: server, - service: service, host: peerhost, interval: interval, tag: serviceTag, @@ -111,34 +75,38 @@ func NewMdnsService(ctx context.Context, peerhost host.Host, interval time.Durat } func (m *mdnsService) Close() error { - return m.server.Shutdown() + m.server.Shutdown() + // grandcat/zerconf swallows error, satisfy interface + return nil } func (m *mdnsService) pollForEntries(ctx context.Context) { ticker := time.NewTicker(m.interval) for { + resolver, err := mdns.NewResolver(nil) + if err != nil { + log.Error("Failed to initialize resolver:", err) + } + //execute mdns query right away at method call and then with every tick entriesCh := make(chan *mdns.ServiceEntry, 16) - go func() { - for entry := range entriesCh { + go func(results <-chan *mdns.ServiceEntry) { + for entry := range results { m.handleEntry(entry) } - }() + }(entriesCh) log.Debug("starting mdns query") - qp := &mdns.QueryParam{ - Domain: "local", - Entries: entriesCh, - Service: m.tag, - Timeout: time.Second * 5, - } - err := mdns.Query(qp) - if err != nil { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) + defer cancel() + + if err := resolver.Browse(ctx, m.tag, "local", entriesCh); err != nil { log.Error("mdns lookup error: ", err) } close(entriesCh) + log.Debug("mdns query complete") select { @@ -152,8 +120,13 @@ func (m *mdnsService) pollForEntries(ctx context.Context) { } func (m *mdnsService) handleEntry(e *mdns.ServiceEntry) { - log.Debugf("Handling MDNS entry: %s:%d %s", e.AddrV4, e.Port, e.Info) - mpeer, err := peer.IDB58Decode(e.Info) + // pull out the txt + info := strings.Join(e.Text, "|") + + // addripv4 potential issue + log.Debugf("Handling MDNS entry: %s:%d %s", e.AddrIPv4[0], e.Port, info) + + mpeer, err := peer.IDB58Decode(info) if err != nil { log.Warning("Error parsing peer ID from mdns entry: ", err) return @@ -165,7 +138,7 @@ func (m *mdnsService) handleEntry(e *mdns.ServiceEntry) { } maddr, err := manet.FromNetAddr(&net.TCPAddr{ - IP: e.AddrV4, + IP: e.AddrIPv4[0], Port: e.Port, }) if err != nil { From c5a5cf3332bad9772e336ccd2e6de11483c51dd8 Mon Sep 17 00:00:00 2001 From: Raghav Gulati Date: Tue, 27 Feb 2018 10:21:57 -0800 Subject: [PATCH 2/8] Address PR feedback. 1. If we receive multiple ipv4 addresses, ensure that we convert them all to multiaddrs and store them in the peerstore. 2. Ensure we don't allocate a new resolver on every tick. Instead reuse this by storing it in our mdns type. 3. Attempt choosing a port. --- p2p/discovery/mdns.go | 62 ++++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/p2p/discovery/mdns.go b/p2p/discovery/mdns.go index 4d594b85cf..d0673d48f5 100644 --- a/p2p/discovery/mdns.go +++ b/p2p/discovery/mdns.go @@ -34,9 +34,10 @@ type Notifee interface { } type mdnsService struct { - server *mdns.Server - host host.Host - tag string + server *mdns.Server + service *mdns.Resolver + host host.Host + tag string lk sync.Mutex notifees []Notifee @@ -48,7 +49,7 @@ func NewMdnsService(ctx context.Context, peerhost host.Host, interval time.Durat // TODO: dont let mdns use logging... golog.SetOutput(ioutil.Discard) - port := 4001 + port := 42424 myid := peerhost.ID().Pretty() info := []string{myid} @@ -56,6 +57,11 @@ func NewMdnsService(ctx context.Context, peerhost host.Host, interval time.Durat serviceTag = ServiceTag } + resolver, err := mdns.NewResolver(nil) + if err != nil { + log.Error("Failed to initialize resolver:", err) + } + // Create the mDNS server, defer shutdown server, err := mdns.Register(myid, serviceTag, "", port, info, nil) if err != nil { @@ -64,6 +70,7 @@ func NewMdnsService(ctx context.Context, peerhost host.Host, interval time.Durat s := &mdnsService{ server: server, + service: resolver, host: peerhost, interval: interval, tag: serviceTag, @@ -81,14 +88,8 @@ func (m *mdnsService) Close() error { } func (m *mdnsService) pollForEntries(ctx context.Context) { - ticker := time.NewTicker(m.interval) for { - resolver, err := mdns.NewResolver(nil) - if err != nil { - log.Error("Failed to initialize resolver:", err) - } - //execute mdns query right away at method call and then with every tick entriesCh := make(chan *mdns.ServiceEntry, 16) go func(results <-chan *mdns.ServiceEntry) { @@ -102,7 +103,7 @@ func (m *mdnsService) pollForEntries(ctx context.Context) { ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) defer cancel() - if err := resolver.Browse(ctx, m.tag, "local", entriesCh); err != nil { + if err := m.service.Browse(ctx, m.tag, "local", entriesCh); err != nil { log.Error("mdns lookup error: ", err) } close(entriesCh) @@ -123,9 +124,6 @@ func (m *mdnsService) handleEntry(e *mdns.ServiceEntry) { // pull out the txt info := strings.Join(e.Text, "|") - // addripv4 potential issue - log.Debugf("Handling MDNS entry: %s:%d %s", e.AddrIPv4[0], e.Port, info) - mpeer, err := peer.IDB58Decode(info) if err != nil { log.Warning("Error parsing peer ID from mdns entry: ", err) @@ -137,25 +135,29 @@ func (m *mdnsService) handleEntry(e *mdns.ServiceEntry) { return } - maddr, err := manet.FromNetAddr(&net.TCPAddr{ - IP: e.AddrIPv4[0], - Port: e.Port, - }) - if err != nil { - log.Warning("Error parsing multiaddr from mdns entry: ", err) - return - } + for _, ipv4 := range e.AddrIPv4 { + log.Debugf("Handling MDNS entry: %s:%d %s", ipv4, e.Port, info) - pi := pstore.PeerInfo{ - ID: mpeer, - Addrs: []ma.Multiaddr{maddr}, - } + maddr, err := manet.FromNetAddr(&net.TCPAddr{ + IP: ipv4, + Port: e.Port, + }) + if err != nil { + log.Warning("Error parsing multiaddr from mdns entry: ", err) + return + } - m.lk.Lock() - for _, n := range m.notifees { - go n.HandlePeerFound(pi) + pi := pstore.PeerInfo{ + ID: mpeer, + Addrs: []ma.Multiaddr{maddr}, + } + + m.lk.Lock() + for _, n := range m.notifees { + go n.HandlePeerFound(pi) + } + m.lk.Unlock() } - m.lk.Unlock() } func (m *mdnsService) RegisterNotifee(n Notifee) { From f4981054c7fdf805e6fda6883a5cb92c52822dc1 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 23 Jul 2018 12:02:24 -0700 Subject: [PATCH 3/8] fix gx imports --- package.json | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 1df54e44e0..c8beebd545 100644 --- a/package.json +++ b/package.json @@ -13,11 +13,6 @@ "name": "go-semver", "version": "0.0.0" }, - { - "hash": "QmNeRzgrSpwbMU7VKLRyfvbqf1nRrAiQ7fEXaBxWGT5Ygr", - "name": "mdns", - "version": "0.1.3" - }, { "hash": "QmPdKqUcHGFdeSpvjVoaTRPPstGif9GBZb5Q56RVw9o69A", "name": "go-ipfs-util", @@ -249,6 +244,12 @@ "hash": "QmSSeQqc5QeuefkaM6JFV5tSF9knLUkXKVhW1eYRiqe72W", "name": "uuid", "version": "0.1.0" + }, + { + "author": "grandcat", + "hash": "QmUTmYpDk9jVk7PeBgmr3n1oGmLJ6yEuSVqyZaWUpF13f3", + "name": "zeroconf", + "version": "0.2.0" } ], "gxVersion": "0.4.0", From 528f1571ebdab953ecc13bf412c4a034620fd0b9 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 23 Jul 2018 12:37:14 -0700 Subject: [PATCH 4/8] try to pick a decent port in mdns Ideally, we'd dynamically announce ports based on the current interfaces. However, we'd like to merge this ASAP. --- p2p/discovery/mdns.go | 46 ++++++++++++++++++++++++++++++++++- p2p/discovery/mdns_test.go | 49 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 92 insertions(+), 3 deletions(-) diff --git a/p2p/discovery/mdns.go b/p2p/discovery/mdns.go index d0673d48f5..4e03648173 100644 --- a/p2p/discovery/mdns.go +++ b/p2p/discovery/mdns.go @@ -2,6 +2,7 @@ package discovery import ( "context" + "errors" "io" "io/ioutil" golog "log" @@ -44,12 +45,55 @@ type mdnsService struct { interval time.Duration } +// Tries to pick the best port. +func getBestPort(addrs []ma.Multiaddr) (int, error) { + var best *net.TCPAddr + for _, addr := range addrs { + na, err := manet.ToNetAddr(addr) + if err != nil { + continue + } + tcp, ok := na.(*net.TCPAddr) + if !ok { + continue + } + // Don't bother with multicast and + if tcp.IP.IsMulticast() { + continue + } + // We don't yet support link-local + if tcp.IP.IsLinkLocalUnicast() { + continue + } + // Unspecified listeners are *always* the best choice. + if tcp.IP.IsUnspecified() { + return tcp.Port, nil + } + // If we don't have a best choice, use this addr. + if best == nil { + best = tcp + continue + } + // If the best choice is a loopback address, replace it. + if best.IP.IsLoopback() { + best = tcp + } + } + if best == nil { + return 0, errors.New("failed to find good external addr from peerhost") + } + return best.Port, nil +} + func NewMdnsService(ctx context.Context, peerhost host.Host, interval time.Duration, serviceTag string) (Service, error) { // TODO: dont let mdns use logging... golog.SetOutput(ioutil.Discard) - port := 42424 + port, err := getBestPort(peerhost.Network().ListenAddresses()) + if err != nil { + return nil, err + } myid := peerhost.ID().Pretty() info := []string{myid} diff --git a/p2p/discovery/mdns_test.go b/p2p/discovery/mdns_test.go index bb0e84d6f1..38b131c2f9 100644 --- a/p2p/discovery/mdns_test.go +++ b/p2p/discovery/mdns_test.go @@ -8,9 +8,9 @@ import ( bhost "github.com/libp2p/go-libp2p/p2p/host/basic" host "github.com/libp2p/go-libp2p-host" - swarmt "github.com/libp2p/go-libp2p-swarm/testing" - pstore "github.com/libp2p/go-libp2p-peerstore" + swarmt "github.com/libp2p/go-libp2p-swarm/testing" + ma "github.com/multiformats/go-multiaddr" ) type DiscoveryNotifee struct { @@ -21,6 +21,51 @@ func (n *DiscoveryNotifee) HandlePeerFound(pi pstore.PeerInfo) { n.h.Connect(context.Background(), pi) } +func TestGetBestPort(t *testing.T) { + port, err := getBestPort([]ma.Multiaddr{ma.StringCast("/ip4/1.2.3.4/tcp/2222"), ma.StringCast("/ip4/0.0.0.0/tcp/1234")}) + if err != nil { + t.Fatal(err) + } + if port != 1234 { + t.Errorf("expected port 1234, got port %d", port) + } + + port, err = getBestPort([]ma.Multiaddr{ma.StringCast("/ip4/127.0.0.1/tcp/2222"), ma.StringCast("/ip4/0.0.0.0/tcp/1234")}) + if err != nil { + t.Fatal(err) + } + if port != 1234 { + t.Errorf("expected port 1234, got port %d", port) + } + + port, err = getBestPort([]ma.Multiaddr{ma.StringCast("/ip4/1.2.3.4/tcp/2222"), ma.StringCast("/ip4/127.0.0.1/tcp/1234")}) + if err != nil { + t.Fatal(err) + } + if port != 2222 { + t.Errorf("expected port 2222, got port %d", port) + } + port, err = getBestPort([]ma.Multiaddr{ma.StringCast("/ip4/127.0.0.1/tcp/1234"), ma.StringCast("/ip4/1.2.3.4/tcp/2222")}) + if err != nil { + t.Fatal(err) + } + if port != 2222 { + t.Errorf("expected port 2222, got port %d", port) + } + port, err = getBestPort([]ma.Multiaddr{ma.StringCast("/ip4/127.0.0.1/tcp/1234")}) + if err != nil { + t.Fatal(err) + } + if port != 1234 { + t.Errorf("expected port 1234, got port %d", port) + } + + _, err = getBestPort([]ma.Multiaddr{}) + if err == nil { + t.Fatal("expected error") + } +} + func TestMdnsDiscovery(t *testing.T) { //TODO: re-enable when the new lib will get integrated t.Skip("TestMdnsDiscovery fails randomly with current lib") From da471d9e759715b0da4db5d025f17ce719d648fa Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 23 Jul 2018 16:45:34 -0700 Subject: [PATCH 5/8] use fixed zeroconf gxified package --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index c8beebd545..f0898cb65a 100644 --- a/package.json +++ b/package.json @@ -247,9 +247,9 @@ }, { "author": "grandcat", - "hash": "QmUTmYpDk9jVk7PeBgmr3n1oGmLJ6yEuSVqyZaWUpF13f3", + "hash": "QmWrJTtJCQ6kCCTtLBM82Cx9h52QMQ5hcbrsePFuRSHuWC", "name": "zeroconf", - "version": "0.2.0" + "version": "0.2.1" } ], "gxVersion": "0.4.0", From 6651bb965bc671ff5310668d62394c6ba0af34cc Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Tue, 24 Jul 2018 21:19:20 -0700 Subject: [PATCH 6/8] don't concat txt records when handling dns-sd announcements --- p2p/discovery/mdns.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/p2p/discovery/mdns.go b/p2p/discovery/mdns.go index 4e03648173..c21f6d5cef 100644 --- a/p2p/discovery/mdns.go +++ b/p2p/discovery/mdns.go @@ -7,7 +7,6 @@ import ( "io/ioutil" golog "log" "net" - "strings" "sync" "time" @@ -165,10 +164,12 @@ func (m *mdnsService) pollForEntries(ctx context.Context) { } func (m *mdnsService) handleEntry(e *mdns.ServiceEntry) { + if len(e.Text) != 1 { + log.Warningf("Expected exactly one TXT record, got: %v", e.Text) + return + } // pull out the txt - info := strings.Join(e.Text, "|") - - mpeer, err := peer.IDB58Decode(info) + mpeer, err := peer.IDB58Decode(e.Text[0]) if err != nil { log.Warning("Error parsing peer ID from mdns entry: ", err) return @@ -180,7 +181,7 @@ func (m *mdnsService) handleEntry(e *mdns.ServiceEntry) { } for _, ipv4 := range e.AddrIPv4 { - log.Debugf("Handling MDNS entry: %s:%d %s", ipv4, e.Port, info) + log.Debugf("Handling MDNS entry: %s:%d %s", ipv4, e.Port, e.Text[0]) maddr, err := manet.FromNetAddr(&net.TCPAddr{ IP: ipv4, From aaa7f87189bdb9e37ea5c0e4287b3837b305e94d Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Tue, 24 Jul 2018 21:20:34 -0700 Subject: [PATCH 7/8] mdns: put all multiaddrs into a single peer info --- p2p/discovery/mdns.go | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/p2p/discovery/mdns.go b/p2p/discovery/mdns.go index c21f6d5cef..90e2800e81 100644 --- a/p2p/discovery/mdns.go +++ b/p2p/discovery/mdns.go @@ -180,6 +180,7 @@ func (m *mdnsService) handleEntry(e *mdns.ServiceEntry) { return } + var pi pstore.PeerInfo for _, ipv4 := range e.AddrIPv4 { log.Debugf("Handling MDNS entry: %s:%d %s", ipv4, e.Port, e.Text[0]) @@ -188,21 +189,17 @@ func (m *mdnsService) handleEntry(e *mdns.ServiceEntry) { Port: e.Port, }) if err != nil { - log.Warning("Error parsing multiaddr from mdns entry: ", err) + log.Errorf("error creating multiaddr from mdns entry (%s:%d): %s", ipv4, e.Port, err) return } + pi.Addrs = append(pi.Addrs, maddr) + } - pi := pstore.PeerInfo{ - ID: mpeer, - Addrs: []ma.Multiaddr{maddr}, - } - - m.lk.Lock() - for _, n := range m.notifees { - go n.HandlePeerFound(pi) - } - m.lk.Unlock() + m.lk.Lock() + for _, n := range m.notifees { + go n.HandlePeerFound(pi) } + m.lk.Unlock() } func (m *mdnsService) RegisterNotifee(n Notifee) { From a7e8c11bb04cb27e33b4f602e54d8c076ed1987d Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Tue, 24 Jul 2018 21:31:22 -0700 Subject: [PATCH 8/8] mdns: also handle IPv6 addresses --- p2p/discovery/mdns.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/p2p/discovery/mdns.go b/p2p/discovery/mdns.go index 90e2800e81..b0eb9782c0 100644 --- a/p2p/discovery/mdns.go +++ b/p2p/discovery/mdns.go @@ -180,16 +180,20 @@ func (m *mdnsService) handleEntry(e *mdns.ServiceEntry) { return } + addrs := make([]net.IP, len(e.AddrIPv4)+len(e.AddrIPv6)) + copy(addrs, e.AddrIPv4) + copy(addrs[len(e.AddrIPv4):], e.AddrIPv6) + var pi pstore.PeerInfo - for _, ipv4 := range e.AddrIPv4 { - log.Debugf("Handling MDNS entry: %s:%d %s", ipv4, e.Port, e.Text[0]) + for _, ip := range addrs { + log.Debugf("Handling MDNS entry: %s:%d %s", ip, e.Port, e.Text[0]) maddr, err := manet.FromNetAddr(&net.TCPAddr{ - IP: ipv4, + IP: ip, Port: e.Port, }) if err != nil { - log.Errorf("error creating multiaddr from mdns entry (%s:%d): %s", ipv4, e.Port, err) + log.Errorf("error creating multiaddr from mdns entry (%s:%d): %s", ip, e.Port, err) return } pi.Addrs = append(pi.Addrs, maddr)