Mark vnet loader tunnables with CTLFLAG_NOFETCH

[zlei]

Summary

With recent change D39638, the flag CTLFLAG_VNET can no longer prevent kernel from fetching value from kernel environment, mark vnet loader tunnables with CTLFLAG_NOFETCH to restore the previous behavior.

Fixes: D39638 sysctl(9): Enable vnet sysctl variables to be loader tunable
MFC after: 1 week

[tuexen]

Could you explain what the problem is?

[zlei]

In D41525#946274, @tuexen wrote:

Could you explain what the problem is?

The original comment is in D39638.

This may tigger panic if SYSCTL nodes are declared with SYSCTL_PROC and without flag CTLFLAG_NOFETCH, such as

SYSCTL_PROC(_net_inet_carp, OID_AUTO, allow,
    CTLFLAG_VNET | CTLTYPE_INT | CTLFLAG_RWTUN | CTLFLAG_MPSAFE,
    &VNET_NAME(carp_allow), 0, carp_allow_sysctl, "I",
    "Accept incoming CARP packets");

in sys/netinet/ip_carp.c .

I will post separated fixes for them.

[zlei]

Maybe it is better that SYSCTL_PROC be refined to include flag CTLFLAG_NOFETCH

/* Oid for a procedure.  Specified by a pointer and an arg. */
 #define        SYSCTL_PROC(parent, nbr, name, access, ptr, arg, handler, fmt, descr) \
-       SYSCTL_OID(parent, nbr, name, (access),                         \
+       SYSCTL_OID(parent, nbr, name, CTLFLAG_NOFETCH | (access),                               \
            ptr, arg, handler, fmt, descr);                             \
        CTASSERT(((access) & CTLTYPE) != 0)
 
@@ -821,7 +821,7 @@ TAILQ_HEAD(sysctl_ctx_list, sysctl_ctx_entry);
 ({                                                                     \
        CTASSERT(((access) & CTLTYPE) != 0);                            \
        SYSCTL_ENFORCE_FLAGS(access);                                   \
-       sysctl_add_oid(ctx, parent, nbr, name, (access),                \
+       sysctl_add_oid(ctx, parent, nbr, name, CTLFLAG_NOFETCH | (access),              \
            (ptr), (arg), (handler), (fmt), __DESCR(descr), NULL);      \
 })

[zlei]

In D41525#946280, @zlei wrote:

Maybe it is better that SYSCTL_PROC be refined to include flag CTLFLAG_NOFETCH

/* Oid for a procedure.  Specified by a pointer and an arg. */
 #define        SYSCTL_PROC(parent, nbr, name, access, ptr, arg, handler, fmt, descr) \
-       SYSCTL_OID(parent, nbr, name, (access),                         \
+       SYSCTL_OID(parent, nbr, name, CTLFLAG_NOFETCH | (access),                               \
            ptr, arg, handler, fmt, descr);                             \
        CTASSERT(((access) & CTLTYPE) != 0)
 
@@ -821,7 +821,7 @@ TAILQ_HEAD(sysctl_ctx_list, sysctl_ctx_entry);
 ({                                                                     \
        CTASSERT(((access) & CTLTYPE) != 0);                            \
        SYSCTL_ENFORCE_FLAGS(access);                                   \
-       sysctl_add_oid(ctx, parent, nbr, name, (access),                \
+       sysctl_add_oid(ctx, parent, nbr, name, CTLFLAG_NOFETCH | (access),              \
            (ptr), (arg), (handler), (fmt), __DESCR(descr), NULL);      \
 })

A second thought about it, well SYSCTL_PROC defines tunables with custom handler, the handler may or may not have side effect and CTLFLAG_NOFETCH seems too strict.
Here is one of the examples:

% grep -ErA3 'SYSCTL_PROC' sys | grep -Eb1 'CTLFLAG_R[DW]TUN'                                
2588-sys/kern/tty_info.c:SYSCTL_PROC(_kern, OID_AUTO, tty_info_kstacks,
2655:sys/kern/tty_info.c-    CTLFLAG_RWTUN | CTLFLAG_MPSAFE | CTLTYPE_INT, NULL, 0,
2734-sys/kern/tty_info.c-    sysctl_tty_info_kstacks, "I",
--

Currently only a combination of SYSCTL_PROC and CTLFLAG_VNET | CTLFLAG_TUN implicit requires`CTLFLAG_NOFETCH` but there're only few such combinations.

% grep -ErA3 'SYSCTL_PROC' sys | grep -Eb1 'CTLFLAG_VNET.*CTLFLAG_R[DW]TUN|CTLFLAG_R[DW]TUN.*CTLFLAG_VNET'
53349-sys/net/route/route_tables.c:SYSCTL_PROC(_net, OID_AUTO, fibs,
53412:sys/net/route/route_tables.c-    CTLFLAG_VNET | CTLTYPE_U32 | CTLFLAG_RWTUN | CTLFLAG_NOFETCH | CTLFLAG_MPSAFE,
53524-sys/net/route/route_tables.c-    NULL, 0, &sysctl_fibs, "IU",
--
73447-sys/netinet/tcp_fastopen.c:SYSCTL_PROC(_net_inet_tcp_fastopen, OID_AUTO, ccache_bucket_limit,
73541:sys/netinet/tcp_fastopen.c-    CTLFLAG_VNET | CTLTYPE_UINT | CTLFLAG_RWTUN | CTLFLAG_NEEDGIANT,
73637-sys/netinet/tcp_fastopen.c-    NULL, 0, &sysctl_net_inet_tcp_fastopen_ccache_bucket_limit, "IU",
--
84497-sys/netinet/ip_carp.c:SYSCTL_PROC(_net_inet_carp, OID_AUTO, allow,
84564:sys/netinet/ip_carp.c-    CTLFLAG_VNET | CTLTYPE_INT | CTLFLAG_RWTUN | CTLFLAG_MPSAFE,
84651-sys/netinet/ip_carp.c-    &VNET_NAME(carp_allow), 0, carp_allow_sysctl, "I",

[zlei]

Changed the summary to be more descriptive.

@tuexen Is it OK?

[zlei]

sys/netinet/tcp_fastopen.c

274 ↗

(On Diff #126311)

A quick test shows loader tunable net.inet.tcp.fastopen.ccache_bucket_limit does NOT work. I'll file a dedicated PR.

[zlei]

sys/netinet/tcp_fastopen.c

274 ↗

(On Diff #126311)

Please see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=273509 and D41691