golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.83k stars 17.65k forks source link

net: add support for MPTCP #56539

Closed matttbe closed 1 year ago

matttbe commented 1 year ago

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?

apparentlymart commented 1 year ago

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 commented 1 year ago

cc @neild

matttbe commented 1 year ago

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:

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:

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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:

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 commented 1 year ago

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 commented 1 year ago

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

neild commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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,

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 commented 1 year ago

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 commented 1 year ago

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

rsc commented 1 year ago

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

matttbe commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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

apparentlymart commented 1 year ago

Clarification about the approved API:

If I call SetMultipathTCP(true), does that mean "use multipath TCP if supported by the OS, and use normal TCP otherwise", or does it mean "use multipath TCP or return an error if that isn't possible"?

If it's the former, should there also be a function to query for whether multipath TCP is available, so that applications which require it could fail early when it's not possible? Or would the assumption be that it's never reasonable to require multipath TCP, and so an available fallback to traditional TCP is always okay?

(when I say "return an error" above I of course mean when subsequently calling Dial or Listen, rather than immediately when calling SetMultipathTCP, because SetMultipathTCP itself cannot return an error and so presumably shouldn't be doing anything other than setting a flag somewhere.)

Jorropo commented 1 year ago

I've looked at implementing this and using geters and seters methods for Dialer and ListenConfig feels wrong. They both use public struct fields for all options right now, and I don't fully understand the point of having methods if not for doing automatic feature detection. However if you want to do automatic feature detection in thoses methods, I feel they should be able to return an error type as this would require IO.

ianlancetaylor commented 1 year ago

@apparentlymart I believe that the semantics for SetMultipathTCP should be "use MPTCP if available, fall back if not." For the case where a program absolutely must get MPTCP for some reason, it can check whether a socket uses MPTCP by using the SyscallConn method and calling unix.GetsockoptInt(sock, unix.SOL_SOCKET, unix.SO_DOMAIN).

@Jorropo The problem with an exported field is that this is a three-way setting: use the system default, never use MPTCP, use MPTCP if available. It seems that a method is clearer.

Jorropo commented 1 year ago

The problem with an exported field is that this is a three-way setting: use the system default, never use MPTCP, use MPTCP if available. It seems that a method is clearer.

This probably is the wrong time for me to say that once this has been accepted but I would have done:

type MultipathTCPBehaviour uint8

const (
  MultipathTCPUseSystemDefault uint8 = iota
  MultipathTCPNever
  MultipathTCPIfAvailable
  MultipathTCPAlways // error if mptcp is not available, mostly usefull for listeners
)

func (b MultipathTCPBehaviour) String() string {
  switch b {
  // ...  
  }
}

type Dialer struct {
  // ...

  // MultipathTCP defines what stence should be followed when trying to use Multipath TCP
  MultipathTCP MultipathTCPBehaviour
}
matttbe commented 1 year ago

For the case where a program absolutely must get MPTCP for some reason, it can check whether a socket uses MPTCP by using the SyscallConn method and calling unix.GetsockoptInt(sock, unix.SOL_SOCKET, unix.SO_DOMAIN).

@ianlancetaylor indeed. Just a small detail: you will need to use SO_PROTOCOL (to get IPPROTO_MPTCP) instead of SO_DOMAIN (which will get AF_INET for example).

Please note that if the program wants to know if a fallback to TCP happened, it can be done via getsockopt(SOL_MPTCP, MPTCP_INFO) and check for errors as explained there (EOPNOTSUPP is expected): https://github.com/multipath-tcp/mptcp_net-next/issues/294#issuecomment-1301920288

apparentlymart commented 1 year ago

Based on what's been discussed so far, assuming we'll use the API that was accepted with the clarification from my question above, it seems like the most straightforward implementation on Linux would be:

  1. In Dial/Listen, check whether the internal "try to use MPTCP" flag is set. If not, skip immediately to step 4.
  2. Try to open an IPPROTO_MPTCP socket. If that succeeds, use the resulting socket and stop.
  3. If the error from step 2 is anything other than "not supported", return that error and stop.
  4. Try to open an IPPROTO_TCP socket. If that succeeds, use the resulting socket and stop.
  5. If we reach this step, return the error from step 4.

The internal flag would, as described above, have a default value that may vary over different versions of Go. Today's existing versions of Go behave as if it is always false. Future versions of Go might set it to true by default in some cases.

If the flag is true, that does mean an extra system call when running on a Linux system whose kernel does not yet support IPPROTO_MPTCP, before falling back on the original call to use IPPROTO_TCP. Calling SetMultipartTCP(false) forces skipping that extra system call. Does that seem reasonable?

Perhaps the approach would be to wait until some suitable percentage of Linux targets are likely to have IPPROTO_MPTCP support before making the flag be on by default?

matttbe commented 1 year ago

That looks like a good plan!

If the error from step 2 is anything other than "not supported", return that error and stop.

A small hint for the implementer: if I'm not mistaken, there are three expected errors when creating an IPPROTO_MPTCP sockets that certainly justify a fallback to TCP:

If the flag is true, that does mean an extra system call when running on a Linux system whose kernel does not yet support IPPROTO_MPTCP, before falling back on the original call to use IPPROTO_TCP.

That's probably not an issue from my point of view: if MPTCP is not supported, the kernel will quickly return an error without allocating resources for a new socket, etc.

Note that if the error you get at step 2 is EPROTONOSUPPORT or EINVAL, it means the current kernel doesn't support MPTCP. An implementer can then cache the info the first time and directly go to step 4 next time a stream socket is needed.

matttbe commented 1 year ago

Perhaps the approach would be to wait until some suitable percentage of Linux targets are likely to have IPPROTO_MPTCP support before making the flag be on by default?

Note that MPTCP is supported by RHEL 8 and Ubuntu 22.04 (also by Ubuntu 20.04.5 from what I see). But I don't know what it represents in term of percentage.

ianlancetaylor commented 1 year ago

@matttbe Thanks for the SO_PROTOCOL correction.

I agree that we should just cache a failed MPTCP call the first time.

matttbe commented 1 year ago

Hello,

FYI, a few days ago, we started to look at adding support for MPTCP in the net package.

We have a few commits in this branch: https://github.com/multipath-tcp/go/commits/wip I'm personally not a Go developers so I'm sorry if some ideas or modifications we did were totally wrong or if we misunderstood some messages here.

Before submitting our modifications, we have some questions about the global design we picked. The current version adds MPTCP capability to TCPConn and TCPListener objects. At the end, we can use it like that (here, without handling errors to make the code shorter):

d := &net.Dialer{UseMultipathTCP: true}  // force MPTCP on
conn, err := d.Dial("tcp", addr)         // MPTCP is used if available
tcp, ok := conn.(*net.TCPConn)           // it should be a TCPConn
mptcp, err := tcp.MultipathTCP()         // we can check if a fallback has been done

It is needed to pass the information that we want an MPTCP socket before creating the socket. We can then not create the conn / listen object and then do a .SetMultipathTCP(true).

Would it be OK to use it like that?


Also, we wanted Dial / Listen to return new MPTCPConn or MPTCPListener objects "extending" the TCP ones. By doing that, we could add MPTCP specific features only available when using an MPTCP object. But if we are not mistaken, we think it is not a good idea to do that if one day, MPTCP is used by default: a different object will be returned when invoking Dial() or Listen() and this could break apps checking the returned object, no? Or maybe there is a way to have the returned object still being seen as a TCP one somehow?

Thank you!

Jorropo commented 1 year ago

Or maybe there is a way to have the returned object still being seen as a TCP one somehow?

No.

Go allows you to do composition:

type MPTCPConn struct{
  *TCPConn
}

however it will be seen as this different type:

var someMPTCPConn *MPTCPConn

var interfaceObject net.Conn = (*MPTCPConn)(nil)

tcpConn, ok := interfaceObject.(*TCPConn)
fmt.Println(ok) // false, the type assertion failed
szuecs commented 1 year ago

@mattbe How would this look like for containers that have only one virtual network device? Would this automatically use TCP or try MPTCP?

My question is because I would be happy to have MPTCP in "edge" cases, but maybe not inside the "data center" (or cluster) for service to service communication. As most communication in my case is not "edge", I am not sure if I want to have MPTCP enabled by default.

I am looking forward to play around with MPTCP and this makes it simple for me, so many thanks :)

