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

net: add support for MPTCP #56539

Closed
matttbe opened this issue Nov 3, 2022 · 56 comments
Closed

net: add support for MPTCP #56539

matttbe opened this issue Nov 3, 2022 · 56 comments

Comments

@matttbe
Copy link
Contributor

matttbe commented Nov 3, 2022

Hello,

I'm part of the Linux MPTCP Upstream community and some users reported difficulties to support MPTCP with Go.

Multipath TCP or MPTCP is an extension to TCP. The goal is to exchange data for a single connection over different paths, simultaneously (or not). Its support in the Linux kernel has started in Linux v5.6.

For the userspace, the usage is similar to TCP. The main difference is visible when creating the socket because the MPTCP protocol has to be set:

socket(AF_INET(6), SOCK_STREAM, IPPROTO_MPTCP);

After that, the application uses the socket like it would do if it was TCP. It can also get and set socket options from different levels: socket, IP(v6), TCP and also a new one: SOL_MPTCP.

(Note that it is also possible to create MPTCP connection on MacOS but the API is different)

An easy way to force an application to support MPTCP is to use mptcpize tool which uses LD_PRELOAD with a lib changing the behaviour of libc's socket API: if TCP is asked, force MPTCP instead. Of course, this is not possible with Go apps as this trick is specific to the libc.

I'm not a Go developer but from what I saw, it looks like the recommended way to communicate with a server with TCP is to use the net package. It would be great if the net package could support MPTCP, e.g.

conn, err := net.Dial("mptcp", "golang.org:80")

From what I saw, TCP sockets from the net package calls (1)(2)(3) internetSocket() function with 0 for the protocol ID. (This function will call: socket()sysSocket()socketFunc()syscall.Socket re-using the protocol ID given in argument.)

It should not be difficult to allow setting this protocol ID argument to IPPROTO_MPTCP (262) instead, no?

@gopherbot gopherbot added this to the Proposal milestone Nov 3, 2022
@apparentlymart
Copy link

apparentlymart commented Nov 4, 2022

Hi @matttbe!

Multipath TCP is new to me so I must admit that I've only read the introductory sections of RFC8684. I apologize if the following questions are naive.

As a developer of network applications, when I'm choosing between TCP and UDP (the two main choices available through the net package) I tend to do so based on fundamental differences between the two; stream-oriented vs. packet oriented being the key consideration.

The introductory material for Multipart TCP in the RFC describes it as being a backward-compatible extension to TCP, capable of interacting with existing TCP-only implementations and able to "progressive-enhance" (to borrow terminology from the web platform) a TCP connection into a MPTCP connection by negotiating subflows after an initial standard TCP handshake, if both parties set the MP_CAPABLE option on their handshake packets.

That leaves me a little unsure as to how I would decide between using MPTCP and TCP if presented in the way you've proposed here. If I take the RFC introductory content at face value, it appears that I should change all of my programs to unconditionally specify "mptcp" instead of "tcp" and never use "tcp" again moving forward. However, I see that the Linux kernel implementation is opt-in, so I have to assume the decision criteria are not as obvious as a naive reading of the RFC content would suggest.

I imagine there's a hypothetical alternative proposal here of making net.Dial("tcp", "golang.org:80") unconditionally set IPPROTO_MPTCP if it can somehow prove that it's running on a kernel which has support for that protocol, and then leave the kernel to negotiate subflows where possible or just use regular TCP if that's not possible or applicable. This is perhaps analogous to how Go's net/http package automatically selects HTTP 2 when possible, but uses HTTP 1.1 otherwise.

Can you comment on what the drawbacks might be of that alternative design?

Thanks!

@mengzhuo
Copy link
Contributor

mengzhuo commented Nov 4, 2022

cc @neild

@matttbe
Copy link
Contributor Author

matttbe commented Nov 7, 2022

Hi @apparentlymart

Thank you for your reply!

