diff --git a/p2p/net/swarm/dial_ranker.go b/p2p/net/swarm/dial_ranker.go index a77e2cc6a5..f0b6ca621f 100644 --- a/p2p/net/swarm/dial_ranker.go +++ b/p2p/net/swarm/dial_ranker.go @@ -1,7 +1,6 @@ package swarm import ( - "net/netip" "sort" "strconv" "time" @@ -53,16 +52,8 @@ func noDelayRanker(addrs []ma.Multiaddr) []network.AddrDelay { // The correct solution is to detect this situation, and not attempt to dial IPv6 addresses at all. // IPv6 blackhole detection is tracked in https://github.com/libp2p/go-libp2p/issues/1605. // -// Within each group (private, public IPv4, public IPv6, relay addresses) we apply the following logic: -// -// 1. Filter out addresses we don't want to dial: -// 1. If a /quic-v1 address is present, filter out /quic and /webtransport address on the same 2-tuple: -// QUIC v1 is preferred over the deprecated QUIC draft-29, and given the choice, we prefer using -// raw QUIC over using WebTransport. -// 2. If a /tcp address is present, filter out /ws or /wss addresses on the same 2-tuple: -// We prefer using raw TCP over using WebSocket. -// -// 2. Rank addresses: +// Within each group (private, public IPv4, public IPv6, relay addresses) we apply the following +// ranking logic: // // 1. If two QUIC addresses are present, dial the QUIC address with the lowest port first: // This is more likely to be the listen port. After this we dial the rest of the QUIC addresses delayed by @@ -99,49 +90,11 @@ func DefaultDialRanker(addrs []ma.Multiaddr) []network.AddrDelay { // addresses relative to direct addresses func getAddrDelay(addrs []ma.Multiaddr, tcpDelay time.Duration, quicDelay time.Duration, offset time.Duration) []network.AddrDelay { - - // First make a map of QUICV1 and TCP AddrPorts. - quicV1Addr := make(map[netip.AddrPort]struct{}) - tcpAddr := make(map[netip.AddrPort]struct{}) - for _, a := range addrs { - switch { - case isProtocolAddr(a, ma.P_WEBTRANSPORT): - case isProtocolAddr(a, ma.P_QUIC_V1): - quicV1Addr[addrPort(a, ma.P_UDP)] = struct{}{} - case isProtocolAddr(a, ma.P_WS) || isProtocolAddr(a, ma.P_WSS): - case isProtocolAddr(a, ma.P_TCP): - tcpAddr[addrPort(a, ma.P_TCP)] = struct{}{} - } - } - - // Filter addresses we are sure we don't want to dial - selectedAddrs := addrs - i := 0 - for _, a := range addrs { - switch { - // If a QUICDraft29 or webtransport address is reachable, QUIC-v1 will also be reachable. So we - // drop the QUICDraft29 or webtransport address - // We prefer QUIC-v1 over the older QUIC-draft29 address. - // We prefer QUIC-v1 over webtransport as it is more performant. - case isProtocolAddr(a, ma.P_WEBTRANSPORT) || isProtocolAddr(a, ma.P_QUIC): - if _, ok := quicV1Addr[addrPort(a, ma.P_UDP)]; ok { - continue - } - // If a ws address is reachable, TCP will also be reachable and it'll be more performant - case isProtocolAddr(a, ma.P_WS) || isProtocolAddr(a, ma.P_WSS): - if _, ok := tcpAddr[addrPort(a, ma.P_TCP)]; ok { - continue - } - } - selectedAddrs[i] = a - i++ - } - selectedAddrs = selectedAddrs[:i] - sort.Slice(selectedAddrs, func(i, j int) bool { return score(selectedAddrs[i]) < score(selectedAddrs[j]) }) + sort.Slice(addrs, func(i, j int) bool { return score(addrs[i]) < score(addrs[j]) }) res := make([]network.AddrDelay, 0, len(addrs)) quicCount := 0 - for _, a := range selectedAddrs { + for _, a := range addrs { delay := offset switch { case isProtocolAddr(a, ma.P_QUIC) || isProtocolAddr(a, ma.P_QUIC_V1): @@ -192,16 +145,6 @@ func score(a ma.Multiaddr) int { return (1 << 30) } -// addrPort returns the ip and port for a. p should be either ma.P_TCP or ma.P_UDP. -// a must be an (ip, TCP) or (ip, udp) address. -func addrPort(a ma.Multiaddr, p int) netip.AddrPort { - ip, _ := manet.ToIP(a) - port, _ := a.ValueForProtocol(p) - pi, _ := strconv.Atoi(port) - addr, _ := netip.AddrFromSlice(ip) - return netip.AddrPortFrom(addr, uint16(pi)) -} - func isProtocolAddr(a ma.Multiaddr, p int) bool { found := false ma.ForEach(a, func(c ma.Component) bool { diff --git a/p2p/net/swarm/dial_ranker_test.go b/p2p/net/swarm/dial_ranker_test.go index 28cb6e8c1a..070932541f 100644 --- a/p2p/net/swarm/dial_ranker_test.go +++ b/p2p/net/swarm/dial_ranker_test.go @@ -28,6 +28,7 @@ func TestNoDelayRanker(t *testing.T) { q3 := ma.StringCast("/ip4/1.2.3.4/udp/3/quic") q3v1 := ma.StringCast("/ip4/1.2.3.4/udp/3/quic-v1") q4 := ma.StringCast("/ip4/1.2.3.4/udp/4/quic") + t1 := ma.StringCast("/ip4/1.2.3.5/tcp/1/") testCase := []struct { name string @@ -35,13 +36,18 @@ func TestNoDelayRanker(t *testing.T) { output []network.AddrDelay }{ { - name: "quic+webtransport filtered when quicv1", - addrs: []ma.Multiaddr{q1, q2, q3, q4, q1v1, q2v1, q3v1, wt1}, + name: "quic-ranking", + addrs: []ma.Multiaddr{q1, q2, q3, q4, q1v1, q2v1, q3v1, wt1, t1}, output: []network.AddrDelay{ + {Addr: q1, Delay: 0}, + {Addr: q2, Delay: 0}, + {Addr: q3, Delay: 0}, + {Addr: q4, Delay: 0}, {Addr: q1v1, Delay: 0}, {Addr: q2v1, Delay: 0}, {Addr: q3v1, Delay: 0}, - {Addr: q4, Delay: 0}, + {Addr: wt1, Delay: 0}, + {Addr: t1, Delay: 0}, }, }, } @@ -103,13 +109,17 @@ func TestDelayRankerQUICDelay(t *testing.T) { }, }, { - name: "quic+webtransport filtered when quicv1", + name: "quic-quic-v1-webtransport", addrs: []ma.Multiaddr{q1, q2, q3, q4, q1v1, q2v1, q3v1, wt1}, output: []network.AddrDelay{ {Addr: q1v1, Delay: 0}, + {Addr: q1, Delay: PublicQUICDelay}, + {Addr: q2, Delay: PublicQUICDelay}, + {Addr: q3, Delay: PublicQUICDelay}, + {Addr: q4, Delay: PublicQUICDelay}, {Addr: q2v1, Delay: PublicQUICDelay}, {Addr: q3v1, Delay: PublicQUICDelay}, - {Addr: q4, Delay: PublicQUICDelay}, + {Addr: wt1, Delay: PublicQUICDelay}, }, }, { diff --git a/p2p/net/swarm/swarm_dial.go b/p2p/net/swarm/swarm_dial.go index 7aa2befe7a..1f5feb13c6 100644 --- a/p2p/net/swarm/swarm_dial.go +++ b/p2p/net/swarm/swarm_dial.go @@ -4,6 +4,8 @@ import ( "context" "errors" "fmt" + "net/netip" + "strconv" "sync" "time" @@ -433,8 +435,9 @@ func (s *Swarm) nonProxyAddr(addr ma.Multiaddr) bool { // filterKnownUndialables takes a list of multiaddrs, and removes those // that we definitely don't want to dial: addresses configured to be blocked, // IPv6 link-local addresses, addresses without a dial-capable transport, -// and addresses that we know to be our own. -// This is an optimization to avoid wasting time on dials that we know are going to fail. +// addresses that we know to be our own, and addresses with a better tranport +// available. This is an optimization to avoid wasting time on dials that we +// know are going to fail or for which we have a better alternative. func (s *Swarm) filterKnownUndialables(p peer.ID, addrs []ma.Multiaddr) []ma.Multiaddr { lisAddrs, _ := s.InterfaceListenAddresses() var ourAddrs []ma.Multiaddr @@ -448,20 +451,10 @@ func (s *Swarm) filterKnownUndialables(p peer.ID, addrs []ma.Multiaddr) []ma.Mul }) } - // Make a map of udp ports we are listening on to filter peers web transport addresses - ourLocalHostUDPPorts := make(map[string]bool, 2) - for _, a := range ourAddrs { - if !manet.IsIPLoopback(a) { - continue - } - if p, err := a.ValueForProtocol(ma.P_UDP); err == nil { - ourLocalHostUDPPorts[p] = true - } - } + addrs = filterLowPriorityAddresses(addrs) return ma.FilterAddrs(addrs, func(addr ma.Multiaddr) bool { return !ma.Contains(ourAddrs, addr) }, - func(addr ma.Multiaddr) bool { return checkLocalHostUDPAddrs(addr, ourLocalHostUDPPorts) }, s.canDial, // TODO: Consider allowing link-local addresses func(addr ma.Multiaddr) bool { return !manet.IsIP6LinkLocal(addr) }, @@ -559,15 +552,79 @@ func isRelayAddr(addr ma.Multiaddr) bool { return err == nil } -// checkLocalHostUDPAddrs returns false for addresses that have the same localhost port -// as the one we are listening on -// This is useful for filtering out peer's localhost webtransport addresses. -func checkLocalHostUDPAddrs(addr ma.Multiaddr, ourUDPPorts map[string]bool) bool { - if !manet.IsIPLoopback(addr) { - return true +// filterLowPriorityAddresses removes addresses inplace for which we have a better alternative +// 1. If a /quic-v1 address is present, filter out /quic and /webtransport address on the same 2-tuple: +// QUIC v1 is preferred over the deprecated QUIC draft-29, and given the choice, we prefer using +// raw QUIC over using WebTransport. +// 2. If a /tcp address is present, filter out /ws or /wss addresses on the same 2-tuple: +// We prefer using raw TCP over using WebSocket. +func filterLowPriorityAddresses(addrs []ma.Multiaddr) []ma.Multiaddr { + // make a map of QUIC v1 and TCP AddrPorts. + quicV1Addr := make(map[netip.AddrPort]struct{}) + tcpAddr := make(map[netip.AddrPort]struct{}) + for _, a := range addrs { + switch { + case isProtocolAddr(a, ma.P_WEBTRANSPORT): + case isProtocolAddr(a, ma.P_QUIC_V1): + ap, err := addrPort(a, ma.P_UDP) + if err != nil { + continue + } + quicV1Addr[ap] = struct{}{} + case isProtocolAddr(a, ma.P_WS) || isProtocolAddr(a, ma.P_WSS): + case isProtocolAddr(a, ma.P_TCP): + ap, err := addrPort(a, ma.P_TCP) + if err != nil { + continue + } + tcpAddr[ap] = struct{}{} + } + } + + i := 0 + for _, a := range addrs { + switch { + case isProtocolAddr(a, ma.P_WEBTRANSPORT) || isProtocolAddr(a, ma.P_QUIC): + ap, err := addrPort(a, ma.P_UDP) + if err != nil { + break + } + if _, ok := quicV1Addr[ap]; ok { + continue + } + case isProtocolAddr(a, ma.P_WS) || isProtocolAddr(a, ma.P_WSS): + ap, err := addrPort(a, ma.P_TCP) + if err != nil { + break + } + if _, ok := tcpAddr[ap]; ok { + continue + } + } + addrs[i] = a + i++ } - if p, err := addr.ValueForProtocol(ma.P_UDP); err == nil { - return !ourUDPPorts[p] + return addrs[:i] +} + +// addrPort returns the ip and port for a. p should be either ma.P_TCP or ma.P_UDP. +// a must be an (ip, TCP) or (ip, udp) address. +func addrPort(a ma.Multiaddr, p int) (netip.AddrPort, error) { + ip, err := manet.ToIP(a) + if err != nil { + return netip.AddrPort{}, err + } + port, err := a.ValueForProtocol(p) + if err != nil { + return netip.AddrPort{}, err + } + pi, err := strconv.Atoi(port) + if err != nil { + return netip.AddrPort{}, err + } + addr, ok := netip.AddrFromSlice(ip) + if !ok { + return netip.AddrPort{}, fmt.Errorf("failed to parse IP %s", ip) } - return true + return netip.AddrPortFrom(addr, uint16(pi)), nil } diff --git a/p2p/net/swarm/swarm_dial_test.go b/p2p/net/swarm/swarm_dial_test.go index ce60701875..f22144ee8c 100644 --- a/p2p/net/swarm/swarm_dial_test.go +++ b/p2p/net/swarm/swarm_dial_test.go @@ -1,9 +1,11 @@ package swarm import ( + "bytes" "context" "crypto/rand" "net" + "sort" "testing" "time" @@ -14,11 +16,12 @@ import ( "github.com/libp2p/go-libp2p/core/test" "github.com/libp2p/go-libp2p/p2p/host/eventbus" "github.com/libp2p/go-libp2p/p2p/host/peerstore/pstoremem" - quic "github.com/libp2p/go-libp2p/p2p/transport/quic" + libp2pquic "github.com/libp2p/go-libp2p/p2p/transport/quic" "github.com/libp2p/go-libp2p/p2p/transport/quicreuse" "github.com/libp2p/go-libp2p/p2p/transport/tcp" "github.com/libp2p/go-libp2p/p2p/transport/websocket" - webtransport "github.com/libp2p/go-libp2p/p2p/transport/webtransport" + libp2pwebtransport "github.com/libp2p/go-libp2p/p2p/transport/webtransport" + "github.com/quic-go/quic-go" ma "github.com/multiformats/go-multiaddr" madns "github.com/multiformats/go-multiaddr-dns" @@ -135,6 +138,23 @@ func newTestSwarmWithResolver(t *testing.T, resolver *madns.Resolver) *Swarm { err = s.AddTransport(tpt) require.NoError(t, err) + connmgr, err := quicreuse.NewConnManager(quic.StatelessResetKey{}) + require.NoError(t, err) + quicTpt, err := libp2pquic.NewTransport(priv, connmgr, nil, nil, &network.NullResourceManager{}) + require.NoError(t, err) + err = s.AddTransport(quicTpt) + require.NoError(t, err) + + wtTpt, err := libp2pwebtransport.New(priv, nil, connmgr, nil, &network.NullResourceManager{}) + require.NoError(t, err) + err = s.AddTransport(wtTpt) + require.NoError(t, err) + + wsTpt, err := websocket.New(nil, &network.NullResourceManager{}) + require.NoError(t, err) + err = s.AddTransport(wsTpt) + require.NoError(t, err) + return s } @@ -242,35 +262,71 @@ func TestAddrResolutionRecursive(t *testing.T) { require.Contains(t, addrs2, addr1) } -func TestLocalHostWebTransportRemoved(t *testing.T) { - resolver, err := madns.NewResolver() - if err != nil { - t.Fatal(err) - } +func TestAddrsForDialFiltering(t *testing.T) { + q1 := ma.StringCast("/ip4/1.2.3.4/udp/1/quic") + q1v1 := ma.StringCast("/ip4/1.2.3.4/udp/1/quic-v1") + wt1 := ma.StringCast("/ip4/1.2.3.4/udp/1/quic-v1/webtransport/") - s := newTestSwarmWithResolver(t, resolver) - p, err := test.RandPeerID() - if err != nil { - t.Error(err) - } - reuse, err := quicreuse.NewConnManager([32]byte{}) - require.NoError(t, err) - defer reuse.Close() + q2 := ma.StringCast("/ip4/1.2.3.4/udp/2/quic") + q2v1 := ma.StringCast("/ip4/1.2.3.4/udp/2/quic-v1") + wt2 := ma.StringCast("/ip4/1.2.3.4/udp/2/quic-v1/webtransport/") - quicTr, err := quic.NewTransport(s.Peerstore().PrivKey(s.LocalPeer()), reuse, nil, nil, nil) - require.NoError(t, err) - require.NoError(t, s.AddTransport(quicTr)) + q3 := ma.StringCast("/ip4/1.2.3.4/udp/3/quic") - webtransportTr, err := webtransport.New(s.Peerstore().PrivKey(s.LocalPeer()), nil, reuse, nil, nil) - require.NoError(t, err) - s.AddTransport(webtransportTr) + t1 := ma.StringCast("/ip4/1.2.3.4/tcp/1") + ws1 := ma.StringCast("/ip4/1.2.3.4/tcp/1/ws") - err = s.AddListenAddr(ma.StringCast("/ip4/127.0.0.1/udp/10000/quic-v1/")) + resolver, err := madns.NewResolver(madns.WithDefaultResolver(&madns.MockResolver{})) require.NoError(t, err) + s := newTestSwarmWithResolver(t, resolver) + ourAddrs := s.ListenAddresses() + + testCases := []struct { + name string + input []ma.Multiaddr + output []ma.Multiaddr + }{ + { + name: "quic-filtered", + input: []ma.Multiaddr{q1, q1v1, q2, q2v1, q3}, + output: []ma.Multiaddr{q1v1, q2v1, q3}, + }, + { + name: "webtransport-filtered", + input: []ma.Multiaddr{q1, q1v1, wt1, wt2}, + output: []ma.Multiaddr{q1v1, wt2}, + }, + { + name: "all", + input: []ma.Multiaddr{q1, q1v1, wt1, q2, q2v1, wt2, t1, ws1}, + output: []ma.Multiaddr{q1v1, q2v1, t1}, + }, + { + name: "our-addrs-filtered", + input: append([]ma.Multiaddr{q1}, ourAddrs...), + output: []ma.Multiaddr{q1}, + }, + } + + ctx := context.Background() + p1 := test.RandPeerIDFatal(t) - res := s.filterKnownUndialables(p, []ma.Multiaddr{ma.StringCast("/ip4/127.0.0.1/udp/10000/quic-v1/webtransport")}) - if len(res) != 0 { - t.Errorf("failed to filter localhost webtransport address") + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s.Peerstore().ClearAddrs(p1) + s.Peerstore().AddAddrs(p1, tc.input, peerstore.PermanentAddrTTL) + result, err := s.addrsForDial(ctx, p1) + require.NoError(t, err) + sort.Slice(result, func(i, j int) bool { return bytes.Compare(result[i].Bytes(), result[j].Bytes()) < 0 }) + sort.Slice(tc.output, func(i, j int) bool { return bytes.Compare(tc.output[i].Bytes(), tc.output[j].Bytes()) < 0 }) + if len(result) != len(tc.output) { + t.Fatalf("output mismatch got: %s want: %s", result, tc.output) + } + for i := 0; i < len(result); i++ { + if !result[i].Equal(tc.output[i]) { + t.Fatalf("output mismatch got: %s want: %s", result, tc.output) + } + } + }) } - s.Close() }