> On 26 Apr 2021, at 01:43, Theo de Raadt <dera...@openbsd.org> wrote:
>
> 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.
Agreed. Also according sysctl_int_bounded() code this behaviour looks
like non obvious side effect.
>
> 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
>>