The introductory material for Multipart TCP in the RFC describes it as being a backward-compatible extension to TCP, capable of interacting with existing TCP-only implementations and able to "progressive-enhance" (to borrow terminology from the web platform) a TCP connection into a MPTCP connection by negotiating subflows after an initial standard TCP handshake, if both parties set the MP_CAPABLE option on their handshake packets.

That's correct but I prefer to add a few more precisions:

  • MPTCP is indeed an extension to TCP: technically, some bytes specific to MPTCP are added in the TCP options field.
  • After the 3-way handshake and with the current implementation, some MPTCP options will still be present in the following packets, even if you are only using one path: it means that compared to TCP, a few bytes are added in each packet (when you transfer a lot of data, the overhead is only around 1%).
  • Fallback to TCP is supported:
    • if the client doesn't support MPTCP, the server will reply with regular TCP packets
    • if the server doesn't support MPTCP, by design, it will ignore MPTCP options added in the first packet by the client and reply with regular TCP packets. The client will no longer add MPTCP options in the following packets attached to this connection
    • intermediate devices between the client and the server are supposed to forward the packets and not cause troubles but some middleboxes on the Internet might modify the connection and interfere with MPTCP, e.g. firewall can drop MPTCP options, transparent proxy can duplicate them but some can be problematic for MPTCP like injecting data without modifying MPTCP data sequence numbers (a NAT modifying packets from old protocols like FTP) and blocking the whole connection (very rare).
    • Most scenarios are covered but still, if a stupid annoying firewall decides to block the whole connection if it sees MPTCP option (I don't know why it would do that instead of removing the option but well), this will impact the user experience.

Please note that Apple has been using MPTCP for some apps since 2013. They started with Siri to better support handovers. Today they are still using it with more apps like Apple Music and Map. So it means MPTCP can survive on the wild Internet.

https://en.wikipedia.org/wiki/MPTCP
http://blog.multipath-tcp.org/blog/html/2018/12/15/apple_and_multipath_tcp.html
https://www.tessares.net/apples-mptcp-story-so-far/
https://www.tessares.net/technology/mptcp/

That leaves me a little unsure as to how I would decide between using MPTCP and TCP if presented in the way you've proposed here. If I take the RFC introductory content at face value, it appears that I should change all of my programs to unconditionally specify "mptcp" instead of "tcp" and never use "tcp" again moving forward. However, I see that the Linux kernel implementation is opt-in, so I have to assume the decision criteria are not as obvious as a naive reading of the RFC content would suggest.

On top of the possible (rare) network issues and the few additional bytes carried by each packet, there are a few additional points to raise here:

  • MPTCP in the Linux kernel implementation is indeed opt-in. It was a requirement to get MPTCP in the Upstream Linux kernel and there are multiple reasons:
    • The implementation had to be incremental: e.g. in Linux v5.6, only one subflow could be created and many socket options were not supported. New features were added progressively (changelog) but what I mean is that it would have broken many apps if from one version to another, all TCP connections were in fact MPTCP ones with limited support at that time.
    • TCP in the Linux kernel is highly optimised: on one hand, you don't want to introduce instabilities by adding new features turned on by default ; on the other hand, any extra layer somehow impacts performances, even if it is under 1%.
    • Each modification in the TCP code had to be motivated with a proof that was not impacting the performance for "plain" TCP sockets (at least in the "fast" path).
    • In other words, TCP will always perform better than MPTCP with a single flow: that's important for some use cases, not for all (most?). But of course, MPTCP generally compensates that by being able to use more paths.
    • One last thing is that because we add more features, there is always a risk of discovering new issues or new "misuses" including security issues. From a security point of view, the protocol has been deeply analysed. Even if fixes can come quickly, there can be some issues in the code and that's another reason why network maintainer didn't want to have MPTCP enabled by default.
  • Some (very) specific features available with TCP sockets are maybe missing when MPTCP is used: for example, there is a huge variety of socket option ([gs]etsockopt()), and some might be missing depending on the kernel you are using, e.g. TCP FASTOPEN is being implemented now (client part available in kernel 6.1), TCP ZeroCopy is not supported yet (quite specific), TCP MD5 cannot work with MPTCP (who cares? :) ), etc. → those are very specific to some use cases, not needed for more common use cases (e.g. HTTP) but still important to keep in mind.
  • That's not really an issue but you need to configure the kernel to use multiple endpoints (IP addresses): some tools are starting to configure that automatically (e.g. Network Manager) but if you do want to use multiple paths, it is important to remember this bit. And of course, like any configurations, it has to be done properly, e.g. the user should not tell the kernel it can use a costly path (e.g. cellular network) for all connections for example.

