Skip to content

Commit 686a32a

Browse files
sukunrtsukunrt
authored
swarm: cleanup address filtering logic (#2333)
1 parent d46406c commit 686a32a

File tree

4 files changed

+187
-115
lines changed

4 files changed

+187
-115
lines changed

‎p2p/net/swarm/dial_ranker.go‎

Lines changed: 4 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package swarm
22

33
import (
4-
"net/netip"
54
"sort"
65
"strconv"
76
"time"
@@ -53,16 +52,8 @@ func noDelayRanker(addrs []ma.Multiaddr) []network.AddrDelay {
5352
// The correct solution is to detect this situation, and not attempt to dial IPv6 addresses at all.
5453
// IPv6 blackhole detection is tracked in https://github.com/libp2p/go-libp2p/issues/1605.
5554
//
56-
// Within each group (private, public IPv4, public IPv6, relay addresses) we apply the following logic:
57-
//
58-
// 1. Filter out addresses we don't want to dial:
59-
// 1. If a /quic-v1 address is present, filter out /quic and /webtransport address on the same 2-tuple:
60-
// QUIC v1 is preferred over the deprecated QUIC draft-29, and given the choice, we prefer using
61-
// raw QUIC over using WebTransport.
62-
// 2. If a /tcp address is present, filter out /ws or /wss addresses on the same 2-tuple:
63-
// We prefer using raw TCP over using WebSocket.
64-
//
65-
// 2. Rank addresses:
55+
// Within each group (private, public IPv4, public IPv6, relay addresses) we apply the following
56+
// ranking logic:
6657
//
6758
// 1. If two QUIC addresses are present, dial the QUIC address with the lowest port first:
6859
// 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 {
9990
// addresses relative to direct addresses
10091
func getAddrDelay(addrs []ma.Multiaddr, tcpDelay time.Duration, quicDelay time.Duration,
10192
offset time.Duration) []network.AddrDelay {
102-
103-
// First make a map of QUICV1 and TCP AddrPorts.
104-
quicV1Addr := make(map[netip.AddrPort]struct{})
105-
tcpAddr := make(map[netip.AddrPort]struct{})
106-
for _, a := range addrs {
107-
switch {
108-
case isProtocolAddr(a, ma.P_WEBTRANSPORT):
109-
case isProtocolAddr(a, ma.P_QUIC_V1):
110-
quicV1Addr[addrPort(a, ma.P_UDP)] = struct{}{}
111-
case isProtocolAddr(a, ma.P_WS) || isProtocolAddr(a, ma.P_WSS):
112-
case isProtocolAddr(a, ma.P_TCP):
113-
tcpAddr[addrPort(a, ma.P_TCP)] = struct{}{}
114-
}
115-
}
116-
117-
// Filter addresses we are sure we don't want to dial
118-
selectedAddrs := addrs
119-
i := 0
120-
for _, a := range addrs {
121-
switch {
122-
// If a QUICDraft29 or webtransport address is reachable, QUIC-v1 will also be reachable. So we
123-
// drop the QUICDraft29 or webtransport address
124-
// We prefer QUIC-v1 over the older QUIC-draft29 address.
125-
// We prefer QUIC-v1 over webtransport as it is more performant.
126-
case isProtocolAddr(a, ma.P_WEBTRANSPORT) || isProtocolAddr(a, ma.P_QUIC):
127-
if _, ok := quicV1Addr[addrPort(a, ma.P_UDP)]; ok {
128-
continue
129-
}
130-
// If a ws address is reachable, TCP will also be reachable and it'll be more performant
131-
case isProtocolAddr(a, ma.P_WS) || isProtocolAddr(a, ma.P_WSS):
132-
if _, ok := tcpAddr[addrPort(a, ma.P_TCP)]; ok {
133-
continue
134-
}
135-
}
136-
selectedAddrs[i] = a
137-
i++
138-
}
139-
selectedAddrs = selectedAddrs[:i]
140-
sort.Slice(selectedAddrs, func(i, j int) bool { return score(selectedAddrs[i]) < score(selectedAddrs[j]) })
93+
sort.Slice(addrs, func(i, j int) bool { return score(addrs[i]) < score(addrs[j]) })
14194

14295
res := make([]network.AddrDelay, 0, len(addrs))
14396
quicCount := 0
144-
for _, a := range selectedAddrs {
97+
for _, a := range addrs {
14598
delay := offset
14699
switch {
147100
case isProtocolAddr(a, ma.P_QUIC) || isProtocolAddr(a, ma.P_QUIC_V1):
@@ -192,16 +145,6 @@ func score(a ma.Multiaddr) int {
192145
return (1 << 30)
193146
}
194147

195-
// addrPort returns the ip and port for a. p should be either ma.P_TCP or ma.P_UDP.
196-
// a must be an (ip, TCP) or (ip, udp) address.
197-
func addrPort(a ma.Multiaddr, p int) netip.AddrPort {
198-
ip, _ := manet.ToIP(a)
199-
port, _ := a.ValueForProtocol(p)
200-
pi, _ := strconv.Atoi(port)
201-
addr, _ := netip.AddrFromSlice(ip)
202-
return netip.AddrPortFrom(addr, uint16(pi))
203-
}
204-
205148
func isProtocolAddr(a ma.Multiaddr, p int) bool {
206149
found := false
207150
ma.ForEach(a, func(c ma.Component) bool {

‎p2p/net/swarm/dial_ranker_test.go‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,26 @@ func TestNoDelayRanker(t *testing.T) {
2828
q3 := ma.StringCast("/ip4/1.2.3.4/udp/3/quic")
2929
q3v1 := ma.StringCast("/ip4/1.2.3.4/udp/3/quic-v1")
3030
q4 := ma.StringCast("/ip4/1.2.3.4/udp/4/quic")
31+
t1 := ma.StringCast("/ip4/1.2.3.5/tcp/1/")
3132

3233
testCase := []struct {
3334
name string
3435
addrs []ma.Multiaddr
3536
output []network.AddrDelay
3637
}{
3738
{
38-
name: "quic+webtransport filtered when quicv1",
39-
addrs: []ma.Multiaddr{q1, q2, q3, q4, q1v1, q2v1, q3v1, wt1},
39+
name: "quic-ranking",
40+
addrs: []ma.Multiaddr{q1, q2, q3, q4, q1v1, q2v1, q3v1, wt1, t1},
4041
output: []network.AddrDelay{
42+
{Addr: q1, Delay: 0},
43+
{Addr: q2, Delay: 0},
44+
{Addr: q3, Delay: 0},
45+
{Addr: q4, Delay: 0},
4146
{Addr: q1v1, Delay: 0},
4247
{Addr: q2v1, Delay: 0},
4348
{Addr: q3v1, Delay: 0},
44-
{Addr: q4, Delay: 0},
49+
{Addr: wt1, Delay: 0},
50+
{Addr: t1, Delay: 0},
4551
},
4652
},
4753
}
@@ -103,13 +109,17 @@ func TestDelayRankerQUICDelay(t *testing.T) {
103109
},
104110
},
105111
{
106-
name: "quic+webtransport filtered when quicv1",
112+
name: "quic-quic-v1-webtransport",
107113
addrs: []ma.Multiaddr{q1, q2, q3, q4, q1v1, q2v1, q3v1, wt1},
108114
output: []network.AddrDelay{
109115
{Addr: q1v1, Delay: 0},
116+
{Addr: q1, Delay: PublicQUICDelay},
117+
{Addr: q2, Delay: PublicQUICDelay},
118+
{Addr: q3, Delay: PublicQUICDelay},
119+
{Addr: q4, Delay: PublicQUICDelay},
110120
{Addr: q2v1, Delay: PublicQUICDelay},
111121
{Addr: q3v1, Delay: PublicQUICDelay},
112-
{Addr: q4, Delay: PublicQUICDelay},
122+
{Addr: wt1, Delay: PublicQUICDelay},
113123
},
114124
},
115125
{

‎p2p/net/swarm/swarm_dial.go‎

Lines changed: 86 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"net/netip"
8+
"strconv"
79
"sync"
810
"time"
911

@@ -433,8 +435,9 @@ func (s *Swarm) nonProxyAddr(addr ma.Multiaddr) bool {
433435
// filterKnownUndialables takes a list of multiaddrs, and removes those
434436
// that we definitely don't want to dial: addresses configured to be blocked,
435437
// IPv6 link-local addresses, addresses without a dial-capable transport,
436-
// and addresses that we know to be our own.
437-
// This is an optimization to avoid wasting time on dials that we know are going to fail.
438+
// addresses that we know to be our own, and addresses with a better tranport
439+
// available. This is an optimization to avoid wasting time on dials that we
440+
// know are going to fail or for which we have a better alternative.
438441
func (s *Swarm) filterKnownUndialables(p peer.ID, addrs []ma.Multiaddr) []ma.Multiaddr {
439442
lisAddrs, _ := s.InterfaceListenAddresses()
440443
var ourAddrs []ma.Multiaddr
@@ -448,21 +451,17 @@ func (s *Swarm) filterKnownUndialables(p peer.ID, addrs []ma.Multiaddr) []ma.Mul
448451
})
449452
}
450453

451-
// Make a map of udp ports we are listening on to filter peers web transport addresses
452-
ourLocalHostUDPPorts := make(map[string]bool, 2)
453-
for _, a := range ourAddrs {
454-
if !manet.IsIPLoopback(a) {
455-
continue
456-
}
457-
if p, err := a.ValueForProtocol(ma.P_UDP); err == nil {
458-
ourLocalHostUDPPorts[p] = true
459-
}
460-
}
454+
// The order of these two filters is important. If we can only dial /webtransport,
455+
// we don't want to filter /webtransport addresses out because the peer had a /quic-v1
456+
// address
457+
458+
// filter addresses we cannot dial
459+
addrs = ma.FilterAddrs(addrs, s.canDial)
460+
// filter low priority addresses among the addresses we can dial
461+
addrs = filterLowPriorityAddresses(addrs)
461462

462463
return ma.FilterAddrs(addrs,
463464
func(addr ma.Multiaddr) bool { return !ma.Contains(ourAddrs, addr) },
464-
func(addr ma.Multiaddr) bool { return checkLocalHostUDPAddrs(addr, ourLocalHostUDPPorts) },
465-
s.canDial,
466465
// TODO: Consider allowing link-local addresses
467466
func(addr ma.Multiaddr) bool { return !manet.IsIP6LinkLocal(addr) },
468467
func(addr ma.Multiaddr) bool {
@@ -559,15 +558,79 @@ func isRelayAddr(addr ma.Multiaddr) bool {
559558
return err == nil
560559
}
561560

562-
// checkLocalHostUDPAddrs returns false for addresses that have the same localhost port
563-
// as the one we are listening on
564-
// This is useful for filtering out peer's localhost webtransport addresses.
565-
func checkLocalHostUDPAddrs(addr ma.Multiaddr, ourUDPPorts map[string]bool) bool {
566-
if !manet.IsIPLoopback(addr) {
567-
return true
561+
// filterLowPriorityAddresses removes addresses inplace for which we have a better alternative
562+
// 1. If a /quic-v1 address is present, filter out /quic and /webtransport address on the same 2-tuple:
563+
// QUIC v1 is preferred over the deprecated QUIC draft-29, and given the choice, we prefer using
564+
// raw QUIC over using WebTransport.
565+
// 2. If a /tcp address is present, filter out /ws or /wss addresses on the same 2-tuple:
566+
// We prefer using raw TCP over using WebSocket.
567+
func filterLowPriorityAddresses(addrs []ma.Multiaddr) []ma.Multiaddr {
568+
// make a map of QUIC v1 and TCP AddrPorts.
569+
quicV1Addr := make(map[netip.AddrPort]struct{})
570+
tcpAddr := make(map[netip.AddrPort]struct{})
571+
for _, a := range addrs {
572+
switch {
573+
case isProtocolAddr(a, ma.P_WEBTRANSPORT):
574+
case isProtocolAddr(a, ma.P_QUIC_V1):
575+
ap, err := addrPort(a, ma.P_UDP)
576+
if err != nil {
577+
continue
578+
}
579+
quicV1Addr[ap] = struct{}{}
580+
case isProtocolAddr(a, ma.P_WS) || isProtocolAddr(a, ma.P_WSS):
581+
case isProtocolAddr(a, ma.P_TCP):
582+
ap, err := addrPort(a, ma.P_TCP)
583+
if err != nil {
584+
continue
585+
}
586+
tcpAddr[ap] = struct{}{}
587+
}
568588
}
569-
if p, err := addr.ValueForProtocol(ma.P_UDP); err == nil {
570-
return !ourUDPPorts[p]
589+
590+
i := 0
591+
for _, a := range addrs {
592+
switch {
593+
case isProtocolAddr(a, ma.P_WEBTRANSPORT) || isProtocolAddr(a, ma.P_QUIC):
594+
ap, err := addrPort(a, ma.P_UDP)
595+
if err != nil {
596+
break
597+
}
598+
if _, ok := quicV1Addr[ap]; ok {
599+
continue
600+
}
601+
case isProtocolAddr(a, ma.P_WS) || isProtocolAddr(a, ma.P_WSS):
602+
ap, err := addrPort(a, ma.P_TCP)
603+
if err != nil {
604+
break
605+
}
606+
if _, ok := tcpAddr[ap]; ok {
607+
continue
608+
}
609+
}
610+
addrs[i] = a
611+
i++
612+
}
613+
return addrs[:i]
614+
}
615+
616+
// addrPort returns the ip and port for a. p should be either ma.P_TCP or ma.P_UDP.
617+
// a must be an (ip, TCP) or (ip, udp) address.
618+
func addrPort(a ma.Multiaddr, p int) (netip.AddrPort, error) {
619+
ip, err := manet.ToIP(a)
620+
if err != nil {
621+
return netip.AddrPort{}, err
622+
}
623+
port, err := a.ValueForProtocol(p)
624+
if err != nil {
625+
return netip.AddrPort{}, err
626+
}
627+
pi, err := strconv.Atoi(port)
628+
if err != nil {
629+
return netip.AddrPort{}, err
630+
}
631+
addr, ok := netip.AddrFromSlice(ip)
632+
if !ok {
633+
return netip.AddrPort{}, fmt.Errorf("failed to parse IP %s", ip)
571634
}
572-
return true
635+
return netip.AddrPortFrom(addr, uint16(pi)), nil
573636
}

0 commit comments

Comments
 (0)