On Thu, Nov 11, 2021 at 11:31:17AM +0000, 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 <r...@freebsd.org>
> AuthorDate: 2021-11-11 11:28:18 +0000
> Commit: Randall Stewart <r...@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
Hi Randall,
This change causes a panic when starting up a vnet jail.
V_default_cc_ptr is NULL in the new vnet, it doesn't automatically get
copied from the parent vnet. I wrote the patch below and it allows my
system to boot, but it's just a hack and I'm not sure what the right
policy is.
diff --git a/sys/netinet/cc/cc.c b/sys/netinet/cc/cc.c
index 0a61aff37c96..f5ec10e487cf 100644
--- a/sys/netinet/cc/cc.c
+++ b/sys/netinet/cc/cc.c
@@ -309,6 +309,23 @@ cc_register_algo(struct cc_algo *add_cc)
return (err);
}
+static void
+vnet_cc_sysinit(void *arg)
+{
+ struct cc_algo *cc;
+
+ if (IS_DEFAULT_VNET(curvnet))
+ return;
+
+ CURVNET_SET(vnet0);
+ cc = V_default_cc_ptr;
+ CURVNET_RESTORE();
+
+ V_default_cc_ptr = cc;
+}
+VNET_SYSINIT(vnet_cc_sysinit, SI_SUB_PROTO_IFATTACHDOMAIN, SI_ORDER_ANY,
+ vnet_cc_sysinit, NULL);
+
/*
* Perform any necessary tasks before we exit congestion recovery.
*/