In short, it might still be needed for some people to ask for "plain" TCP sockets (without MPTCP).

I imagine there's a hypothetical alternative proposal here of making net.Dial("tcp", "golang.org:80") unconditionally set IPPROTO_MPTCP if it can somehow prove that it's running on a kernel which has support for that protocol, and then leave the kernel to negotiate subflows where possible or just use regular TCP if that's not possible or applicable. This is perhaps analogous to how Go's net/http package automatically selects HTTP 2 when possible, but uses HTTP 1.1 otherwise.

It would be really great to have MPTCP more widely used by setting it by default and I think it would help to improve handover use cases or to get more bandwidth for example. But from my point of view, I think we should have the possibility to use MPTCP or not when we ask to create "a basic socket" (net.Dial("mptcp", "...")).

On the other hand, libraries built on top of it could decide to use MPTCP by default (on Linux) and retry with plain TCP if the connection is rejected for example or if MPTCP is not supported by the kernel (errno set to ENOPROTOOPT (Protocol not available) or EPROTONOSUPPORT (Protocol not supported)). Some useful functions could be available in net, e.g. to create a TCP socket if it is not possible to create an MPTCP one, to check if the created socket is a TCP/MPTCP one, to check if later on, the connection fell back to TCP, etc.

Please also note that in the Linux kernel, MPTCP "listening" sockets will create "plain" "accepted" TCP sockets if the connection request (initial TCP SYN) didn't contain any MPTCP options. In other words, from a performance/impact point of view, it makes sense to enable MPTCP support for the server side "by default": it will only be used if the client asks for it.

One last thing, Apple proved the protocol works on the Internet, it should probably be used by default by more libs/apps and net/http is probably a good candidate! Before, net API should of course allow users to create MPTCP sockets.

I hope this answers your questions! Do not hesitate to ask more if you have any others.

@apparentlymart
Copy link

Thanks for those details, @matttbe!

Based on what you've shared -- particularly that MPTCP seems compatible enough with TCP that it's been successfully deployed for production systems like Apple's -- I think I'd personally prefer a design which by default lets the system choose what it deems to be the best available protocol, but offers a way to constrain more tightly as an alternative for those who need more control. This would then echo the design of net/http with regard to HTTP 2 and other successor protocols which are mechanically different than but conceptually compatible with HTTP, so that Go programs would immediately gain support for MPTCP when talking with an MPTCP-supporting peer.

I suppose I can see the argument that if someone is programming at the level of sockets (net) rather than using an application-layer client (like net/http) then it is justified to expose these low-level details. But I also note this common pattern of just forcing arbitrary existing libc-based software to use MPTCP using LD_PRELOAD tricks, which puts that decision outside of the direct control of the developer of that software, suggesting to me that it's typically fine to just swap out TCP for MPTCP in existing software without causing any significant problems.

I must admit though that I'm coming at this from the perspective of someone who only occasionally deals with sockets directly in my Go programs, and so my position might not be shared by those who more frequently do low-level protocol implementation in terms of sockets. With that in mind, I'll bow out of the discussion at this point to make space for others to share different perspectives.

Thanks again for the detailed response!

@rsc
Copy link
Contributor

rsc commented Nov 9, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Nov 9, 2022

