net: mptcp: force using MPTCP with GODEBUG

When adding MPTCP support to address the proposal #56539, I missed the
GODEBUG setting from Russ Cox's plan:

  I am inclined to say that we add MPTCP as an opt-in for a release or
  two, and then make it opt-out. There should be a GODEBUG setting (...)

See: https://github.com/golang/go/issues/56539#issuecomment-1309294637

Thanks to andrius4669 for having reported this issue to me.

It makes sense to have this GODEBUG setting not to have to modify
applications to use MPTCP (if available). It can then be useful to
estimate the impact in case we want to switch from opt-in to opt-out
later.

The MPTCP E2E test has been modified to make sure we can enable MPTCP
either via the source code like it was already the case before or with
this environment variable:

  GODEBUG=multipathtcp=1

The documentation has been adapted accordingly.

I don't know if it is too late for Go 1.21 but I had to put a version in
the documentation. The modification is small, the risk seems low and
this was supposed to be there from the beginning according to Russ Cox's
specifications. It can also be backported or only be present in the
future v1.22 if it is easier.

Note: I didn't re-open #56539 or open a new one. It is not clear to me
what I should do in this case.

Fixes #56539

Change-Id: I9201f4dc0b99e3643075a34c7032a95528c48fa0
Reviewed-on: https://go-review.googlesource.com/c/go/+/507375
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>

diff --git a/doc/godebug.md b/doc/godebug.md
index 43dbcd645a6b66e7cd40e111019da52419a29360..7a6d70e48782ad958135181dc64c551766979e2e 100644 (file)
--- a/doc/godebug.md
+++ b/doc/godebug.md
@@ -142,6 +142,10 @@ forms, controlled by the
 respectively.
 This behavior was backported to Go 1.19.8+ and Go 1.20.3+.
 
