Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: quic,webtransport: enable both quic-draft29 and quic-v1 addrs on quic. only quic-v1 on webtransport #1881

Merged
merged 19 commits into from
Nov 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/transport/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ type Listener interface {
Accept() (CapableConn, error)
Close() error
Addr() net.Addr
Multiaddr() ma.Multiaddr
Multiaddrs() []ma.Multiaddr
}

// TransportNetwork is an inet.Network with methods for managing transports.
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ require (
github.com/minio/sha256-simd v1.0.0
github.com/mr-tron/base58 v1.2.0
github.com/multiformats/go-base32 v0.1.0
github.com/multiformats/go-multiaddr v0.7.0
github.com/multiformats/go-multiaddr v0.8.0
github.com/multiformats/go-multiaddr-dns v0.3.1
github.com/multiformats/go-multiaddr-fmt v0.1.0
github.com/multiformats/go-multibase v0.1.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,8 @@ github.com/multiformats/go-base36 v0.1.0 h1:JR6TyF7JjGd3m6FbLU2cOxhC0Li8z8dLNGQ8
github.com/multiformats/go-base36 v0.1.0/go.mod h1:kFGE83c6s80PklsHO9sRn2NCoffoRdUUOENyW/Vv6sM=
github.com/multiformats/go-multiaddr v0.1.1/go.mod h1:aMKBKNEYmzmDmxfX88/vz+J5IU55txyt0p4aiWVohjo=
github.com/multiformats/go-multiaddr v0.2.0/go.mod h1:0nO36NvPpyV4QzvTLi/lafl2y95ncPj0vFwVF6k6wJ4=
github.com/multiformats/go-multiaddr v0.7.0 h1:gskHcdaCyPtp9XskVwtvEeQOG465sCohbQIirSyqxrc=
github.com/multiformats/go-multiaddr v0.7.0/go.mod h1:Fs50eBDWvZu+l3/9S6xAE7ZYj6yhxlvaVZjakWN7xRs=
github.com/multiformats/go-multiaddr v0.8.0 h1:aqjksEcqK+iD/Foe1RRFsGZh8+XFiGo7FgUCZlpv3LU=
github.com/multiformats/go-multiaddr v0.8.0/go.mod h1:Fs50eBDWvZu+l3/9S6xAE7ZYj6yhxlvaVZjakWN7xRs=
github.com/multiformats/go-multiaddr-dns v0.3.1 h1:QgQgR+LQVt3NPTjbrLLpsaT2ufAA2y0Mkk+QRVJbW3A=
github.com/multiformats/go-multiaddr-dns v0.3.1/go.mod h1:G/245BRQ6FJGmryJCrOuTdB37AMA5AMOVuO6NY3JwTk=
github.com/multiformats/go-multiaddr-fmt v0.1.0 h1:WLEFClPycPkp4fnIzoFoV9FVd49/eQsuaL3/CWe167E=
Expand Down
6 changes: 3 additions & 3 deletions p2p/host/autonat/dialpolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ func (l *mockL) Accept() (transport.CapableConn, error) {
<-l.ctx.Done()
return nil, errors.New("expected in mocked test")
}
func (l *mockL) Close() error { return nil }
func (l *mockL) Addr() net.Addr { return nil }
func (l *mockL) Multiaddr() multiaddr.Multiaddr { return l.addr }
func (l *mockL) Close() error { return nil }
func (l *mockL) Addr() net.Addr { return nil }
func (l *mockL) Multiaddrs() []multiaddr.Multiaddr { return []multiaddr.Multiaddr{l.addr} }

func TestSkipDial(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
Expand Down
8 changes: 7 additions & 1 deletion p2p/net/swarm/swarm.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,14 @@ func (s *Swarm) close() {
s.transports.m = nil
s.transports.Unlock()

var wg sync.WaitGroup
// Dedup transports that may be listening on multiple protocols
transportsToClose := make(map[transport.Transport]struct{}, len(transports))
for _, t := range transports {
transportsToClose[t] = struct{}{}
}

var wg sync.WaitGroup
for t := range transportsToClose {
if closer, ok := t.(io.Closer); ok {
wg.Add(1)
go func(c io.Closer) {
Expand Down
4 changes: 2 additions & 2 deletions p2p/net/swarm/swarm_addr.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ func (s *Swarm) ListenAddresses() []ma.Multiaddr {
}

func (s *Swarm) listenAddressesNoLock() []ma.Multiaddr {
addrs := make([]ma.Multiaddr, 0, len(s.listeners.m))
addrs := make([]ma.Multiaddr, 0, len(s.listeners.m)+10) // A bit extra so we may avoid an extra allocation in the for loop below.
for l := range s.listeners.m {
addrs = append(addrs, l.Multiaddr())
addrs = append(addrs, l.Multiaddrs()...)
}
return addrs
}
Expand Down
2 changes: 1 addition & 1 deletion p2p/net/swarm/swarm_addr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func TestDialAddressSelection(t *testing.T) {
require.Equal(t, tcpTr, s.TransportForDialing(ma.StringCast("/ip4/127.0.0.1/tcp/1234")))
require.Equal(t, quicTr, s.TransportForDialing(ma.StringCast("/ip4/127.0.0.1/udp/1234/quic")))
require.Equal(t, circuitTr, s.TransportForDialing(ma.StringCast(fmt.Sprintf("/ip4/127.0.0.1/udp/1234/quic/p2p-circuit/p2p/%s", id))))
require.Equal(t, webtransportTr, s.TransportForDialing(ma.StringCast(fmt.Sprintf("/ip4/127.0.0.1/udp/1234/quic/webtransport/certhash/%s", certHash))))
require.Equal(t, webtransportTr, s.TransportForDialing(ma.StringCast(fmt.Sprintf("/ip4/127.0.0.1/udp/1234/quic-v1/webtransport/certhash/%s", certHash))))
require.Nil(t, s.TransportForDialing(ma.StringCast("/ip4/1.2.3.4")))
require.Nil(t, s.TransportForDialing(ma.StringCast("/ip4/1.2.3.4/tcp/443/ws")))
}
87 changes: 74 additions & 13 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 @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

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]
}
14 changes: 13 additions & 1 deletion p2p/net/swarm/swarm_dial_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,11 +196,23 @@ func TestAddrResolutionRecursive(t *testing.T) {
func TestRemoveWebTransportAddrs(t *testing.T) {
tcpAddr := ma.StringCast("/ip4/9.5.6.4/tcp/1234")
quicAddr := ma.StringCast("/ip4/1.2.3.4/udp/443/quic")
webtransportAddr := ma.StringCast("/ip4/1.2.3.4/udp/443/quic/webtransport")
webtransportAddr := ma.StringCast("/ip4/1.2.3.4/udp/443/quic-v1/webtransport")

require.Equal(t, []ma.Multiaddr{tcpAddr, quicAddr}, maybeRemoveWebTransportAddrs([]ma.Multiaddr{tcpAddr, quicAddr}))
require.Equal(t, []ma.Multiaddr{tcpAddr, webtransportAddr}, maybeRemoveWebTransportAddrs([]ma.Multiaddr{tcpAddr, webtransportAddr}))
require.Equal(t, []ma.Multiaddr{tcpAddr, quicAddr}, maybeRemoveWebTransportAddrs([]ma.Multiaddr{tcpAddr, webtransportAddr, quicAddr}))
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}))
}
35 changes: 24 additions & 11 deletions p2p/net/swarm/swarm_listen.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,23 +37,27 @@ func (s *Swarm) Listen(addrs ...ma.Multiaddr) error {
return nil
}

// ListenClose stop and delete listeners for all of the given addresses.
// ListenClose stop and delete listeners for all of the given addresses. If an
// any address belongs to one of the addreses a Listener provides, then the
// Listener will close for *all* addresses it provides. For example if you close
// and address with `/quic`, then the QUIC listener will close and also close
// any `/quic-v1` address.
func (s *Swarm) ListenClose(addrs ...ma.Multiaddr) {
var listenersToClose []transport.Listener
listenersToClose := make(map[transport.Listener]struct{}, len(addrs))

s.listeners.Lock()
for l := range s.listeners.m {
if !containsMultiaddr(addrs, l.Multiaddr()) {
if !containsSomeMultiaddr(addrs, l.Multiaddrs()) {
MarcoPolo marked this conversation as resolved.
Show resolved Hide resolved
continue
}

delete(s.listeners.m, l)
listenersToClose = append(listenersToClose, l)
listenersToClose[l] = struct{}{}
}
s.listeners.cacheEOL = time.Time{}
s.listeners.Unlock()

for _, l := range listenersToClose {
for l := range listenersToClose {
l.Close()
}
}
Expand Down Expand Up @@ -92,11 +96,13 @@ func (s *Swarm) AddListenAddr(a ma.Multiaddr) error {
s.listeners.cacheEOL = time.Time{}
s.listeners.Unlock()

maddr := list.Multiaddr()
maddrs := list.Multiaddrs()

// signal to our notifiees on listen.
s.notifyAll(func(n network.Notifiee) {
n.Listen(s, maddr)
for _, maddr := range maddrs {
n.Listen(s, maddr)
}
})

go func() {
Expand All @@ -116,7 +122,9 @@ func (s *Swarm) AddListenAddr(a ma.Multiaddr) error {

// signal to our notifiees on listen close.
s.notifyAll(func(n network.Notifiee) {
n.ListenClose(s, maddr)
for _, maddr := range maddrs {
n.ListenClose(s, maddr)
}
})
s.refs.Done()
}()
Expand Down Expand Up @@ -147,11 +155,16 @@ func (s *Swarm) AddListenAddr(a ma.Multiaddr) error {
return nil
}

func containsMultiaddr(addrs []ma.Multiaddr, addr ma.Multiaddr) bool {
for _, a := range addrs {
if addr == a {
func containsSomeMultiaddr(hayStack []ma.Multiaddr, needles []ma.Multiaddr) bool {
seenSet := make(map[string]struct{}, len(needles))
for _, a := range needles {
seenSet[string(a.Bytes())] = struct{}{}
}
for _, a := range hayStack {
if _, found := seenSet[string(a.Bytes())]; found {
return true
}
}
return false

}
2 changes: 1 addition & 1 deletion p2p/net/swarm/swarm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ func TestListenCloseCount(t *testing.T) {
t.Fatal(err)
}
listenedAddrs := s.ListenAddresses()
require.Equal(t, 2, len(listenedAddrs))
require.Equal(t, 3, len(listenedAddrs))

s.ListenClose(listenedAddrs...)

Expand Down
5 changes: 5 additions & 0 deletions p2p/net/upgrader/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

logging "github.com/ipfs/go-log/v2"
tec "github.com/jbenet/go-temp-err-catcher"
"github.com/multiformats/go-multiaddr"
manet "github.com/multiformats/go-multiaddr/net"
)

Expand Down Expand Up @@ -175,4 +176,8 @@ func (l *listener) String() string {
return fmt.Sprintf("<stream.Listener %s>", l.Multiaddr())
}

func (l *listener) Multiaddrs() []multiaddr.Multiaddr {
return []multiaddr.Multiaddr{l.Multiaddr()}
}

var _ transport.Listener = (*listener)(nil)
Loading