This does feel a lot like adding support for HTTP/2 or TLS 1.3, both of which did not require users to modify their programs to get the newer, better protocols. It seems like the right end state is for Go programs to "do the right thing" and use MPTCP whenever that makes sense in place of plain TCP for dialed "tcp" connections. Programs shouldn't have to update to take advantage of the new protocol. Of course we don't want to break people either, if MPTCP would break them. See my recent Compatibility talk for more of my thinking about these kinds of changes.

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 and possibly also a source-code mechanism, probably for both listening and dialing. I'm not sure what the source code mechanism would be. Maybe a field in net.Dialer and net.ListenConfig, but it seems a little out-of-place, and the field would need to be more than just a single bool, because you need three values: force on, force off, default.

@Jorropo
Copy link
Member

Jorropo commented Nov 9, 2022

@rsc given that this would be implemented in kernel and might be disabled, it would be more like copy_file_range for os.(*File).WriteTo where the runtime would first parse kernel fields (/sys/... afait) to check of MPTCP is enabled and then use it if available ?

@ianlancetaylor
Copy link
Contributor

@Jorropo More likely we would first try to use IPPROTO_MPTCP and if that returns ENOSYS or EINVAL fall back to using IPPROTO_TCP.

@rsc
Copy link
Contributor

rsc commented Nov 10, 2022

@Jorropo, for the case where we are holding an fd in a net.Conn and don't know whether it's TCP or MPTCP and want to do some optimized code path like sendfile(2) that only supports TCP, it seems like we would just call sendfile and fall back to generic code if sendfile returns an error. We already do that fallback today in code that uses sendfile and copy_file_range. I don't think we'd have to update anything, but if we do it wouldn't be much.

@matttbe
Copy link
Contributor Author

matttbe commented Nov 10, 2022

@rsc Thank you for having added this ticket to the proposals project!

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 and possibly also a source-code mechanism, probably for both listening and dialing. I'm not sure what the source code mechanism would be. Maybe a field in net.Dialer and net.ListenConfig, but it seems a little out-of-place, and the field would need to be more than just a single bool, because you need three values: force on, force off, default.

That looks like a good plan!

@Jorropo More likely we would first try to use IPPROTO_MPTCP and if that returns ENOSYS or EINVAL fall back to using IPPROTO_TCP.

@ianlancetaylor Yes, that's probably the easiest solution.

Please note that if I'm not mistaken, there can be 3 different errors:

  • ENOPROTOOPT 92 Protocol not available: if MPTCP has been disabled with sysctl net.mptcp.enabled=0 for example
  • EPROTONOSUPPORT 93 Protocol not supported: if MPTCP option has not been is not compiled on kernel >=v5.6
  • EINVAL 22 Invalid argument: if MPTCP is not available on kernels <5.6
  • maybe others if MPTCP is blocked by custom "rules" from SELinux, eBPF, etc.

If you get EPROTONOSUPPORT or EINVAL the first time, it means MPTCP will never be possible with the current kernel. It is then possible to have an optimisation there and directly use IPPROTO_TCP for the next sockets.

@neild
Copy link
Contributor

neild commented Nov 10, 2022

Maybe a field in net.Dialer and net.ListenConfig, but it seems a little out-of-place, and the field would need to be more than just a single bool, because you need three values: force on, force off, default.

I don't see any place other than net.Dialer and net.ListenConfig to put the option. These structs already have the Control callback func field; changing the socket protocol seems analogous.

Are we willing to add unexported fields to these types? If so, we could add methods to enable/disable MPTCP. That might be simpler to use than a tristate field.

func (d *Dialer) EnableMultipathTCP(enabled bool)
func (lc *ListenConfig) EnableMultipathTCP(enabled bool)

@rsc
Copy link
Contributor

rsc commented Nov 16, 2022

EnableMPTCP(false) reads funny; it should probably be SetMPTCP.
Otherwise this sounds fine.
The method approach is nice.

@neild
Copy link
Contributor

neild commented Nov 16, 2022

Accessors as well?

func (*Dialer) MPTCP() bool
func (*ListenConfig) MPTCP() bool

