Re: git: b8d60729deef - main - tcp: Congestion control cleanup.

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 <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
> 
>      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