matttbe commented 1 year ago

How would this look like for containers that have only one virtual network device? Would this automatically use TCP or try MPTCP?

@szuecs thank you for the question. I think it is important to distinguish client vs server use-cases. For example here, your server could have one interface while the client might want to use multiple paths because it has access to different networks.

Also, it is important to say that on the server side, if you create an MPTCP listening socket and the connection request you get is in "plain" TCP (without MPTCP options), the kernel will totally fallback to plain TCP: the impact is only limited to the very beginning of the connection (checking if there are MPTCP options in the TCP SYN request).

My question is because I would be happy to have MPTCP in "edge" cases, but maybe not inside the "data center" (or cluster) for service to service communication. As most communication in my case is not "edge", I am not sure if I want to have MPTCP enabled by default.

I understand. Even if the impact is low, it is important to be able to disable it if there is no need to use it.

Please note that a few scientific studies have shown an interest to have MPTCP inside a data centre to benefit from the multiple paths available between nodes: failover or speed boost use cases.

I am looking forward to play around with MPTCP and this makes it simple for me, so many thanks :)

That's good there is an interest :-)

matttbe commented 1 year ago

however it will be seen as this different type:

@Jorropo thank you for the confirmation!

Maybe we overthought about that: would it be an issue if Dial and Listen return a different type (e.g. MPTCPConn vs TCPConn)?

