On 2025-05-17 at 07:19:59, Muhammad Nuzaihan wrote: > Hi Brian. > > On Fri, May 16 2025 at 08:33:03 PM +0000, brian m. carlson > <sandals@xxxxxxxxxxxxxxxxxxxx> wrote: > > What happens here if I compile this on a system that has a kernel that > > supports MPTCP but then switch to one that does not? The reason I ask > > is that I have worked at places where we shipped binaries, including > > Git, based on a standard CentOS or RHEL system, but then some people > > used our software on a system with a very stripped down kernel (in some > > cases, where IPv6 was not even compiled in) because doing so meant that > > they could make about $5 more per server per month. > > > MPTCP supports *both* IPv4 and IPv6. Don't tell me people would also remove > even IPv4 as well? I had written an #ifdef statement to check if > IPPROTO_MPTCP > exists and enables that. I provide this as an example of people compiling even "essential" features out of their kernel. The question remains: if I compile on, say, Debian, which has this, and then I switch to the same version of Debian, but with a custom kernel that removes MPTCP from the kernel completely, does this change continue to work, or do we end up with an EINVAL from the `socket` call? I want to point out that the kernel and libc headers used to compile a binary need not reflect the actual code in the running kernel. With the advent of containers, people frequently run a different operating system inside a container than they do outside a container and thus we need to consider all of the possible combinations. > > Do the operating systems which support MPTCP make it a compulsory part > > of the TCP stack, or could we end up with cases where we're unable to > > connect here? > > > > In addition, Wikipedia mentions that FreeBSD has only IPv4 support, but > > I don't know if that's up to date. What happens if we run on a system > > where MPTCP is used, but it doesn't work with IPv6 and the only remote > > IP is IPv6? Do we fall back properly, or do things fail? > > This patch *specifically* targets Linux to check if IPPROTO_MPTCP exists > in the Linux system. I think you have not read my initial patch description > properly nor even read about the new changes for MPTCP. Git runs on lots of operating systems, not just Linux. If the case is that the `IPPROTO_MPTCP` #define is only ever available on Linux and no other operating system ever ships that option or ever will, then that's fine, but the commit message needs to say that. I know that many operating systems ship MPTCP, so I'm going to ask about how this works on some non-Linux systems because your commit message didn't explain that to me. > Please read up on how MPTCP falls back to regular TCP if it could not > connect using MPTCP. Again, your patch tells me how things work on Linux. I am interested in patches that work across a variety of other operating systems as well. > > I ask these questions not because I'm opposed to this feature but > > because I want to be sure we don't accidentally break things for users. > > > I'm not sure but you have not even bothered to read the documentation about > MPTCP. On the Git list, we try not to assume that everyone has read all of the technical documentation about a subject and instead we explain, at a high level, how the change is and how it's supposed to work. Your commit message should convince me (and everyone else, especially Junio, the maintainer) that your change is valuable and should be applied. > > I know that for instance Go 1.24 enabled MPTCP and that ended up causing > > problems in some environments, so I would recommend that we make this a > > configurable option instead. We can definitely default to MPTCP, but we > > probably need an option to fall back. > MPTCP v1 (again i am repeating myself) and not the old MPTCP v0 does the > fallback > more effectively. > > Do you know of any references that mentions that Go 1.24 with MPTCP enabled > (normally this is the current MPTCP v1) is causing the issues? I know that there were circumstances in which there could be kernel panics or similar problems with it enabled[0]. I haven't heard of actual network problems, though. Since most people were previously not using MPTCP and Go 1.24 enabled it by default, upgrading to that version caused some people's systems to panic under load. I do think that enabling features that cause Git to induce a kernel panic or the like, even though that's a bug in the kernel, should be configurable. > But what you explained about the downsides of MPTCP (without evidences) > and not even implementing MPTCP for git protocol does not make sense. I'm not arguing any downsides of MPTCP. I'm stating that we have a large variety of platforms that have to be supported and you haven't explained how this works or will work anywhere other than Linux; that there are people who compile out important features from their kernel and, though that is improvident, we should probably not break Git for them; and that we should be careful about enabling features which have been known to cause system problems. [0] https://www.wiz.io/vulnerability-database/cve/cve-2022-49198 -- brian m. carlson (they/them) Toronto, Ontario, CA
Attachment:
signature.asc
Description: PGP signature
