Date: Sun, 14 Nov 2021 10:03:06 -0500 From: Randall Stewart via dev-commits-src-all <dev-commits-src-all@freebsd.org> To: John Baldwin <jhb@freebsd.org> 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: <5A60AA15-F560-44D9-89A0-BD0A197E5E58@netflix.com> In-Reply-To: <2a564b11-b1f4-a4fe-745b-27f45fb134eb@FreeBSD.org> References: <202111111131.1ABBVH6s017371@gitrepo.freebsd.org> <DDB6E732-7054-4C57-ADFD-4534CBAD5109@freebsd.org> <2a564b11-b1f4-a4fe-745b-27f45fb134eb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Apple-Mail=_3E97E46A-D01D-461F-9B18-231FA993F6C0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 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.=20 And I think Warner=E2=80=99s patch with an ifndef in cc.c works = perfectly 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 ;) R > On Nov 12, 2021, at 12:01 PM, John Baldwin <jhb@freebsd.org> wrote: >=20 > On 11/12/21 8:19 AM, Jessica Clarke wrote: >> On 11 Nov 2021, at 11:31, Randall Stewart <rrs@FreeBSD.org> wrote: >>>=20 >>> The branch main has been updated by rrs: >>>=20 >>> URL: = https://www.google.com/url?q=3Dhttps://cgit.FreeBSD.org/src/commit/?id%3Db= 8d60729deefa0bd13e6a395fcab4928e6e10445&source=3Dgmail-imap&ust=3D16373413= 08000000&usg=3DAOvVaw0ox_w6BYoj1_WxabaN9-lG >>>=20 >>> commit b8d60729deefa0bd13e6a395fcab4928e6e10445 >>> Author: Randall Stewart <rrs@FreeBSD.org> >>> AuthorDate: 2021-11-11 11:28:18 +0000 >>> Commit: Randall Stewart <rrs@FreeBSD.org> >>> CommitDate: 2021-11-11 11:28:18 +0000 >>>=20 >>> tcp: Congestion control cleanup. >>>=20 >>> NOTE: HEADS UP read the note below if your kernel config is not = including GENERIC!! >>>=20 >>> This patch does a bit of cleanup on TCP congestion control = modules. There were some rather >>> interesting surprises that one could get i.e. where you use a = socket option to change >>> from one CC (say cc_cubic) to another CC (say cc_vegas) and you = could in theory get >>> a memory failure and end up on cc_newreno. This is not what one = would expect. The >>> new code fixes this by requiring a cc_data_sz() function so we = can malloc with M_WAITOK >>> and pass in to the init function preallocated memory. The CC init = is expected in this >>> case *not* to fail but if it does and a module does break the >>> "no fail with memory given" contract we do fall back to the CC = that was in place at the time. >>>=20 >>> This also fixes up a set of common newreno utilities that can be = shared amongst other >>> CC modules instead of the other CC modules reaching into newreno = and executing >>> what they think is a "common and understood" function. Lets put = these functions in >>> cc.c and that way we have a common place that is easily findable = by future developers or >>> bug fixers. This also allows newreno to evolve and grow support = for its features i.e. ABE >>> and HYSTART++ without having to dance through hoops for other CC = modules, instead >>> both newreno and the other modules just call into the common = functions if they desire >>> that behavior or roll there own if that makes more sense. >>>=20 >>> Note: This commit changes the kernel configuration!! If you are = not using GENERIC in >>> some form you must add a CC module option (one of CC_NEWRENO, = CC_VEGAS, CC_CUBIC, >>> CC_CDG, CC_CHD, CC_DCTCP, CC_HTCP, CC_HD). You can have more than = one defined >>> as well if you desire. Note that if you create a kernel = configuration that does not >>> define a congestion control module and includes INET or INET6 the = kernel compile will >>> break. Also you need to define a default, generic adds 'options = CC_DEFAULT=3D\"newreno\" >>> but you can specify any string that represents the name of the CC = module (same names >>> that show up in the CC module list under net.inet.tcp.cc). If you = fail to add the >>> options CC_DEFAULT in your kernel configuration the kernel build = will also break. >> Not doing so breaks tinderbox, as well as configs not hooks up to >> tinderbox. I don=E2=80=99t think this is acceptable. >=20 > We discussed this a bit on IRC, but I think in this case the default = CC_* > options belong in DEFAULTS like the default GEOM_PART_* options rather > than in GENERIC. (Though we mostly avoid changing DEFAULTS, this is = one > of the rare cases when I think it makes sense.) Handling the default = for > CC_DEFAULT does not work in DEFAULTS since you can't later override = it, > but that could be handled by simply having the default for CC_DEFAULT = live > in the code itself under an #ifndef instead. >=20 > I think Warner is already testing a patchset to make this change. >=20 > --=20 > John Baldwin ------ Randall Stewart rrs@netflix.com --Apple-Mail=_3E97E46A-D01D-461F-9B18-231FA993F6C0 Content-Disposition: attachment; filename=smime.p7s Content-Type: application/pkcs7-signature; name=smime.p7s Content-Transfer-Encoding: base64 MIAGCSqGSIb3DQEHAqCAMIACAQExDzANBglghkgBZQMEAgEFADCABgkqhkiG9w0BBwEAAKCCAzYw ggMyMIICGqADAgECAgqxywKqrHPB2ybTMA0GCSqGSIb3DQEBCwUAMEcxGDAWBgNVBAMTD1JhbmRh bGwgU3Rld2FydDEeMBwGCSqGSIb3DQEJARYPcnJzQG5ldGZsaXguY29tMQswCQYDVQQGEwJVUzAe Fw0yMTAxMjQxMjIwMTRaFw0yNjAxMjQxMjIwMTRaMEcxGDAWBgNVBAMTD1JhbmRhbGwgU3Rld2Fy dDEeMBwGCSqGSIb3DQEJARYPcnJzQG5ldGZsaXguY29tMQswCQYDVQQGEwJVUzCCASIwDQYJKoZI hvcNAQEBBQADggEPADCCAQoCggEBAMUAht2nr/NFlK+tmmN9PdO3DBPfeYh9fLcbVihR+/dipO41 AsFy9y+2uDVaFhTEvp406P0o9PQQTuYXqrCr76eWQIj3V787e1WKjTup1mIyQeWHGf1gvb/7vmI2 zHg6QZEIC4W8xeO8SLKyHiwlFHZn8Rn1HxtB7Ge+NulygkgUgJYhXD5E29jVGXAc6Qn9Vr9AexPf KaOhHCaNB/Twcinayz6D8CO/Ym1LOs3+ceSOa4cB07fepmbqDSXDkOeA3U7KLaluHrRTlj6DO+JU nqKXX7jJ68KTYSZ0qH4fZsk8cxFkwYI/3HDJi+oF+FDkf7SRo1Q2w+e3M/5MReLIQ7sCAwEAAaMg MB4wDwYJKoZIhvcvAQEKBAIFADALBgNVHQ8EBAMCB4AwDQYJKoZIhvcNAQELBQADggEBAHJfum1j 1WIVFjOJT/hqMIN751aXkablmwesW94lNJKjslPULbbcP5nZGg2lGpHcZ+0I5F/1TTiEsT2H2rhA uAnSsUxTpxRA+aoe+xtJOa5vle3CprhFkHAvB7EIoLiNaPd0DNK6kKYsbvr5Z5Eq7TF3SIO77Qh9 /8VgUfOb0ARDgix59Q6MM0NmIabEwh0cDWQYlGgDDtN9DNk5PGM4pjs48suwEdLmFTMOmGTkCp7I Vq6iHDNinBiB6+BB4VYMAO1o9qS+0pnfdmPJybt0zVGrhm/c1Fmm3Jec7NEuiKeXmhPIwdwMkKyp AsX0sHuFCYwioBTHHZpvnir+H2rRakgxggHrMIIB5wIBATBVMEcxGDAWBgNVBAMTD1JhbmRhbGwg U3Rld2FydDEeMBwGCSqGSIb3DQEJARYPcnJzQG5ldGZsaXguY29tMQswCQYDVQQGEwJVUwIKscsC qqxzwdsm0zANBglghkgBZQMEAgEFAKBpMBgGCSqGSIb3DQEJAzELBgkqhkiG9w0BBwEwHAYJKoZI hvcNAQkFMQ8XDTIxMTExNDE1MDMwNlowLwYJKoZIhvcNAQkEMSIEIJlRgPgZmE4Z4oF3GOM8jT1T a48Wr3V3P8qI6+oMIuy2MA0GCSqGSIb3DQEBCwUABIIBAFFMHMOwajPGior49lS47vi3koSb2WMz NTHAFbCuJwKZGNGc/LrbWIKbE+pSq2k1+Fw2OFbglvX+PYmaQAxTvigLHip9aYuo4kLaZDhXplfE 0htFWkaP1yJwa7L+dq1gXpPQour+qoNDX26KtKNZNaxJ/uVxJ5ky1NitXeq2A3ca1Ez68/LhnTd2 cHm6ge+Tunu26hZggJuaWwPobEUxLhb9M3EAwU+EUwIHUcC+jcLYjjo6qeVprBwswFFSzQ/WXvS3 s+0bcu/9+2P0qL/LYDOey4+xqhVX+OlB4jPJf9Jrx6e72BT2usGMCpoToJco/mbTSpy9vnBRfsYm Xr/WyEEAAAAAAAA= --Apple-Mail=_3E97E46A-D01D-461F-9B18-231FA993F6C0--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5A60AA15-F560-44D9-89A0-BD0A197E5E58>