Date: Tue, 27 Nov 2012 11:34:09 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Garrett Cooper <yanegomi@gmail.com> Cc: freebsd-net@freebsd.org Subject: Re: [RFC] Better document net.inet6 sysctls and prune dead sysctls Message-ID: <20121127103538.O1832@besplex.bde.org> In-Reply-To: <alpine.BSF.2.00.1211261436070.31097@toaster.local> References: <alpine.BSF.2.00.1211261436070.31097@toaster.local>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 26 Nov 2012, Garrett Cooper wrote:
> As noted in a previous thread, I set out to better document the
> net.inet6 sysctls after having to tweak the knobs to get things to work for
> TAHI, and this is the resulting draft (so far). I also took the liberty of
> removing the ip6_rr_prune and icmp6_redirtimeout sysctls because they weren't
> in use anywhere else in the sys/... portion of the tree. I was wondering if
> there are any points that should be corrected/clarified before I submit a PR
> with the resulting patch.
It would be good to fix the style bugs when changing lots.
> Index: sys/netinet6/in6_proto.c
> ===================================================================
> --- sys/netinet6/in6_proto.c (revision 242903)
> +++ sys/netinet6/in6_proto.c (working copy)
> @@ -443,7 +441,6 @@
>
> /* ICMPV6 parameters */
> VNET_DEFINE(int, icmp6_rediraccept) = 1;/* accept and process redirects */
> -VNET_DEFINE(int, icmp6_redirtimeout) = 10 * 60; /* 10 minutes */
> VNET_DEFINE(int, icmp6errppslim) = 100; /* 100pps */
> /* control how to respond to NI queries */
> VNET_DEFINE(int, icmp6_nodeinfo) =
> @@ -515,15 +512,20 @@
> }
>
> SYSCTL_VNET_INT(_net_inet6_ip6, IPV6CTL_FORWARDING, forwarding, CTLFLAG_RW,
> - &VNET_NAME(ip6_forwarding), 0, "");
> + &VNET_NAME(ip6_forwarding), 0,
> + "Forward IPv6 packets via node.");
All the sysctl names spell ipv6 without a v. This is hard to fix. This
bug is missing in the names of the sysctl numbers, but it is a bug to
used named numbers instead of OID_AUTO in code newer than OID_AUTO, and
ipv6 is much newer.
New style bug in almost every description. Sysctl descriptions are not
normally terminated by a ".".
Old style bug in almost every ipv6 sysctl. Large SYSCTL declarations are
normally normally-indented, with 4-space continuation indents. This rule
is broken farily consistenly in ipv6 sysctl. More ivp4 SYSCTL descriptions
are actually formatted normally.
> @@ -594,35 +613,45 @@
> #define V_ip6_output_flowtable_size
> VNET(ip6_output_flowtable_size)
>
> SYSCTL_VNET_INT(_net_inet6_ip6, OID_AUTO, output_flowtable_size,
> CTLFLAG_RDTUN,
> - &VNET_NAME(ip6_output_flowtable_size), 2048,
> - "number of entries in the per-cpu output flow caches");
> + &VNET_NAME(ip6_output_flowtable_size), 2048,
> + "number of entries in the per-cpu output flow caches");
> ...
Here the formatting change is backwards (from normal continuation indent
to abnormal ipv6 continuation indent.
This and some other old sysctl descriptions have differnent style bugs:
they are normally terminated (not with a '.'), but are not normally
capitalized (with capaitals).
> SYSCTL_VNET_INT(_net_inet6_icmp6, ICMPV6CTL_ND6_PRUNE, nd6_prune,
> CTLFLAG_RW,
Some mailer mangled the formatting (the line isn't too long and shouldn't
have been mangled).
> - &VNET_NAME(nd6_prune), 0, "");
> + &VNET_NAME(nd6_prune), 0,
> + "Period (in seconds) for performing nd6 expiration checks of default
> "
More mangling by some mailer.
> + "routes and prefix lists");
However, the string is too long. The message is obfuscated by splitting it.
This keeps the line length short in the source code, but it is still too
long in the output. Sysctl descriptions should be no longer than 50 or
60 characters, since the sysctl name will expand the output by 20 or 30
characters. sysctl names longer than 20 or 30 are a larger bug, since
they are hard to write as well as hard to read.
This sysctl description is normally terminated (not with a ".").
> ...
> SYSCTL_VNET_INT(_net_inet6_icmp6, ICMPV6CTL_ND6_UMAXTRIES, nd6_umaxtries,
> - CTLFLAG_RW, &VNET_NAME(nd6_umaxtries), 0, "");
> + CTLFLAG_RW, &VNET_NAME(nd6_umaxtries), 0,
> + "Maximum number of unicast queries to perform via nd6.");
Another type of sysctl misformatting is generated by names that are so
verbose that CTLFLAG* cannot be formatted normally (on the first line).
I missed earlier instances of these bugs.
Statistic on output style in sysctl descriptions: on freefall now:
sysctl -da: 3546 lines
sysctl -da | grep -v ': *$': 3192 lines
[this filters out sysctls with null descriptions. The output is
misformatted with a space before the null description]
sysctl -nda: 3546 lines
sysctl -nda | grep -v '^: *$': 3192 lines
[since this 3192 is the same as the number of sysctls with non-null
descreiptions, there must be no multi-line descriptions with embedded
newlines]
sysctl -nda | grep -v '^: *$' | grep -v '^[A-Z]':
1555 lines
[most of non-capitalizations are style bugs. The last 1395 of them
are automatically generated for device sysctls. This is part of 5
lines of automatically generated spam for device sysctls (descriptions
are duplicated ad spamium for %desc, %driver, %location, %pnpinfo and
%parent). A few are correct because they are for a proper name or
keyword or a number]
sysctl -nda | grep -v '^: *$' | grep '\.$':
74 lines
[the style bug of terminating with a '.' is uncommon. In a few cases
it goes with the larger style bug of a multi-line description]
sysctl -nda | grep -v '^: *$' | grep '.... [81 dots]':
11 lines
[these lines are too long even without the sysctl names. Abnormal
termination is denser than usual in these lines (8 of 11)]
sysctl -nda | grep -v '^: *$' | grep '\..*\.':
8 lines
[2 of these are for multi-sentence descriptions].
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121127103538.O1832>