Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 15 Nov 2021 09:26:12 -0700
From:      Warner Losh <imp@bsdimp.com>
To:        John Baldwin <jhb@freebsd.org>
Cc:        Randall Stewart <rrs@netflix.com>, 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:  <CANCZdfq5za83xACVVA1UFQG0dx2wM_UC%2B55_-b9zjR9_iZQceQ@mail.gmail.com>
In-Reply-To: <5cef43ac-0c37-5b3b-c938-1024fc746cb0@FreeBSD.org>
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> <5cef43ac-0c37-5b3b-c938-1024fc746cb0@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
--0000000000000bc8c005d0d64526
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

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=E2=80=
=99s a
> > good solution.
> >
> > And I think Warner=E2=80=99s patch with an ifndef in cc.c works perfect=
ly that
> > way if you are say netapp and don=E2=80=99t use newreno you can do a
> >
> > nooptions CC_NEWRENO
> > options CC_CUBIC
> > options CC_DEFAULT=3D\=E2=80=9Dcubic\=E2=80=9D
> >
> > 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 ca=
n
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 chang=
e
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

--0000000000000bc8c005d0d64526--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfq5za83xACVVA1UFQG0dx2wM_UC%2B55_-b9zjR9_iZQceQ>