Hi Matthieu, first, sorry for the long delay, but each time it's the same, the list of pending urgent things drags me away. I'm back on this.
On Mon, Jun 03, 2024 at 05:33:31PM +0200, Matthieu Baerts wrote: > >>> 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. That looks pretty interesting and clean like this! I find your approach of duplicating the tcp definition interesting, but I'd still prefer to have it pre-defined rather than duplicated from tcp though. It's the only one being done like this and I'm concerned that sooner or later we'll break it the dirty way by checking that a list element is empty, that a pointer is NULL or whatever. It's no big deal to have duplicated definitions like this, and it eases "git grep" to find all affected protocols when we want to touch a function. I'm just wondering if the "alt" argument (that you renamed from ctrl_dgram) is always exclusive with the mptcp one. It looks so at first glance. In the worst case this would just end up with a bit field instead of just a boolean, like many other functions. But yes, overall I think that the approach looks like the correct one, and it offers quite a lot of flexibility that way. Out of curiosity, did you test it on the backend side ? I don't know what it implies to do mptcp on the backend (if anything at all), it's more to make sure we're not overlooking anything. > >> 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 ;-) Sure, but I'd like to get this merged before 3.1 is out ;-) Feel free to send your patch(es) with a possibly updated commit messages here on the list. I don't know if we can make a portable enough regtest for this, especially if we consider that a silent fallback might lead to TCP being used. Thanks! Willy