I've got a mild preference for spelling out MultipathTCP; https://www.multipath-tcp.org/ mostly uses "Multipath TCP" in prose and only occasionally abbreviates to MPTCP, but the abbreviation is fine as well.

@matttbe
Copy link
Contributor Author

matttbe commented Nov 17, 2022

I've got a mild preference for spelling out MultipathTCP; https://www.multipath-tcp.org/ mostly uses "Multipath TCP" in prose and only occasionally abbreviates to MPTCP, but the abbreviation is fine as well.

@neild : I don't have any preferences nor recommendations. All I can say is that in the Linux kernel, we are usually referring to MPTCP (IPPROTO_MPTCP, TCPOPT_MPTCP, SOL_MPTCP, /proc/sys/net/mptcp/, etc.) but that's mainly to have a short name. In technical documents and paper, we can find both, e.g. Apple's description, Annotated bibliography (pdf), etc.

Note that Apple is also using applemultipath in some programs, e.g. in their OpenSSH's fork. But I would recommend not to just use "multipath" without "TCP" because it is too vague, there are other multipath protocols. On the other hand, I think some people quickly short "Multipath TCP" to "Multipath". When "MPTCP" is used, it is less tempting to short it even more :)

@apparentlymart
Copy link

apparentlymart commented Nov 17, 2022

I see that there is some discussion about optional additional API surface area specifically for Multipath TCP. For example, I see An enhanced socket API for Multipath TCP although I do not have access to the content of the paper, only to this abstract. It seems plausible to me that it's related to IETF draft-hesmans-mptcp-socket-00, given the overlap of authorship, so perhaps that draft is sufficient to understand the shape of the proposed API.

This might be ignorable for now to keep initial scope small, but I raise it in case it might be desirable to reserve API surface area for some future way to dynamically obtain something implementing that new API given an active socket that is already using the Multipath TCP protocol. (I assume that Go would want to expose an idiomatic API equivalent to the lower-level socket API proposed there, rather than requiring direct use of setsockopt and getsockopt, but that may be presumptuous in itself given how early this stuff seems to be.)

@matttbe
Copy link
Contributor Author

matttbe commented Nov 18, 2022

I see that there is some discussion about optional additional API surface area specifically for Multipath TCP. For example, I see An enhanced socket API for Multipath TCP although I do not have access to the content of the paper, only to this abstract. It seems plausible to me that it's related to IETF draft-hesmans-mptcp-socket-00, given the overlap of authorship, so perhaps that draft is sufficient to understand the shape of the proposed API.

@apparentlymart these drafts have been written by two colleagues and they were designed for a previous out of tree implementation (kernels <= v5.4, custom kernel), not the one in the "official" upstream Linux kernel (kernels >= v5.6).

In the "upstream" project, we decided to be as transparent and as generic as possible: it means that we try to have a behaviour similar to TCP if that makes sense. In other words, userspace app can continue to set and get socket options as it was doing with TCP. Typically, options are propagated to all subflows, even the new ones later. Of course, this needs to be checked case by case: some options don't make sense for MPTCP (specific to other protocols), some cannot work with MPTCP (e.g. TCP MD5 but that's very specific, typically for routers), some only affect the MPTCP sockets and not the subflows (e.g. poll related ones), some are only for the first subflow (e.g. TFO), etc. There are also new options specific to MPTCP in a dedicated space (SOL_MPTCP), e.g. MPTCP_INFO, similar to TCP_INFO but adapted to MPTCP.

The idea is not to have a complex socket API exposed to userspace but keep it simple and have dedicated daemons handling specific use cases. For example,

  • mptcpd is initially dedicated to act as a userspace path manager (but not limited to).
  • If an application needs something specific to handle socket options per path, it can use eBPF to do that.
  • eBPF will "soon" be used to customise the packet scheduler behaviour (WIP).

If you are interested by the subject, I recommend you to check a recent presentation about that: abstract, slides, video

One last thing: if userspace devs noticed something is missing, it is never too late to introduce new features in the kernel ;-)

@rsc
Copy link
Contributor

