From nobody Tue Nov 16 17:46:00 2021 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 92D0B189C0D1; Tue, 16 Nov 2021 17:46:02 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HttlG3q1cz3gxj; Tue, 16 Nov 2021 17:46:02 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from John-Baldwins-MacBook-Pro.local (ralph.baldwin.cx [66.234.199.215]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id E6D111FC5; Tue, 16 Nov 2021 17:46:01 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Subject: Re: git: b8d60729deef - main - tcp: Congestion control cleanup. To: Randall Stewart , src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org References: <202111111131.1ABBVH6s017371@gitrepo.freebsd.org> From: John Baldwin Message-ID: <9382fdb3-4cf5-41bf-626f-7fed2bf21045@FreeBSD.org> Date: Tue, 16 Nov 2021 09:46:00 -0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 In-Reply-To: <202111111131.1ABBVH6s017371@gitrepo.freebsd.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-ThisMailContainsUnwantedMimeParts: N On 11/11/21 3:31 AM, Randall Stewart wrote: > The branch main has been updated by rrs: > > URL: https://cgit.FreeBSD.org/src/commit/?id=b8d60729deefa0bd13e6a395fcab4928e6e10445 > > commit b8d60729deefa0bd13e6a395fcab4928e6e10445 > Author: Randall Stewart > AuthorDate: 2021-11-11 11:28:18 +0000 > Commit: Randall Stewart > CommitDate: 2021-11-11 11:28:18 +0000 > > tcp: Congestion control cleanup. > > NOTE: HEADS UP read the note below if your kernel config is not including GENERIC!! > > 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. > > 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. > > 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=\"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. > > Reviewed by: Michael Tuexen > Sponsored by: Netflix Inc. > RELNOTES:YES > Differential Revision: https://reviews.freebsd.org/D32693 FYI, I've seen a new LOR on shutdown that I think might be from this commit: lock order reversal: 1st 0xffffffff81dccc90 cc_list (cc_list, rw) @ /mnt/jhb/work/git/crypto/sys/netinet/cc/cc.c:245 2nd 0xffffffff81dcc510 vnet_rwlock (vnet_rwlock, rw) @ /mnt/jhb/work/git/crypto/sys/netinet/cc/cc.c:208 lock order cc_list -> vnet_rwlock attempted at: #0 0xffffffff80b6e803 at witness_checkorder+0x9a3 #1 0xffffffff80b0430d at __rw_rlock_int+0x8d #2 0xffffffff80b049ed at __rw_rlock+0xd #3 0xffffffff80cb8e76 at cc_check_default+0x46 #4 0xffffffff80cb8d2b at cc_deregister_algo+0x4b #5 0xffffffff80cb941a at cc_modevent+0x1a #6 0xffffffff80ae702b at module_shutdown+0x6b #7 0xffffffff80b0903e at kern_reboot+0x34e #8 0xffffffff80b0898a at sys_reboot+0x4a #9 0xffffffff80f7131c at syscallenter+0x10c #10 0xffffffff80f71045 at amd64_syscall+0x15 #11 0xffffffff80f492db at fast_syscall_common+0xf8 -- John Baldwin