From 83babedf0db16c2cc7767a861602b7a9450ea308 Mon Sep 17 00:00:00 2001 From: sukun Date: Sun, 27 Aug 2023 15:40:27 +0530 Subject: [PATCH] use the host black hole detector to check for dialability --- config/config.go | 6 +++++ core/network/network.go | 18 +++++++++++++-- p2p/net/mock/mock_peernet.go | 4 ++-- p2p/net/swarm/black_hole_detector.go | 28 +++++++++++++++++++++++ p2p/net/swarm/black_hole_detector_test.go | 12 ++++++++++ p2p/net/swarm/swarm.go | 4 ++-- p2p/net/swarm/swarm_dial.go | 21 ++++++++++++++--- p2p/protocol/autonatv2/autonat.go | 12 ++++++---- p2p/protocol/autonatv2/server.go | 11 ++++++++- 9 files changed, 101 insertions(+), 15 deletions(-) diff --git a/config/config.go b/config/config.go index 21cbbb66e8..04a1054813 100644 --- a/config/config.go +++ b/config/config.go @@ -216,6 +216,12 @@ func (cfg *Config) makeAutoNATHost() (*blankhost.BlankHost, error) { PeerKey: autonatPrivKey, Peerstore: ps, DialRanker: swarm.NoDelayDialRanker, + SwarmOpts: []swarm.Option{ + // Disable black hole detection on autonat dialers + // It is better to attempt a dial and fail for AutoNAT use cases + swarm.WithUDPBlackHoleConfig(false, 0, 0), + swarm.WithIPv6BlackHoleConfig(false, 0, 0), + }, } dialer, err := autoNatCfg.makeSwarm(eventbus.NewBus(), false) diff --git a/core/network/network.go b/core/network/network.go index cd50052da8..42d352a1bd 100644 --- a/core/network/network.go +++ b/core/network/network.go @@ -152,6 +152,20 @@ type Network interface { ResourceManager() ResourceManager } +// Dialability indicates how dialable a (peer, addr) combination is. +type Dialability int + +const ( + // DialabilityUnknown indicates that the dialer cannot dial the (peer, addr) combination + DialabilityUnknown Dialability = iota + + // DialabilityDialable indicates that the dialer can dial the (peer, addr) combination + DialabilityDialable + + // DialabilityUndialable indicates that the dialer cannot dial the (peer, addr) combinaton + DialabilityUndialable +) + // Dialer represents a service that can dial out to peers // (this is usually just a Network, but other services may not need the whole // stack, and thus it becomes easier to mock) @@ -186,8 +200,8 @@ type Dialer interface { Notify(Notifiee) StopNotify(Notifiee) - // CanDial returns whether an address is dialable - CanDial(a ma.Multiaddr) bool + // CanDial returns whether the dialer can dial peer p at addr + CanDial(p peer.ID, addr ma.Multiaddr) Dialability } // AddrDelay provides an address along with the delay after which the address diff --git a/p2p/net/mock/mock_peernet.go b/p2p/net/mock/mock_peernet.go index 5f8c63c73c..8a06a95637 100644 --- a/p2p/net/mock/mock_peernet.go +++ b/p2p/net/mock/mock_peernet.go @@ -435,6 +435,6 @@ func (pn *peernet) ResourceManager() network.ResourceManager { return &network.NullResourceManager{} } -func (pn *peernet) CanDial(addr ma.Multiaddr) bool { - return true +func (pn *peernet) CanDial(p peer.ID, addr ma.Multiaddr) network.Dialability { + return network.DialabilityUnknown } diff --git a/p2p/net/swarm/black_hole_detector.go b/p2p/net/swarm/black_hole_detector.go index dd7849eea6..be789beae8 100644 --- a/p2p/net/swarm/black_hole_detector.go +++ b/p2p/net/swarm/black_hole_detector.go @@ -144,6 +144,13 @@ func (b *blackHoleFilter) updateState() { } } +func (b *blackHoleFilter) State() blackHoleState { + b.mu.Lock() + defer b.mu.Unlock() + + return b.state +} + func (b *blackHoleFilter) trackMetrics() { if b.metricsTracer == nil { return @@ -244,6 +251,27 @@ func (d *blackHoleDetector) RecordResult(addr ma.Multiaddr, success bool) { } } +func (d *blackHoleDetector) State(addr ma.Multiaddr) blackHoleState { + if !manet.IsPublicAddr(addr) { + return blackHoleStateAllowed + } + + if d.udp != nil && isProtocolAddr(addr, ma.P_UDP) { + udpState := d.udp.State() + if udpState != blackHoleStateAllowed { + return udpState + } + } + + if d.ipv6 != nil && isProtocolAddr(addr, ma.P_IP6) { + ipv6State := d.ipv6.State() + if ipv6State != blackHoleStateAllowed { + return ipv6State + } + } + return blackHoleStateAllowed +} + // blackHoleConfig is the config used for black hole detection type blackHoleConfig struct { // Enabled enables black hole detection diff --git a/p2p/net/swarm/black_hole_detector_test.go b/p2p/net/swarm/black_hole_detector_test.go index dfbb30f90d..f62e8ca8c6 100644 --- a/p2p/net/swarm/black_hole_detector_test.go +++ b/p2p/net/swarm/black_hole_detector_test.go @@ -17,6 +17,9 @@ func TestBlackHoleFilterReset(t *testing.T) { if bhf.HandleRequest() != blackHoleResultProbing { t.Fatalf("expected calls up to n to be probes") } + if bhf.State() != blackHoleStateProbing { + t.Fatalf("expected state to be probing got %s", bhf.State()) + } bhf.RecordResult(false) } @@ -26,6 +29,9 @@ func TestBlackHoleFilterReset(t *testing.T) { if (i%n == 0 && result != blackHoleResultProbing) || (i%n != 0 && result != blackHoleResultBlocked) { t.Fatalf("expected every nth dial to be a probe") } + if bhf.State() != blackHoleStateBlocked { + t.Fatalf("expected state to be blocked, got %s", bhf.State()) + } } bhf.RecordResult(true) @@ -34,12 +40,18 @@ func TestBlackHoleFilterReset(t *testing.T) { if bhf.HandleRequest() != blackHoleResultProbing { t.Fatalf("expected black hole detector state to reset after success") } + if bhf.State() != blackHoleStateProbing { + t.Fatalf("expected state to be probing got %s", bhf.State()) + } bhf.RecordResult(false) } // next call should be blocked if bhf.HandleRequest() != blackHoleResultBlocked { t.Fatalf("expected dial to be blocked") + if bhf.State() != blackHoleStateBlocked { + t.Fatalf("expected state to be blocked, got %s", bhf.State()) + } } } diff --git a/p2p/net/swarm/swarm.go b/p2p/net/swarm/swarm.go index 5155cd2228..b851f4e916 100644 --- a/p2p/net/swarm/swarm.go +++ b/p2p/net/swarm/swarm.go @@ -111,7 +111,7 @@ func WithDialRanker(d network.DialRanker) Option { } } -// WithUDPBlackHoleConfig configures swarm to use c as the config for UDP black hole detection +// WithUDPBlackHoleConfig configures swarm to use the provided config for UDP black hole detection // n is the size of the sliding window used to evaluate black hole state // min is the minimum number of successes out of n required to not block requests func WithUDPBlackHoleConfig(enabled bool, n, min int) Option { @@ -121,7 +121,7 @@ func WithUDPBlackHoleConfig(enabled bool, n, min int) Option { } } -// WithIPv6BlackHoleConfig configures swarm to use c as the config for IPv6 black hole detection +// WithIPv6BlackHoleConfig configures swarm to use the provided config for IPv6 black hole detection // n is the size of the sliding window used to evaluate black hole state // min is the minimum number of successes out of n required to not block requests func WithIPv6BlackHoleConfig(enabled bool, n, min int) Option { diff --git a/p2p/net/swarm/swarm_dial.go b/p2p/net/swarm/swarm_dial.go index 917c2ebee0..4657f15fef 100644 --- a/p2p/net/swarm/swarm_dial.go +++ b/p2p/net/swarm/swarm_dial.go @@ -417,9 +417,24 @@ func (s *Swarm) dialNextAddr(ctx context.Context, p peer.ID, addr ma.Multiaddr, return nil } -func (s *Swarm) CanDial(addr ma.Multiaddr) bool { - t := s.TransportForDialing(addr) - return t != nil && t.CanDial(addr) +func (s *Swarm) CanDial(p peer.ID, addr ma.Multiaddr) network.Dialability { + dialable, _ := s.filterKnownUndialables(p, []ma.Multiaddr{addr}) + if len(dialable) == 0 { + return network.DialabilityUndialable + } + + bhState := s.bhd.State(addr) + switch bhState { + case blackHoleStateAllowed: + return network.DialabilityDialable + case blackHoleStateProbing: + return network.DialabilityUnknown + case blackHoleStateBlocked: + return network.DialabilityUndialable + default: + log.Errorf("SWARM BUG: unhandled black hole state for dilability check %s", bhState) + return network.DialabilityUnknown + } } func (s *Swarm) nonProxyAddr(addr ma.Multiaddr) bool { diff --git a/p2p/protocol/autonatv2/autonat.go b/p2p/protocol/autonatv2/autonat.go index df49dae356..26811d0943 100644 --- a/p2p/protocol/autonatv2/autonat.go +++ b/p2p/protocol/autonatv2/autonat.go @@ -87,7 +87,9 @@ type AutoNAT struct { // New returns a new AutoNAT instance. The returned instance runs the server when the provided host // is publicly reachable. -func New(h host.Host, dialer host.Host, opts ...AutoNATOption) (*AutoNAT, error) { +// host and dialerHost should have the same dialing capabilities. In case the host doesn't support +// a transport, dial back requests for address for that transport will be ignored. +func New(host host.Host, dialerHost host.Host, opts ...AutoNATOption) (*AutoNAT, error) { s := defaultSettings() for _, o := range opts { if err := o(s); err != nil { @@ -108,7 +110,7 @@ func New(h host.Host, dialer host.Host, opts ...AutoNATOption) (*AutoNAT, error) // (https://github.com/libp2p/go-libp2p/issues/2229)(to be implemented in a future release) // to determine reachability using v2 client and send this event from Address Pipeline, if // we are publicly reachable. - sub, err := h.EventBus().Subscribe([]interface{}{ + sub, err := host.EventBus().Subscribe([]interface{}{ new(event.EvtLocalReachabilityChanged), new(event.EvtPeerProtocolsUpdated), new(event.EvtPeerConnectednessChanged), @@ -120,12 +122,12 @@ func New(h host.Host, dialer host.Host, opts ...AutoNATOption) (*AutoNAT, error) ctx, cancel := context.WithCancel(context.Background()) an := &AutoNAT{ - host: h, + host: host, ctx: ctx, cancel: cancel, sub: sub, - srv: newServer(h, dialer, s), - cli: newClient(h), + srv: newServer(host, dialerHost, s), + cli: newClient(host), allowAllAddrs: s.allowAllAddrs, peers: newPeersMap(), } diff --git a/p2p/protocol/autonatv2/server.go b/p2p/protocol/autonatv2/server.go index 886c9d22e3..5e788173cb 100644 --- a/p2p/protocol/autonatv2/server.go +++ b/p2p/protocol/autonatv2/server.go @@ -23,6 +23,9 @@ type dataRequestPolicyFunc = func(s network.Stream, dialAddr ma.Multiaddr) bool // server implements the AutoNATv2 server. // It can ask client to provide dial data before attempting the requested dial. // It rate limits requests on a global level, per peer level and on whether the request requires dial data. +// +// This uses the host's dialer as well as the dialerHost's dialer to determine whether an address is +// dialable. type server struct { host host.Host dialerHost host.Host @@ -112,7 +115,13 @@ func (as *server) handleDialRequest(s network.Stream) { continue } if (!as.allowAllAddrs && !manet.IsPublicAddr(a)) || - (!as.dialerHost.Network().CanDial(a)) { + (as.dialerHost.Network().CanDial(p, a) != network.DialabilityDialable) { + continue + } + // Check if the host can dial the address. This check ensures that we do not + // attempt dialing an IPv6 address if we have no IPv6 connectivity as the host dialer's + // black hole detector is likely to be more accurate. + if as.host.Network().CanDial(p, a) != network.DialabilityDialable { continue } dialAddr = a