Skip to content

Commit

Permalink
Removes QUIC draft 29 addrs if we have a QUIC v1 addr...
Browse files Browse the repository at this point in the history
before dialing. Also synchronizes the values for concurrent dials with
the default max inbound connection count.
  • Loading branch information
MarcoPolo committed Nov 16, 2022
1 parent 6a19be3 commit a969c6d
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 16 deletions.
4 changes: 2 additions & 2 deletions p2p/host/resource-manager/limit_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,8 @@ var DefaultLimits = ScalingLimitConfig{
},

PeerBaseLimit: BaseLimit{
ConnsInbound: 4,
ConnsOutbound: 8,
ConnsInbound: 6,
ConnsOutbound: 6,
Conns: 8,
StreamsInbound: 256,
StreamsOutbound: 512,
Expand Down
89 changes: 75 additions & 14 deletions p2p/net/swarm/swarm_dial.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/libp2p/go-libp2p/core/peer"
"github.com/libp2p/go-libp2p/core/peerstore"
"github.com/libp2p/go-libp2p/core/transport"
"github.com/lucas-clemente/quic-go"

ma "github.com/multiformats/go-multiaddr"
madns "github.com/multiformats/go-multiaddr-dns"
Expand Down Expand Up @@ -72,7 +73,7 @@ const ConcurrentFdDials = 160

// DefaultPerPeerRateLimit is the number of concurrent outbound dials to make
// per peer
var DefaultPerPeerRateLimit = 8
var DefaultPerPeerRateLimit = 6

// dialbackoff is a struct used to avoid over-dialing the same, dead peers.
// Whenever we totally time out on a peer (all three attempts), we add them
Expand Down Expand Up @@ -439,15 +440,17 @@ func (s *Swarm) filterKnownUndialables(p peer.ID, addrs []ma.Multiaddr) []ma.Mul
}
}

return maybeRemoveWebTransportAddrs(ma.FilterAddrs(addrs,
func(addr ma.Multiaddr) bool { return !ma.Contains(ourAddrs, addr) },
s.canDial,
// TODO: Consider allowing link-local addresses
func(addr ma.Multiaddr) bool { return !manet.IsIP6LinkLocal(addr) },
func(addr ma.Multiaddr) bool {
return s.gater == nil || s.gater.InterceptAddrDial(p, addr)
},
))
return maybeRemoveWebTransportAddrs(
maybeRemoveQUICDraft29(
ma.FilterAddrs(addrs,
func(addr ma.Multiaddr) bool { return !ma.Contains(ourAddrs, addr) },
s.canDial,
// TODO: Consider allowing link-local addresses
func(addr ma.Multiaddr) bool { return !manet.IsIP6LinkLocal(addr) },
func(addr ma.Multiaddr) bool {
return s.gater == nil || s.gater.InterceptAddrDial(p, addr)
},
)))
}

// limitedDial will start a dial to the given peer when
Expand Down Expand Up @@ -536,9 +539,32 @@ func isWebTransport(addr ma.Multiaddr) bool {
return err == nil
}

func isQUIC(addr ma.Multiaddr) bool {
_, err := addr.ValueForProtocol(ma.P_QUIC)
return err == nil && !isWebTransport(addr)
func quicVersion(addr ma.Multiaddr) (quic.VersionNumber, bool) {
found := false
foundWebTransport := false
var version quic.VersionNumber
ma.ForEach(addr, func(c ma.Component) bool {
switch c.Protocol().Code {
case ma.P_QUIC:
version = quic.VersionDraft29
found = true
return true
case ma.P_QUIC_V1:
version = quic.Version1
found = true
return true
case ma.P_WEBTRANSPORT:
version = quic.Version1
foundWebTransport = true
return false
default:
return true
}
})
if foundWebTransport {
return 0, false
}
return version, found
}

// If we have QUIC addresses, we don't want to dial WebTransport addresses.
Expand All @@ -548,7 +574,7 @@ func isQUIC(addr ma.Multiaddr) bool {
func maybeRemoveWebTransportAddrs(addrs []ma.Multiaddr) []ma.Multiaddr {
var hasQuic, hasWebTransport bool
for _, addr := range addrs {
if isQUIC(addr) {
if _, isQuic := quicVersion(addr); isQuic {
hasQuic = true
}
if isWebTransport(addr) {
Expand All @@ -568,3 +594,38 @@ func maybeRemoveWebTransportAddrs(addrs []ma.Multiaddr) []ma.Multiaddr {
}
return addrs[:c]
}

// If we have QUIC V1 addresses, we don't want to dial QUIC draft29 addresses.
// This is a similar hack to the above. If we add one more hack like this, let's
// define a `Filterer` interface like the `Resolver` interface that transports
// can optionally implement if they want to filter the multiaddrs.
//
// This mutates the input
func maybeRemoveQUICDraft29(addrs []ma.Multiaddr) []ma.Multiaddr {
var hasQuicV1, hasQuicDraft29 bool
for _, addr := range addrs {
v, isQuic := quicVersion(addr)
if !isQuic {
continue
}

if v == quic.Version1 {
hasQuicV1 = true
}
if v == quic.VersionDraft29 {
hasQuicDraft29 = true
}
}
if !hasQuicDraft29 || !hasQuicV1 {
return addrs
}
var c int
for _, addr := range addrs {
if v, isQuic := quicVersion(addr); isQuic && v == quic.VersionDraft29 {
continue
}
addrs[c] = addr
c++
}
return addrs[:c]
}
12 changes: 12 additions & 0 deletions p2p/net/swarm/swarm_dial_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,3 +204,15 @@ func TestRemoveWebTransportAddrs(t *testing.T) {
require.Equal(t, []ma.Multiaddr{quicAddr}, maybeRemoveWebTransportAddrs([]ma.Multiaddr{quicAddr, webtransportAddr}))
require.Equal(t, []ma.Multiaddr{webtransportAddr}, maybeRemoveWebTransportAddrs([]ma.Multiaddr{webtransportAddr}))
}

func TestRemoveQuicDraft29(t *testing.T) {
tcpAddr := ma.StringCast("/ip4/9.5.6.4/tcp/1234")
quicDraft29Addr := ma.StringCast("/ip4/1.2.3.4/udp/443/quic")
quicV1Addr := ma.StringCast("/ip4/1.2.3.4/udp/443/quic-v1")

require.Equal(t, []ma.Multiaddr{tcpAddr, quicV1Addr}, maybeRemoveQUICDraft29([]ma.Multiaddr{tcpAddr, quicV1Addr}))
require.Equal(t, []ma.Multiaddr{tcpAddr, quicDraft29Addr}, maybeRemoveQUICDraft29([]ma.Multiaddr{tcpAddr, quicDraft29Addr}))
require.Equal(t, []ma.Multiaddr{tcpAddr, quicV1Addr}, maybeRemoveQUICDraft29([]ma.Multiaddr{tcpAddr, quicDraft29Addr, quicV1Addr}))
require.Equal(t, []ma.Multiaddr{quicV1Addr}, maybeRemoveQUICDraft29([]ma.Multiaddr{quicV1Addr, quicDraft29Addr}))
require.Equal(t, []ma.Multiaddr{quicDraft29Addr}, maybeRemoveQUICDraft29([]ma.Multiaddr{quicDraft29Addr}))
}

0 comments on commit a969c6d

Please sign in to comment.