Hi Willy, On 30/05/2024 16:08, Willy Tarreau wrote: > Hi Matthieu, > > finally a bit more available again... > > On Fri, Apr 26, 2024 at 06:34:02PM +0200, Matthieu Baerts wrote: >>> I *am* interested in the feature, which has been >>> floating around for a few years already. However I tend to agree with >>> Nicolas that, at least for the principle of least surprise, I'd rather >>> not have this turned on by default. There might even be security >>> implications that some are not willing to take yet, and forcing them >>> to guess that they need to opt out of something is not great in general >>> (it might change in the future though, once everyone turns it on by >>> default, like we did for keep-alive, threads, and h2 for example). >> >> It makes sense as well! I initially suggested to Dorian to first send a >> patch where MPTCP is enabled by default because of the low impact (and >> also after having seen an old comment on that topic [1]). But indeed, >> doing that in steps sounds safer. >> >> [1] https://github.com/haproxy/haproxy/issues/1028#issuecomment-754560146 > > Yeah I understand, it would essentially depend on how all of this evolves > over time and how adoption grows for mptcp. Maybe one day most users will > expect it to work by default.
I understand your position. >>> I'm also concerned by the change at the socket layer that will make all >>> new sockets cause two syscalls on systems where this is not enabled, >> >> I thought the listening sockets were created only once at startup and >> having two syscalls would have been fine. I guess it should be easy to >> remember the failure the first time, to avoid the extra syscalls for the >> next listening sockets. > > What you say is true for the listeners, but the socket() call is also > used when connecting to a backend server (though maybe I missed something > during the review). Oh sorry, I understood from Dorian that the modification he did was only impacting the listener sockets, it was not supposed to affect the others. Otherwise, we have the same behaviour as when we launch HAProxy with: mptcpize run haproxy > >>> and I'd really really prefer that we use the extended syntax for >>> addresses that offers a lot of flexibility. We can definitely have >>> "mptcp@address" and probably mptcp as a variant of tcp. >> >> >From what I understood, Dorian is now looking at that. It is not clear >> if he should create a new proto (struct protocol) or modifying it after >> having called protocol_lookup() in str2sa_range(). > > I *tend* to think that a new struct protocol is easier to add and > maintain. It could just share most (if not all) of the functions > with tcp, and probably be declared in the same file for ease of > sharing code. > > I'm seeing that in the comment you linked above I also proposed a > keyword for "bind" and "server" lines, like we have "tfo" to enable > TCP fast-open. It tends to be slightly easier to implement but then > requires more care everywhere because such options do no apply to > all protocols, so if you have "mptcp on" on a "bind quic4@:443" line, > that's quite confusing so it deserves extra checks to make sure it > is not silently ignored. Also I do have some doubts about how to > retrieve the source address, maybe we'll find that in fact it should > be seen as a new address family and not a new transport layer. But I > think not at this point. And BTW Björn had apparently implemented a > solution based on mptcp@ as well so it's likely workable. Yes, it looks better with a new 'struct protocol'. I worked on a first draft a few weeks ago: https://github.com/matttbe/haproxy/commit/f48f36191 As I mentioned in a previous email, I can wait for you to be back to send a new version on the mailing list. >> Can MPTCP be used as a variant of "rhttp" as well? > > I don't see how it should be related. The rhttp part is always tricky, > but the concept is that we accept a regular connection on a regular > frontend, then based on an explicit rule, return it and assign it as > an idle conn to a server. And for the server setting up pre-connected > idle conns to the gateway, it's just a regular address as well and the > protocol should not be involved there. Thank you, so "rhttp" is not a priority here for MPTCP. >>> Regarding how >>> we'd deal with the fallback, we'll see, but overall, thinking that >>> would explicitly configure mptcp and not even get an error if >>> it cannot bind is problematic to me, because among the most common >>> causes of trouble are things that everyone ignores (module not loaded, >>> missing secondary IP, firewall rules etc) that usually cause problems. >>> Those we can discover at boot time should definitely be reported. >> >> With the v1 Dorian suggested where MPTCP is enabled by default, a >> warning is reported if it is not possible to create an MPTCP socket, and >> a "plain" TCP one has been created instead. >> >> If MPTCP is explicitly asked, I guess it would make sense to fail if it >> is not possible to create an MPTCP socket, no? > > Yes I agree. I anticipate that if the solution eventually becomes > successful, some users will then want a 3-state setting: always-on, > always-off, best-effort. But let's not needlessly complexify the design > for now. Good idea! >>> But >>> let's discuss this in early June instead (I mean, feel free to discuss >>> the points together, but I'm not going to invest more time on this at >>> this moment). >> >> Thank you! >> >> Please note that Dorian is doing this as part of a work with his uni >> (useful contributions to Open-Source), and I think he would prefer >> sending the v2 before June, if that's OK. I can look at rebasing it >> later if needed. If there is no need to implement a fallback if the >> kernel doesn't support MPTCP, the patch should be smaller, it should be >> easy to rebase it. > > Yeah, understood. I will have limited availability for the next two > weeks but if he needs some guidance to finish something almost done, > I'll do my best. It's important to encourage first-time contributors, > especially since it's easy to want to give up on a first patch feedback, > we've all known that :-) Please take your time. As I mentioned in my previous email on the ML, Dorian is no longer available to work on that, so I will do my best to continue. No reason to rush, more to relax ;-) Cheers, Matt -- Sponsored by the NGI0 Core fund.
