Skip to content

Commit cdacc08

Browse files
sukunrtsukunrt
committed
interim commit
1 parent ff7b64c commit cdacc08

File tree

4 files changed

+76
-188
lines changed

4 files changed

+76
-188
lines changed

‎p2p/host/basic/addrs_manager.go‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type observedAddrsManager interface {
3333
AddrsFor(local ma.Multiaddr) []ma.Multiaddr
3434

3535
Record(conn connMultiaddrs, observed ma.Multiaddr)
36-
removeConn(conn connMultiaddrs)
36+
RemoveConn(conn connMultiaddrs)
3737
Start()
3838
getNATType() (network.NATDeviceType, network.NATDeviceType)
3939
io.Closer
@@ -181,14 +181,12 @@ func (a *addrsManager) Close() {
181181
}
182182

183183
func (a *addrsManager) NetNotifee() network.Notifiee {
184-
// Updating addrs in sync provides the nice property that
185-
// host.Addrs() just after host.Network().Listen(x) will return x
186184
return &network.NotifyBundle{
187185
ListenF: func(network.Network, ma.Multiaddr) { a.updateAddrsSync() },
188186
ListenCloseF: func(network.Network, ma.Multiaddr) { a.updateAddrsSync() },
189187
DisconnectedF: func(_ network.Network, conn network.Conn) {
190188
if a.observedAddrsManager != nil {
191-
a.observedAddrsManager.removeConn(conn)
189+
a.observedAddrsManager.RemoveConn(conn)
192190
}
193191
},
194192
}
@@ -511,7 +509,12 @@ func (a *addrsManager) getLocalAddrs() []ma.Multiaddr {
511509

512510
finalAddrs := make([]ma.Multiaddr, 0, 8)
513511
finalAddrs = a.appendPrimaryInterfaceAddrs(finalAddrs, listenAddrs)
514-
finalAddrs = a.appendNATAddrs(finalAddrs, listenAddrs, a.interfaceAddrs.All())
512+
if a.natManager != nil {
513+
finalAddrs = a.appendNATAddrs(finalAddrs, listenAddrs)
514+
}
515+
if a.observedAddrsManager != nil {
516+
finalAddrs = a.appendObservedAddrs(finalAddrs, listenAddrs, a.interfaceAddrs.All())
517+
}
515518

516519
// Remove "/p2p-circuit" addresses from the list.
517520
// The p2p-circuit listener reports its address as just /p2p-circuit. This is
@@ -551,42 +554,33 @@ func (a *addrsManager) appendPrimaryInterfaceAddrs(dst []ma.Multiaddr, listenAdd
551554
// Inferring WebTransport from QUIC depends on the observed address manager.
552555
//
553556
// TODO: Merge the natmgr and identify.ObservedAddrManager in to one NatMapper module.
554-
func (a *addrsManager) appendNATAddrs(dst []ma.Multiaddr, listenAddrs []ma.Multiaddr, ifaceAddrs []ma.Multiaddr) []ma.Multiaddr {
557+
func (a *addrsManager) appendNATAddrs(dst []ma.Multiaddr, listenAddrs []ma.Multiaddr) []ma.Multiaddr {
555558
for _, listenAddr := range listenAddrs {
556-
var natAddr ma.Multiaddr
557-
if a.natManager != nil {
558-
natAddr = a.natManager.GetMapping(listenAddr)
559-
}
559+
natAddr := a.natManager.GetMapping(listenAddr)
560560
if natAddr != nil {
561561
dst = append(dst, natAddr)
562562
}
563-
// This is !Public as opposed to IsPrivate intentionally.
564-
// Public is a more restrictive classification in some cases, like IPv6 addresses which only
565-
// consider unicast IPv6 addresses allocated so far as public(2000::/3).
566-
if a.observedAddrsManager != nil && !manet.IsPublicAddr(natAddr) {
567-
// nat reported non public addr(maybe CGNAT?), add observed addrs too.
568-
dst = a.appendObservedAddrs(dst, listenAddr, ifaceAddrs)
569-
}
570563
}
571564
return dst
572565
}
573566

574-
func (a *addrsManager) appendObservedAddrs(dst []ma.Multiaddr, listenAddr ma.Multiaddr, ifaceAddrs []ma.Multiaddr) []ma.Multiaddr {
575-
// Add it for the listenAddr first.
567+
func (a *addrsManager) appendObservedAddrs(dst []ma.Multiaddr, listenAddrs, ifaceAddrs []ma.Multiaddr) []ma.Multiaddr {
568+
// Add it for all the listenAddr first.
576569
// listenAddr maybe unspecified. That's okay as connections on UDP transports
577570
// will have the unspecified address as the local address.
578-
obsAddrs := a.observedAddrsManager.AddrsFor(listenAddr)
579-
dst = append(dst, obsAddrs...)
571+
for _, la := range listenAddrs {
572+
obsAddrs := a.observedAddrsManager.AddrsFor(la)
573+
dst = append(dst, obsAddrs...)
574+
}
580575

581576
// if it can be resolved into more addresses, add them too
582-
resolved, err := manet.ResolveUnspecifiedAddress(listenAddr, ifaceAddrs)
577+
resolved, err := manet.ResolveUnspecifiedAddresses(listenAddrs, ifaceAddrs)
583578
if err != nil {
584-
log.Warnf("failed to resolve listen addr %s, %s: %s", listenAddr, ifaceAddrs, err)
579+
log.Warnf("failed to resolve listen addr %s, %s: %s", listenAddrs, ifaceAddrs, err)
585580
return dst
586581
}
587582
for _, addr := range resolved {
588-
obsAddrs = a.observedAddrsManager.AddrsFor(addr)
589-
dst = append(dst, obsAddrs...)
583+
dst = append(dst, a.observedAddrsManager.AddrsFor(addr)...)
590584
}
591585
return dst
592586
}

‎p2p/host/basic/addrs_manager_test.go‎

Lines changed: 6 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -15,129 +15,11 @@ import (
1515
"github.com/libp2p/go-libp2p/p2p/protocol/autonatv2"
1616
ma "github.com/multiformats/go-multiaddr"
1717
"github.com/multiformats/go-multiaddr/matest"
18-
manet "github.com/multiformats/go-multiaddr/net"
1918
"github.com/prometheus/client_golang/prometheus"
2019
"github.com/stretchr/testify/assert"
2120
"github.com/stretchr/testify/require"
2221
)
2322

24-
func TestAppendNATAddrs(t *testing.T) {
25-
if1, if2 := ma.StringCast("/ip4/192.168.0.100"), ma.StringCast("/ip4/1.1.1.1")
26-
ifaceAddrs := []ma.Multiaddr{if1, if2}
27-
tcpListenAddr, udpListenAddr := ma.StringCast("/ip4/0.0.0.0/tcp/1"), ma.StringCast("/ip4/0.0.0.0/udp/2/quic-v1")
28-
cases := []struct {
29-
Name string
30-
Listen ma.Multiaddr
31-
Nat ma.Multiaddr
32-
ObsAddrFunc func(ma.Multiaddr) []ma.Multiaddr
33-
Expected []ma.Multiaddr
34-
}{
35-
{
36-
Name: "nat map success",
37-
// nat mapping success, obsaddress ignored
38-
Listen: ma.StringCast("/ip4/0.0.0.0/udp/1/quic-v1"),
39-
Nat: ma.StringCast("/ip4/1.1.1.1/udp/10/quic-v1"),
40-
ObsAddrFunc: func(_ ma.Multiaddr) []ma.Multiaddr {
41-
return []ma.Multiaddr{ma.StringCast("/ip4/2.2.2.2/udp/100/quic-v1")}
42-
},
43-
Expected: []ma.Multiaddr{ma.StringCast("/ip4/1.1.1.1/udp/10/quic-v1")},
44-
},
45-
{
46-
Name: "nat map failure",
47-
// nat mapping fails, obs addresses added
48-
Listen: ma.StringCast("/ip4/0.0.0.0/tcp/1"),
49-
Nat: nil,
50-
ObsAddrFunc: func(a ma.Multiaddr) []ma.Multiaddr {
51-
ipC, _ := ma.SplitFirst(a)
52-
ip := ipC.Multiaddr()
53-
switch {
54-
case ip.Equal(if1):
55-
return []ma.Multiaddr{ma.StringCast("/ip4/2.2.2.2/tcp/100")}
56-
case ip.Equal(if2):
57-
return []ma.Multiaddr{ma.StringCast("/ip4/3.3.3.3/tcp/100")}
58-
default:
59-
return []ma.Multiaddr{}
60-
}
61-
},
62-
Expected: []ma.Multiaddr{ma.StringCast("/ip4/2.2.2.2/tcp/100"), ma.StringCast("/ip4/3.3.3.3/tcp/100")},
63-
},
64-
{
65-
Name: "if addrs ignored if not listening on unspecified",
66-
// nat mapping fails, obs addresses added
67-
Listen: ma.StringCast("/ip4/192.168.1.1/tcp/1"),
68-
Nat: nil,
69-
ObsAddrFunc: func(a ma.Multiaddr) []ma.Multiaddr {
70-
ipC, _ := ma.SplitFirst(a)
71-
ip := ipC.Multiaddr()
72-
switch {
73-
case ip.Equal(if1):
74-
return []ma.Multiaddr{ma.StringCast("/ip4/2.2.2.2/tcp/100")}
75-
case ip.Equal(if2):
76-
return []ma.Multiaddr{ma.StringCast("/ip4/3.3.3.3/tcp/100")}
77-
case ip.Equal(ma.StringCast("/ip4/192.168.1.1")):
78-
return []ma.Multiaddr{ma.StringCast("/ip4/4.4.4.4/tcp/100")}
79-
default:
80-
return []ma.Multiaddr{}
81-
}
82-
},
83-
Expected: []ma.Multiaddr{ma.StringCast("/ip4/4.4.4.4/tcp/100")},
84-
},
85-
{
86-
Name: "nat map success but CGNAT",
87-
// nat addr added, obs address added with nat provided port
88-
Listen: tcpListenAddr,
89-
Nat: ma.StringCast("/ip4/100.100.1.1/tcp/100"),
90-
ObsAddrFunc: func(a ma.Multiaddr) []ma.Multiaddr {
91-
ipC, _ := ma.SplitFirst(a)
92-
ip := ipC.Multiaddr()
93-
if ip.Equal(if1) {
94-
return []ma.Multiaddr{ma.StringCast("/ip4/2.2.2.2/tcp/20")}
95-
}
96-
return []ma.Multiaddr{ma.StringCast("/ip4/3.3.3.3/tcp/30")}
97-
},
98-
Expected: []ma.Multiaddr{
99-
ma.StringCast("/ip4/100.100.1.1/tcp/100"),
100-
ma.StringCast("/ip4/2.2.2.2/tcp/20"),
101-
ma.StringCast("/ip4/3.3.3.3/tcp/30"),
102-
},
103-
},
104-
{
105-
Name: "uses unspecified address for obs address",
106-
// observed address manager should be queries with both specified and unspecified addresses
107-
// udp observed addresses are mapped to unspecified addresses
108-
Listen: udpListenAddr,
109-
Nat: nil,
110-
ObsAddrFunc: func(a ma.Multiaddr) []ma.Multiaddr {
111-
if manet.IsIPUnspecified(a) {
112-
return []ma.Multiaddr{ma.StringCast("/ip4/3.3.3.3/udp/20/quic-v1")}
113-
}
114-
return []ma.Multiaddr{ma.StringCast("/ip4/2.2.2.2/udp/20/quic-v1")}
115-
},
116-
Expected: []ma.Multiaddr{
117-
ma.StringCast("/ip4/2.2.2.2/udp/20/quic-v1"),
118-
ma.StringCast("/ip4/3.3.3.3/udp/20/quic-v1"),
119-
},
120-
},
121-
}
122-
for _, tc := range cases {
123-
t.Run(tc.Name, func(t *testing.T) {
124-
as := &addrsManager{
125-
natManager: &mockNatManager{
126-
GetMappingFunc: func(_ ma.Multiaddr) ma.Multiaddr {
127-
return tc.Nat
128-
},
129-
},
130-
observedAddrsManager: &mockObservedAddrs{
131-
AddrsForFunc: tc.ObsAddrFunc,
132-
},
133-
}
134-
res := as.appendNATAddrs(nil, []ma.Multiaddr{tc.Listen}, ifaceAddrs)
135-
res = ma.Unique(res)
136-
require.ElementsMatch(t, tc.Expected, res, "%s\n%s", tc.Expected, res)
137-
})
138-
}
139-
}
140-
14123
type mockNatManager struct {
14224
GetMappingFunc func(addr ma.Multiaddr) ma.Multiaddr
14325
}
@@ -176,7 +58,7 @@ func (m *mockObservedAddrs) getNATType() (network.NATDeviceType, network.NATDevi
17658
}
17759

17860
// removeConn implements observedAddrsManager.
179-
func (m *mockObservedAddrs) removeConn(_ connMultiaddrs) {}
61+
func (m *mockObservedAddrs) RemoveConn(_ connMultiaddrs) {}
18062

18163
func (m *mockObservedAddrs) Addrs(int) []ma.Multiaddr { return m.AddrsFunc() }
18264

@@ -259,6 +141,7 @@ func TestAddrsManager(t *testing.T) {
259141
lhtcp := ma.StringCast("/ip4/127.0.0.1/tcp/1")
260142

261143
publicQUIC := ma.StringCast("/ip4/1.2.3.4/udp/1/quic-v1")
144+
publicQUIC2 := ma.StringCast("/ip4/1.2.3.4/udp/2/quic-v1")
262145
publicTCP := ma.StringCast("/ip4/1.2.3.4/tcp/1")
263146

264147
t.Run("only nat", func(t *testing.T) {
@@ -296,13 +179,16 @@ func TestAddrsManager(t *testing.T) {
296179
if _, err := addr.ValueForProtocol(ma.P_TCP); err == nil {
297180
return []ma.Multiaddr{publicTCP}
298181
}
182+
if _, err := addr.ValueForProtocol(ma.P_UDP); err == nil {
183+
return []ma.Multiaddr{publicQUIC2}
184+
}
299185
return nil
300186
},
301187
},
302188
ListenAddrs: func() []ma.Multiaddr { return []ma.Multiaddr{lhquic, lhtcp} },
303189
})
304190
require.EventuallyWithT(t, func(collect *assert.CollectT) {
305-
expected := []ma.Multiaddr{lhquic, lhtcp, publicQUIC, publicTCP}
191+
expected := []ma.Multiaddr{lhquic, lhtcp, publicQUIC, publicTCP, publicQUIC2}
306192
assert.ElementsMatch(collect, am.Addrs(), expected, "%s\n%s", am.Addrs(), expected)
307193
}, 5*time.Second, 50*time.Millisecond)
308194
})

‎p2p/host/basic/obsaddr.go‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,8 @@ func (o *ObservedAddrsManager) getTopExternalAddrs(localTWStr string, minObserve
242242
// keep the address list stable by using the lexicographically smaller address
243243
return a.ObservedTWAddr.Compare(b.ObservedTWAddr)
244244
})
245+
// TODO(sukunrt): Improve this logic. Return only if the addresses have a
246+
// threshold fraction of the maximum observations
245247
n := len(observerSets)
246248
if n > maxExternalThinWaistAddrsPerLocalAddr {
247249
n = maxExternalThinWaistAddrsPerLocalAddr
@@ -277,8 +279,12 @@ func (o *ObservedAddrsManager) worker() {
277279
}
278280

279281
func isRelayedAddress(a ma.Multiaddr) bool {
280-
_, err := a.ValueForProtocol(ma.P_CIRCUIT)
281-
return err == nil
282+
for _, c := range a {
283+
if c.Code() == ma.P_CIRCUIT {
284+
return true
285+
}
286+
}
287+
return false
282288
}
283289

284290
func (o *ObservedAddrsManager) shouldRecordObservation(conn connMultiaddrs, observed ma.Multiaddr) (shouldRecord bool, localTW thinWaist, observedTW thinWaist) {
@@ -309,7 +315,11 @@ func (o *ObservedAddrsManager) shouldRecordObservation(conn connMultiaddrs, obse
309315

310316
listenAddrs := o.listenAddrs()
311317
for i, a := range listenAddrs {
312-
tw, _ := thinWaistForm(a) // tw is empty if zero is non nil
318+
tw, err := thinWaistForm(a)
319+
if err != nil {
320+
listenAddrs[i] = nil
321+
continue
322+
}
313323
listenAddrs[i] = tw.TW
314324
}
315325

@@ -401,7 +411,7 @@ func (o *ObservedAddrsManager) addExternalAddrsUnlocked(observedTWAddr ma.Multia
401411
s.ObservedBy[observer]++
402412
}
403413

404-
func (o *ObservedAddrsManager) removeConn(conn connMultiaddrs) {
414+
func (o *ObservedAddrsManager) RemoveConn(conn connMultiaddrs) {
405415
if conn == nil {
406416
return
407417
}
@@ -414,8 +424,6 @@ func (o *ObservedAddrsManager) removeConn(conn connMultiaddrs) {
414424
}
415425
delete(o.connObservedTWAddrs, conn)
416426

417-
// normalize before obtaining the thinWaist so that we are always dealing
418-
// with the normalized form of the address
419427
localTW, err := thinWaistForm(conn.LocalMultiaddr())
420428
if err != nil {
421429
return

0 commit comments

Comments
 (0)