Re: git: b8d60729deef - main - tcp: Congestion control cleanup.
Date: Mon, 15 Nov 2021 16:16:43 UTC
On 11/14/21 7:03 AM, Randall Stewart wrote: > John: > > This is fine to do, but I want to make sure everyone understands that > I was specifically asked to make compile fail on the transport call > if CC_XXXX or CC_DEFAULT was not defined. Its not how I had the code > originally but it was requested specifically. > > I am fine with all the changes aka it showing up in DEFAULT that’s a > good solution. > > And I think Warner’s patch with an ifndef in cc.c works perfectly that > way if you are say netapp and don’t use newreno you can do a > > nooptions CC_NEWRENO > options CC_CUBIC > options CC_DEFAULT=\”cubic\” > > And it all just works for you ;) No worries. I think this is one of those cases where some things just aren't obvious until subjected to wider testing. You sought review (and got a fair bit of it), and without some kind of available pre-commit CI I don't know that we can expect folks to boot changes in qemu for all architectures by hand prior to commit (which I think might have been the only realistic way to catch the breakage on arm64 or the vnet issues). I do think one of the goals of Warner's group is to figure out a way to provide some level of pre-commit CI that folks can opt into. FWIW, the arm64 breakage wasn't really due to the changes in this commit either, it was just that this commit exposed a longstanding bug in the hhook code that hadn't yet surfaced. -- John Baldwin