From eb452992fbc4d1b53099ea7ca15e360f8e813a4d Mon Sep 17 00:00:00 2001 From: Jorropo Date: Thu, 9 Feb 2023 16:43:12 +0100 Subject: [PATCH] feat: remove Mplex Mplex does not implement backpressure, our implementation will randomly reset streams if buffers overflow instead of risking deadlocks. In the past we had a bug where kubo nodes would prefer mplex over yamux. Turning off mplex make our connections to thoses nodes negociate yamux. Closes #9958 --- core/node/libp2p/smux.go | 48 +++++--------------------- docs/changelogs/v0.23.md | 11 +++++- docs/config.md | 24 +++++-------- docs/examples/kubo-as-a-library/go.mod | 1 - docs/examples/kubo-as-a-library/go.sum | 2 -- go.mod | 1 - go.sum | 2 -- test/cli/transports_test.go | 12 ------- 8 files changed, 27 insertions(+), 74 deletions(-) diff --git a/core/node/libp2p/smux.go b/core/node/libp2p/smux.go index c906b6e0237a..04a3f2b4e843 100644 --- a/core/node/libp2p/smux.go +++ b/core/node/libp2p/smux.go @@ -3,13 +3,11 @@ package libp2p import ( "fmt" "os" - "strings" "github.com/ipfs/kubo/config" "github.com/libp2p/go-libp2p" "github.com/libp2p/go-libp2p/core/network" - "github.com/libp2p/go-libp2p/p2p/muxer/mplex" "github.com/libp2p/go-libp2p/p2p/muxer/yamux" ) @@ -23,45 +21,17 @@ func yamuxTransport() network.Multiplexer { } func makeSmuxTransportOption(tptConfig config.Transports) (libp2p.Option, error) { - const yamuxID = "/yamux/1.0.0" - const mplexID = "/mplex/6.7.0" - if prefs := os.Getenv("LIBP2P_MUX_PREFS"); prefs != "" { - // Using legacy LIBP2P_MUX_PREFS variable. - log.Error("LIBP2P_MUX_PREFS is now deprecated.") - log.Error("Use the `Swarm.Transports.Multiplexers' config field.") - muxers := strings.Fields(prefs) - enabled := make(map[string]bool, len(muxers)) - - var opts []libp2p.Option - for _, tpt := range muxers { - if enabled[tpt] { - return nil, fmt.Errorf( - "duplicate muxer found in LIBP2P_MUX_PREFS: %s", - tpt, - ) - } - switch tpt { - case yamuxID: - opts = append(opts, libp2p.Muxer(tpt, yamuxTransport())) - case mplexID: - opts = append(opts, libp2p.Muxer(tpt, mplex.DefaultTransport)) - default: - return nil, fmt.Errorf("unknown muxer: %s", tpt) - } - } - return libp2p.ChainOptions(opts...), nil - } else { - return prioritizeOptions([]priorityOption{{ - priority: tptConfig.Multiplexers.Yamux, - defaultPriority: 100, - opt: libp2p.Muxer(yamuxID, yamuxTransport()), - }, { - priority: tptConfig.Multiplexers.Mplex, - defaultPriority: 200, - opt: libp2p.Muxer(mplexID, mplex.DefaultTransport), - }}), nil + return nil, fmt.Errorf("configuring muxers with LIBP2P_MUX_PREFS is no longer supported") } + if tptConfig.Multiplexers.Mplex != 0 { + return nil, fmt.Errorf("Swarm.Transports.Multiplexers.Mplex is no longer supported") + } + if tptConfig.Multiplexers.Yamux < 0 { + return nil, fmt.Errorf("Swarm.Transports.Multiplexers.Yamux is disabled even tho it is the only multiplexer available") + } + + return libp2p.Muxer(yamux.ID, yamuxTransport()), nil } func SmuxTransport(tptConfig config.Transports) func() (opts Libp2pOpts, err error) { diff --git a/docs/changelogs/v0.23.md b/docs/changelogs/v0.23.md index 5b43d6096752..b5f78615e185 100644 --- a/docs/changelogs/v0.23.md +++ b/docs/changelogs/v0.23.md @@ -6,6 +6,7 @@ - [Overview](#overview) - [๐Ÿ”ฆ Highlights](#-highlights) + - [Mplex removal](#mplex-removal) - [๐Ÿ“ Changelog](#-changelog) - [๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ Contributors](#-contributors) @@ -13,7 +14,15 @@ ### ๐Ÿ”ฆ Highlights +#### Mplex removal + +Support for Mplex was removed, this is because it is unreliable and would +randomly drop streams when sending data too fast. + +New pieces of code rely on backpressure, that means the stream will dynamicaly +slow down if the sending rate if data is getting backed up. +Backpressure is provided by Yamux and QUIC. + ### ๐Ÿ“ Changelog ### ๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ Contributors - diff --git a/docs/config.md b/docs/config.md index b7e21e12a2b7..a31ff17b792e 100644 --- a/docs/config.md +++ b/docs/config.md @@ -2052,6 +2052,8 @@ Type: `flag` Configuration section for libp2p _security_ transports. Transports enabled in this section will be used to secure unencrypted connections. +This does not concern all the QUIC transports which use QUIC's builtin encryption. + Security transports are configured with the `priority` type. When establishing an _outbound_ connection, Kubo will try each security @@ -2094,11 +2096,13 @@ Type: `priority` Configuration section for libp2p _multiplexer_ transports. Transports enabled in this section will be used to multiplex duplex connections. -Multiplexer transports are secured the same way security transports are, with +This does not concern all the QUIC transports which use QUIC's builtin muxing. + +Multiplexer transports are configured the same way security transports are, with the `priority` type. Like with security transports, the initiator gets their first choice. -Supported transports are: Yamux (priority 100) and Mplex (priority 200) +Supported transport is only: Yamux (priority 100) No default priority will ever be less than 100. @@ -2112,21 +2116,9 @@ Type: `priority` ### `Swarm.Transports.Multiplexers.Mplex` -Mplex is the default multiplexer used when communicating between Kubo and all -other IPFS and libp2p implementations. Unlike Yamux: - -* Mplex is a simpler protocol. -* Mplex is more efficient. -* Mplex does not have built-in keepalives. -* Mplex does not support backpressure. Unfortunately, this means that, if a - single stream to a peer gets backed up for a period of time, the mplex - transport will kill the stream to allow the others to proceed. On the other - hand, the lack of backpressure means mplex can be significantly faster on some - high-latency connections. +**DEPRECATED**: See https://github.com/ipfs/kubo/issues/9958 -Default: `200` - -Type: `priority` +Support for Mplex has been removed. Please remove this option from your config. ## `DNS` diff --git a/docs/examples/kubo-as-a-library/go.mod b/docs/examples/kubo-as-a-library/go.mod index 5cc1054a2457..804b3d17ba57 100644 --- a/docs/examples/kubo-as-a-library/go.mod +++ b/docs/examples/kubo-as-a-library/go.mod @@ -110,7 +110,6 @@ require ( github.com/libp2p/go-libp2p-record v0.2.0 // indirect github.com/libp2p/go-libp2p-routing-helpers v0.7.1 // indirect github.com/libp2p/go-libp2p-xor v0.1.0 // indirect - github.com/libp2p/go-mplex v0.7.0 // indirect github.com/libp2p/go-msgio v0.3.0 // indirect github.com/libp2p/go-nat v0.2.0 // indirect github.com/libp2p/go-netroute v0.2.1 // indirect diff --git a/docs/examples/kubo-as-a-library/go.sum b/docs/examples/kubo-as-a-library/go.sum index 3213c24ebc98..ae65f1eb61f4 100644 --- a/docs/examples/kubo-as-a-library/go.sum +++ b/docs/examples/kubo-as-a-library/go.sum @@ -482,8 +482,6 @@ github.com/libp2p/go-libp2p-routing-helpers v0.7.1/go.mod h1:cHStPSRC/wgbfpb5jYd github.com/libp2p/go-libp2p-testing v0.12.0 h1:EPvBb4kKMWO29qP4mZGyhVzUyR25dvfUIK5WDu6iPUA= github.com/libp2p/go-libp2p-xor v0.1.0 h1:hhQwT4uGrBcuAkUGXADuPltalOdpf9aag9kaYNT2tLA= github.com/libp2p/go-libp2p-xor v0.1.0/go.mod h1:LSTM5yRnjGZbWNTA/hRwq2gGFrvRIbQJscoIL/u6InY= -github.com/libp2p/go-mplex v0.7.0 h1:BDhFZdlk5tbr0oyFq/xv/NPGfjbnrsDam1EvutpBDbY= -github.com/libp2p/go-mplex v0.7.0/go.mod h1:rW8ThnRcYWft/Jb2jeORBmPd6xuG3dGxWN/W168L9EU= github.com/libp2p/go-msgio v0.0.4/go.mod h1:63lBBgOTDKQL6EWazRMCwXsEeEeK9O2Cd+0+6OOuipQ= github.com/libp2p/go-msgio v0.3.0 h1:mf3Z8B1xcFN314sWX+2vOTShIE0Mmn2TXn3YCUQGNj0= github.com/libp2p/go-msgio v0.3.0/go.mod h1:nyRM819GmVaF9LX3l03RMh10QdOroF++NBbxAb0mmDM= diff --git a/go.mod b/go.mod index d730dd2f34e1..8885a0089ac5 100644 --- a/go.mod +++ b/go.mod @@ -158,7 +158,6 @@ require ( github.com/libp2p/go-libp2p-asn-util v0.3.0 // indirect github.com/libp2p/go-libp2p-gostream v0.6.0 // indirect github.com/libp2p/go-libp2p-xor v0.1.0 // indirect - github.com/libp2p/go-mplex v0.7.0 // indirect github.com/libp2p/go-msgio v0.3.0 // indirect github.com/libp2p/go-nat v0.2.0 // indirect github.com/libp2p/go-netroute v0.2.1 // indirect diff --git a/go.sum b/go.sum index 22b6ea3cf2b4..c6e2e4b52fd3 100644 --- a/go.sum +++ b/go.sum @@ -545,8 +545,6 @@ github.com/libp2p/go-libp2p-testing v0.12.0 h1:EPvBb4kKMWO29qP4mZGyhVzUyR25dvfUI github.com/libp2p/go-libp2p-testing v0.12.0/go.mod h1:KcGDRXyN7sQCllucn1cOOS+Dmm7ujhfEyXQL5lvkcPg= github.com/libp2p/go-libp2p-xor v0.1.0 h1:hhQwT4uGrBcuAkUGXADuPltalOdpf9aag9kaYNT2tLA= github.com/libp2p/go-libp2p-xor v0.1.0/go.mod h1:LSTM5yRnjGZbWNTA/hRwq2gGFrvRIbQJscoIL/u6InY= -github.com/libp2p/go-mplex v0.7.0 h1:BDhFZdlk5tbr0oyFq/xv/NPGfjbnrsDam1EvutpBDbY= -github.com/libp2p/go-mplex v0.7.0/go.mod h1:rW8ThnRcYWft/Jb2jeORBmPd6xuG3dGxWN/W168L9EU= github.com/libp2p/go-msgio v0.0.4/go.mod h1:63lBBgOTDKQL6EWazRMCwXsEeEeK9O2Cd+0+6OOuipQ= github.com/libp2p/go-msgio v0.3.0 h1:mf3Z8B1xcFN314sWX+2vOTShIE0Mmn2TXn3YCUQGNj0= github.com/libp2p/go-msgio v0.3.0/go.mod h1:nyRM819GmVaF9LX3l03RMh10QdOroF++NBbxAb0mmDM= diff --git a/test/cli/transports_test.go b/test/cli/transports_test.go index 3a7b5ed933a8..601f858921be 100644 --- a/test/cli/transports_test.go +++ b/test/cli/transports_test.go @@ -71,18 +71,6 @@ func TestTransports(t *testing.T) { runTests(nodes) }) - t.Run("tcp with mplex", func(t *testing.T) { - t.Parallel() - nodes := tcpNodes(t) - nodes.ForEachPar(func(n *harness.Node) { - n.UpdateConfig(func(cfg *config.Config) { - cfg.Swarm.Transports.Multiplexers.Yamux = config.Disabled - }) - }) - nodes.StartDaemons().Connect() - runTests(nodes) - }) - t.Run("tcp with NOISE", func(t *testing.T) { t.Parallel() nodes := tcpNodes(t)