rsc commented Nov 30, 2022

It sounds like this is the API:

func (*Dialer) SetMultipathTCP(enabled bool)
func (*Dialer) MultipathTCP() bool
func (*ListenConfig) SetMultipathTCP(enabled bool)
func (*ListenConfig) MultipathTCP() bool

It would be up to the implementation whether the default result from MultipathTCP is on or off, and then programs could override that with SetMultipathTCP. In Go 1.21, we would default it off everywhere, and then in Go 1.22 or later, we would turn it on by default for systems that support it. Support might mean that it's an operating system that we have code for, or it might mean a more precise result, that we've sniffed the current kernel to see if it's available. That detail is up to the implementation and can change over time.

Does that seem reasonable?

@rsc
Copy link
Contributor

rsc commented Dec 7, 2022

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Dec 14, 2022

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: net: add support for MPTCP net: add support for MPTCP Dec 14, 2022
@rsc rsc modified the milestones: Proposal, Backlog Dec 14, 2022
@matttbe
Copy link
Contributor Author

matttbe commented Dec 15, 2022

Hi @rsc

That's great the proposal has been accepted, thank you all for your support!

I'm sorry to ask this "stupid" question but what's next? I mean: I'm not a Go expert, do I need to find someone to do:help for the implementation or ask something somewhere or ...? :)

@bradfitz
Copy link
Contributor

@matttbe, now it sits until somebody steps up to do the work. The code then needs to be reviewed/approved, but the idea/direction is now approved.

@matttbe
Copy link
Contributor Author

matttbe commented Dec 15, 2022

@bradfitz thank you, that's clearer!
Hopefully someone will be interested to help 😊

gopherbot pushed a commit that referenced this issue Mar 24, 2023
This currently defines an internal function supportsMultipathTCP which
reports whether MPTCP[1] is supported on the current platform.

Only Linux is supported here.

The check on Linux is performed once by attemting to create an MPTCP
socket and look at the returned error:

- If the protocol is not supported, EINVAL (kernel < 5.6) or
  EPROTONOSUPPORT (kernel >= 5.6) is returned and there is no point to
  try again.

- Other errors can be returned:
  - ENOPROTOOPT: the sysctl knob net.mptcp.enabled is set to 0
  - Unpredictable ones: if MPTCP is blocked using SELinux, eBPF, etc.

These other errors are due to modifications that can be reverted during
the session: MPTCP can be available again later. In this case, it is
fine to always try to create an MPTCP socket and fallback to TCP in case
of error.

This work has been co-developped by Gregory Detal
<gregory.detal@tessares.net>.

[1] https://www.rfc-editor.org/rfc/rfc8684.html

Updates #56539

Change-Id: Ic84fe85aad887a2be4556a898e649bf6b6f12f03
Reviewed-on: https://go-review.googlesource.com/c/go/+/471135
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
gopherbot pushed a commit that referenced this issue Mar 29, 2023
This function is called when the user has requested MPTCP via
SetMultipathTCP in the Dialer.

This new function falls back to dialTCP on operating systems that do not
support MPTCP or if MPTCP is not supported.

On Dialer side, MultipathTCP function can be used to know if the package
will try to use MPTCP or not when Dial is called.

Note that this new dialMPTCP function returns a TCPConn object, like
dialTCP. A new MPTCPConn object using the following composition could
have been returned:

    type MPTCPConn struct {
        *TCPConn
    }

But the drawback is that if MPTCP is used by default one day (see #56539
issue on GitHub), Dial will return a different object: this new
MPTCPConn type instead of the previously expected TCPConn. This can
cause issues for apps checking the returned object.

This work has been co-developped by Gregory Detal
<gregory.detal@tessares.net>.

Updates #56539

Change-Id: I0f9b5b81f630b39142bdd553d4f1b4c775f1dff0
Reviewed-on: https://go-review.googlesource.com/c/go/+/471136
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
gopherbot pushed a commit that referenced this issue Mar 29, 2023
Similar to dialMPTCP, this listenMPTCP function is called when the user
has requested MPTCP via SetMultipathTCP in the ListenConfig.