gopherbot commented 1 year ago

Change https://go.dev/cl/471135 mentions this issue: net: mptcp: add MPTCP Sock

gopherbot commented 1 year ago

Change https://go.dev/cl/471140 mentions this issue: net: mptcp: add MultipathTCP checker

gopherbot commented 1 year ago

Change https://go.dev/cl/471138 mentions this issue: net: mptcp: fallback to TCP in case of any error

gopherbot commented 1 year ago

Change https://go.dev/cl/471139 mentions this issue: net: poll: add GetsockoptInt

gopherbot commented 1 year ago

Change https://go.dev/cl/471141 mentions this issue: net: mptcp: add end-to-end test

gopherbot commented 1 year ago

Change https://go.dev/cl/471137 mentions this issue: net: mptcp: implement listenMPTCP

gopherbot commented 1 year ago

Change https://go.dev/cl/471136 mentions this issue: net: mptcp: implement dialMPTCP

matttbe commented 1 year ago

As mentioned by @gopherbot, we just sent a first version / draft to Gerrit: https://go-review.googlesource.com/c/go/+/471135 (and following changes).

The last Gerrit Change is a test showing how to use MPTCP (we didn't look at the doc yet, we prefer to know if we are on the right track).

Do not hesitate to have a look and share comments :-) Especially on the questions mentioned above:

Thanks!

matttbe commented 1 year ago

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

In the suggestion we sent to Gerrit, we wrongly used UseMultipathTCP() instead of SetMultipathTCP(). We are fixing that.

For these SetMultipathTCP() methods, it makes sense to have them attached to a Dialer or ListenConfig because this needs to be set before creating the socket. But for MultipathTCP() and because MPTCP is used here as an extension to TCP, would it not be better to add a new method attached to TCPConn? e.g.

pkg net, method (*TCPConn) MultipathTCP() (bool, error) #56539

That's what we proposed in Gerrit. It is similar to other TCP options supported by GO: KeepAlive, No Delay and Linger. We are not sure where we can ask if it is OK to change that :)

ianlancetaylor commented 1 year ago

I'm not sure I get it--why should the MultipathTCP method return an error? The intent was that it reports whether or not the actual connection is using MPTCP. There are no KeepAlive, NoDelay, or Linger methods in the net package, so I'm not sure exactly what you are referring to.

matttbe commented 1 year ago

I'm not sure I get it--why should the MultipathTCP method return an error? The intent was that it reports whether or not the actual connection is using MPTCP.

Yes, we probably don't need to return an error. We only return an error if the FD is not OK, similar to what is done for functions setting socket options. But here, we only want to get the status, not changing anything so yes, we can remove the error.

There are no KeepAlive, NoDelay, or Linger methods in the net package, so I'm not sure exactly what you are referring to.

Sorry, I was referring to the methods in tcpsock.go above where we declared the new MultipathTCP method.

gopherbot commented 1 year ago

Change https://go.dev/cl/477735 mentions this issue: net: dial: add mptcpStatus type

andrius4669 commented 1 year ago

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 commented 1 year ago

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