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

Reply via email to