This function falls back to listenTCP on operating systems that do not
support MPTCP or if MPTCP is not supported.

On ListenConfig side, MultipathTCP function can be used to know if the
package will try to use MPTCP or not when Listen is called.

Note that this new listenMPTCP function returns a TCPListener object and
not a new MPTCP dedicated one. The reasons are similar as the ones
explained in the parent commit introducing dialTCP: if MPTCP is used by
default later, Listen will return a different object that could break
existing applications expecting TCPListener.

This work has been co-developped by Gregory Detal
<gregory.detal@tessares.net>.

Updates #56539

Change-Id: I010f1d87f921bbac9e157cef2212c51917852353
Reviewed-on: https://go-review.googlesource.com/c/go/+/471137
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
gopherbot pushed a commit that referenced this issue Mar 30, 2023
Specific MPTCP errors could happen but only one is detectable: if
ENOPROTOOPT errno is returned, it likely means MPTCP has been disable
via this sysctl knob: net.mptcp.enabled.

But because MPTCP could be blocked by the administrator using different
techniques (SELinux, etc.) making the socket creation returning other
errors, it looks better to always retry to create a "plain" TCP socket
when any errors are returned.

This work has been co-developed by Gregory Detal
<gregory.detal@tessares.net>.

Updates #56539

Change-Id: I94fb8448dae351e1d3135b4f182570979c6b36d3
Reviewed-on: https://go-review.googlesource.com/c/go/+/471138
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
gopherbot pushed a commit that referenced this issue Apr 18, 2023
This new TCPConn method returns whether the connection is using MPTCP or
if a fallback to TCP has been done, e.g. because the other peer doesn't
support MPTCP.

