I am not a fan of this strange behaviour, where the min+max values
have additional behaviours. It is too surprising, and surprising
often turns into error-prone.
Alexander Bluhm <alexander.bl...@gmx.net> wrote:
> On Sun, Apr 25, 2021 at 10:08:08AM -0700, Greg Steuck wrote:
> > - { IPCTL_ARPQUEUED, &la_hold_total, 0, 1000 },
> > + { IPCTL_ARPQUEUED, &la_hold_total, 1, 0 },
> >
> > Will make la_hold_total read-only via sysctl.
>
> This feature surprises me. When reading the caller, is is not clear
> what it does. The between min and max sematics is quite obvious,
> for min > max you have to read the callee.
>
> It shoud be I sysctl_bounded_arr(9) man page. A comment may also
> help.
>
> { IPCTL_ARPQUEUED, &la_hold_total, 1, 0 }, /* read only */
>
> > In the meantime, does this look OK?
>
> As this feature is already used elsewhere, OK bluhm@. But please
> find a way to make it more obvious. When I commited my change, I
> did not see, that min > max would solve my problem.
>
> bluhm
>
> > diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
> > index 7578303d544..5d43458ac88 100644
> > --- a/sys/netinet/ip_input.c
> > +++ b/sys/netinet/ip_input.c
> > @@ -126,6 +126,7 @@ const struct sysctl_bounded_args ipctl_vars[] = {
> > { IPCTL_IPPORT_MAXQUEUE, &ip_maxqueue, 0, 10000 },
> > { IPCTL_MFORWARDING, &ipmforwarding, 0, 1 },
> > { IPCTL_MULTIPATH, &ipmultipath, 0, 1 },
> > + { IPCTL_ARPQUEUED, &la_hold_total, 1, 0 },
> > { IPCTL_ARPTIMEOUT, &arpt_keep, 0, INT_MAX },
> > { IPCTL_ARPDOWN, &arpt_down, 0, INT_MAX },
> > };
> > @@ -1643,8 +1644,6 @@ ip_sysctl(int *name, u_int namelen, void *oldp,
> > size_t *oldlenp, void *newp,
> > case IPCTL_ARPQUEUE:
> > return (sysctl_niq(name + 1, namelen - 1,
> > oldp, oldlenp, newp, newlen, &arpinq));
> > - case IPCTL_ARPQUEUED:
> > - return (sysctl_rdint(oldp, oldlenp, newp, la_hold_total));
> > case IPCTL_STATS:
> > return (ip_sysctl_ipstat(oldp, oldlenp, newp));
> > #ifdef MROUTING
> > --
> > 2.31.1
>