+Go 1.21 adds the support of Multipath TCP but it is only used if the application
+explicitly asked for it. This behavior can be controlled by the
+[`multipathtcp` setting](/pkg/net#Dialer.SetMultipathTCP).
+
 There is no plan to remove any of these settings.
 
 ### Go 1.20

diff --git a/src/internal/godebugs/table.go b/src/internal/godebugs/table.go
index 0fdd146b24dbd931c0e3555e8fe606c0a351cc19..243f9efce11e14e19663f770fc316d870d2db6a8 100644 (file)
--- a/src/internal/godebugs/table.go
+++ b/src/internal/godebugs/table.go
@@ -37,6 +37,7 @@ var All = []Info{
        //{Name: "multipartfiles", Package: "mime/multipart"},
        {Name: "multipartmaxheaders", Package: "mime/multipart"},
        {Name: "multipartmaxparts", Package: "mime/multipart"},
+       {Name: "multipathtcp", Package: "net"},
        {Name: "netdns", Package: "net", Opaque: true},
        {Name: "panicnil", Package: "runtime", Changed: 21, Old: "1"},
        {Name: "randautoseed", Package: "math/rand"},

diff --git a/src/net/dial.go b/src/net/dial.go
index fd1da1ebef070002e3496e2d95e97cde1c4a4104..79bc4958bb291cf89aa98a198e449aed5fe5ff64 100644 (file)
--- a/src/net/dial.go
+++ b/src/net/dial.go
@@ -6,6 +6,7 @@ package net
 
 import (
        "context"
+       "internal/godebug"
        "internal/nettrace"
        "syscall"
        "time"
@@ -21,6 +22,8 @@ const (
        defaultMPTCPEnabled = false
 )
 
+var multipathtcp = godebug.New("multipathtcp")
+
 // mptcpStatus is a tristate for Multipath TCP, see go.dev/issue/56539
 type mptcpStatus uint8
 
@@ -39,6 +42,13 @@ func (m *mptcpStatus) get() bool {
                return false
        }
 
+       // If MPTCP is forced via GODEBUG=multipathtcp=1
+       if multipathtcp.Value() == "1" {
+               multipathtcp.IncNonDefault()
+
+               return true
+       }
+
        return defaultMPTCPEnabled
 }
 
@@ -329,7 +339,7 @@ func (d *Dialer) MultipathTCP() bool {
 
 // SetMultipathTCP directs the Dial methods to use, or not use, MPTCP,
 // if supported by the operating system. This method overrides the
-// system default.
+// system default and the GODEBUG=multipathtcp=... setting if any.
 //
 // If MPTCP is not available on the host or not supported by the server,
 // the Dial methods will fall back to TCP.
@@ -690,7 +700,7 @@ func (lc *ListenConfig) MultipathTCP() bool {
 
 // SetMultipathTCP directs the Listen method to use, or not use, MPTCP,
 // if supported by the operating system. This method overrides the
-// system default.
+// system default and the GODEBUG=multipathtcp=... setting if any.
 //
 // If MPTCP is not available on the host or not supported by the client,
 // the Listen method will fall back to TCP.

diff --git a/src/net/mptcpsock_linux_test.go b/src/net/mptcpsock_linux_test.go
index bf8fc951c53ffc80faac6b5144937f1558837870..5134aba75eabb5d36e168fa1c5733ae7a01f6695 100644 (file)
--- a/src/net/mptcpsock_linux_test.go
+++ b/src/net/mptcpsock_linux_test.go
@@ -12,15 +12,22 @@ import (
        "testing"
 )
 
-func newLocalListenerMPTCP(t *testing.T) Listener {
+func newLocalListenerMPTCP(t *testing.T, envVar bool) Listener {
        lc := &ListenConfig{}
-       if lc.MultipathTCP() {
-               t.Error("MultipathTCP should be off by default")
-       }
 
-       lc.SetMultipathTCP(true)
-       if !lc.MultipathTCP() {
-               t.Fatal("MultipathTCP is not on after having been forced to on")
+       if envVar {
+               if !lc.MultipathTCP() {
+                       t.Fatal("MultipathTCP Listen is not on despite GODEBUG=multipathtcp=1")
+               }
+       } else {
+               if lc.MultipathTCP() {
+                       t.Error("MultipathTCP should be off by default")
+               }
+
+               lc.SetMultipathTCP(true)
+               if !lc.MultipathTCP() {
+                       t.Fatal("MultipathTCP is not on after having been forced to on")
+               }
        }
 
        ln, err := lc.Listen(context.Background(), "tcp", "127.0.0.1:0")
@@ -64,15 +71,22 @@ func postAcceptMPTCP(ls *localServer, ch chan<- error) {
        }
 }
 
-func dialerMPTCP(t *testing.T, addr string) {
+func dialerMPTCP(t *testing.T, addr string, envVar bool) {
        d := &Dialer{}
-       if d.MultipathTCP() {
-               t.Error("MultipathTCP should be off by default")
-       }
 
-       d.SetMultipathTCP(true)
-       if !d.MultipathTCP() {
-               t.Fatal("MultipathTCP is not on after having been forced to on")
+       if envVar {
+               if !d.MultipathTCP() {
+                       t.Fatal("MultipathTCP Dialer is not on despite GODEBUG=multipathtcp=1")
+               }
+       } else {
+               if d.MultipathTCP() {
+                       t.Error("MultipathTCP should be off by default")
+               }
+
+               d.SetMultipathTCP(true)
+               if !d.MultipathTCP() {
+                       t.Fatal("MultipathTCP is not on after having been forced to on")
+               }
        }
 
        c, err := d.Dial("tcp", addr)
@@ -128,12 +142,16 @@ func canCreateMPTCPSocket() bool {
        return true
 }
 
-func TestMultiPathTCP(t *testing.T) {
-       if !canCreateMPTCPSocket() {
-               t.Skip("Cannot create MPTCP sockets")
+func testMultiPathTCP(t *testing.T, envVar bool) {
+       if envVar {
+               t.Log("Test with GODEBUG=multipathtcp=1")
+               t.Setenv("GODEBUG", "multipathtcp=1")
+       } else {
+               t.Log("Test with GODEBUG=multipathtcp=0")
+               t.Setenv("GODEBUG", "multipathtcp=0")
        }
 
-       ln := newLocalListenerMPTCP(t)
+       ln := newLocalListenerMPTCP(t, envVar)
 
        // similar to tcpsock_test:TestIPv6LinkLocalUnicastTCP
        ls := (&streamListener{Listener: ln}).newLocalServer()
@@ -153,7 +171,7 @@ func TestMultiPathTCP(t *testing.T) {
                t.Fatal(err)
        }
 
-       dialerMPTCP(t, ln.Addr().String())
+       dialerMPTCP(t, ln.Addr().String(), envVar)
 
        if err := <-genericCh; err != nil {
                t.Error(err)
@@ -162,3 +180,13 @@ func TestMultiPathTCP(t *testing.T) {
                t.Error(err)
        }
 }
+
+func TestMultiPathTCP(t *testing.T) {
+       if !canCreateMPTCPSocket() {
+               t.Skip("Cannot create MPTCP sockets")
+       }
+
+       for _, envVar := range []bool{false, true} {
+               testMultiPathTCP(t, envVar)
+       }
+}

diff --git a/src/runtime/metrics/doc.go b/src/runtime/metrics/doc.go
index 5c52f784778662a47937245b80810f260f617897..b4d32d135ab5e59690398b72dca5c742cba6add9 100644 (file)
--- a/src/runtime/metrics/doc.go
+++ b/src/runtime/metrics/doc.go
@@ -273,6 +273,10 @@ Below is the full list of supported metrics, ordered lexicographically.
                the mime/multipart package due to a non-default
                GODEBUG=multipartmaxparts=... setting.
 
+       /godebug/non-default-behavior/multipathtcp:events
+               The number of non-default behaviors executed by the net package
+               due to a non-default GODEBUG=multipathtcp=... setting.
+
        /godebug/non-default-behavior/panicnil:events
                The number of non-default behaviors executed by the runtime
                package due to a non-default GODEBUG=panicnil=... setting.