When working on the new E2E test linked to MPTCP (#56539), it looks like
the user might need to know such info to be able to do some special
actions (report, stop, etc.). This also improves the test to make sure
MPTCP has been used as expected.

Regarding the implementation, from kernel version 5.16, it is possible
to use:

    getsockopt(..., SOL_MPTCP, MPTCP_INFO, ...)

and check if EOPNOTSUPP (IPv4) or ENOPROTOOPT (IPv6) is returned. If it
is, it means a fallback to TCP has been done. See this link for more
details:

    multipath-tcp/mptcp_net-next#294

Before v5.16, there is no other simple way, from the userspace, to check
if the created socket did a fallback to TCP. Netlink requests could be
done to try to find more details about a specific socket but that seems
quite a heavy machinery. Instead, only the protocol is checked on older
kernels.

The E2E test has been modified to check that the MPTCP connection didn't
do any fallback to TCP, explicitely validating the two methods
(SO_PROTOCOL and MPTCP_INFO) if it is supported by the host.

This work has been co-developed by Gregory Detal
<gregory.detal@tessares.net> and Benjamin Hesmans
<benjamin.hesmans@tessares.net>.

Fixes #59166

Change-Id: I5a313207146f71c66c349aa8588a2525179dd8b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/471140
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
@andrius4669
Copy link
Contributor

andrius4669 commented May 20, 2023

maybe implement GODEBUG knob for the default too?
something that i can't find mentioned before, but would make sense IMO.
EDIT: actually rsc did mention GODEBUG setting

@gopherbot
Copy link

Change https://go.dev/cl/498601 mentions this issue: doc/go1.21: mention multipath TCP support

gopherbot pushed a commit that referenced this issue May 30, 2023
For #56539
For #59166

Change-Id: Ief392464916a1a74a8fcc6c3c7bdb213e8c6ef98
Reviewed-on: https://go-review.googlesource.com/c/go/+/498601
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Eli Bendersky <eliben@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
@dmitshur dmitshur modified the milestones: Backlog, Go1.21 Jun 4, 2023
Nasfame pushed a commit to golangFame/go that referenced this issue Jun 4, 2023
For golang#56539
For golang#59166

Change-Id: Ief392464916a1a74a8fcc6c3c7bdb213e8c6ef98
Reviewed-on: https://go-review.googlesource.com/c/go/+/498601
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Eli Bendersky <eliben@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
@gopherbot
Copy link

Change https://go.dev/cl/507375 mentions this issue: net: mptcp: force using MPTCP with GODEBUG

gopherbot pushed a commit that referenced this issue Jul 11, 2023
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: #56539 (comment)

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>
@matttbe
Copy link
Contributor Author

matttbe commented Jul 11, 2023

maybe implement GODEBUG knob for the default too? something that i can't find mentioned before, but would make sense IMO. EDIT: actually rsc did mention GODEBUG setting

@andrius4669 thank you for the reminder, done: https://go-review.googlesource.com/c/go/+/507375 ;-)

bradfitz pushed a commit to tailscale/go that referenced this issue Jul 15, 2023
When adding MPTCP support to address the proposal golang#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: golang#56539 (comment)

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 golang#56539 or open a new one. It is not clear to me
what I should do in this case.

Fixes golang#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>
komuw added a commit to komuw/go that referenced this issue Feb 13, 2024
When golang#56539 was accepted, it was decided[1] that MPTCP would be opt-in for a release or two, and then made opt-out.

[1] https://go.dev/issue/56539#issuecomment-1309294637

Updates golang#56539
komuw added a commit to komuw/go that referenced this issue Feb 13, 2024
When golang#56539 was accepted, it was decided[1] that MPTCP would be opt-in for a release or two, and then made opt-out.

[1] https://go.dev/issue/56539#issuecomment-1309294637

Updates golang#56539
@gopherbot
Copy link

Change https://go.dev/cl/563575 mentions this issue: net: enable mptcp by default

komuw added a commit to komuw/go that referenced this issue Feb 13, 2024
When golang#56539 was accepted, it was decided[1] that MPTCP
would be opt-in for a release or two, and then made opt-out.

[1] https://go.dev/issue/56539#issuecomment-1309294637

Updates golang#56539
komuw added a commit to komuw/go that referenced this issue Feb 13, 2024
When golang#56539 was accepted, it was decided[1] that MPTCP
would be opt-in for a release or two, and then made opt-out.

[1] https://go.dev/issue/56539#issuecomment-1309294637

Updates golang#56539
@database64128
Copy link
Contributor

The in-kernel MPTCP implementation is still missing commonly used TCP features like TCP keepalive, see multipath-tcp/mptcp_net-next#383. At this time changing the default would break a lot of applications.

After the kernel gains support for it, we should first change how we detect MPTCP support so that older kernels without MPTCP TCP keepalive support are treated as if they have no MPTCP support. Then we can evaluate whether the default can be flipped.

@database64128
Copy link
Contributor

We currently enable TCP keepalive by default, as implemented in CL 107196 for #23459. But the current implementation ignores all related setsockopt errors. According to tcp(7), Linux has had support for TCP_KEEPIDLE and TCP_KEEPINTVL since v2.4, which is below our minimum requirement of v2.6.32.

Maybe we should update the code to close the socket and return an error if any of the related setsockopt calls fail. This would help us catch problems like this earlier in the process.

@matttbe
Copy link
Contributor Author

matttbe commented Feb 13, 2024

The in-kernel MPTCP implementation is still missing commonly used TCP features like TCP keepalive, see multipath-tcp/mptcp_net-next#383. At this time changing the default would break a lot of applications.

@database64128: Thank you for this reminder! I think we forgot about this, it should be easy to support TCP_KEEPIDLE, TCP_KEEPINTVL, TCP_KEEPCNT.

Just a small precision: MPTCP does support TCP Keep alive feature since v5.13 that is enabled via SO_KEEPALIVE. What is not supported yet is: changing the different timeout values via TCP_KEEPIDLE, TCP_KEEPINTVL, TCP_KEEPCNT which override net.ipv4.tcp_keepalive_* sysctl knobs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Accepted
Development

No branches or pull requests