Re: git: b8d60729deef - main - tcp: Congestion control cleanup.
- In reply to: John Baldwin : "Re: git: b8d60729deef - main - tcp: Congestion control cleanup."
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 15 Nov 2021 16:26:12 UTC
On Mon, Nov 15, 2021 at 9:16 AM John Baldwin <jhb@freebsd.org> wrote: > 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. > To be fair, there is a simple level of pre-commit checking/CI that folks can opt into today. If you push your branch to github or gitlab, CirrusCI will run a simple smoke test and test-boot on both amd64 and arm64, though I don't know if that would have caught the panic due to different ordering issue or not (I've not tried it). There may be a registration of your fork with CirrusCI that's needed, though. I try to use this for any non-trivial change unless I've done a full build/install world/kernel cycle on the changes. If you are interested in this stuff, please subscribe to git@. I'm trying to have a discussion there, and it would benefit from more participation. > 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. > One thing that would have caught more problems, though, is a make universe prior to commit: That would have caught the now-broken config files. While it would be nice to get this Warner