Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 15 Nov 2021 08:16:43 -0800
From:      John Baldwin <jhb@FreeBSD.org>
To:        Randall Stewart <rrs@netflix.com>
Cc:        Jessica Clarke <jrtc27@freebsd.org>, Randall Ray Stewart <rrs@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@freebsd.org>
Subject:   Re: git: b8d60729deef - main - tcp: Congestion control cleanup.
Message-ID:  <5cef43ac-0c37-5b3b-c938-1024fc746cb0@FreeBSD.org>
In-Reply-To: <5A60AA15-F560-44D9-89A0-BD0A197E5E58@netflix.com>
References:  <202111111131.1ABBVH6s017371@gitrepo.freebsd.org> <DDB6E732-7054-4C57-ADFD-4534CBAD5109@freebsd.org> <2a564b11-b1f4-a4fe-745b-27f45fb134eb@FreeBSD.org> <5A60AA15-F560-44D9-89A0-BD0A197E5E58@netflix.com>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5cef43ac-0c37-5b3b-c938-1024fc746cb0>