Re: [PATCH 1/4] net: arp: introduce arp_evict_nocarrier sysctl parameter

[David Ahern <dsahern@gmail.com>]

On 10/14/21 10:59 AM, Jakub Kicinski wrote:
> On Thu, 14 Oct 2021 09:29:05 -0700 James Prestwood wrote:
>>>> diff --git a/include/linux/inetdevice.h
>>>> b/include/linux/inetdevice.h
>>>> index 53aa0343bf69..63180170fdbd 100644
>>>> --- a/include/linux/inetdevice.h
>>>> +++ b/include/linux/inetdevice.h
>>>> @@ -133,6 +133,7 @@ static inline void ipv4_devconf_setall(struct
>>>> in_device *in_dev)
>>>>  #define IN_DEV_ARP_ANNOUNCE(in_dev)    IN_DEV_MAXCONF((in_dev),
>>>> ARP_ANNOUNCE)
>>>>  #define IN_DEV_ARP_IGNORE(in_dev)      IN_DEV_MAXCONF((in_dev),
>>>> ARP_IGNORE)
>>>>  #define IN_DEV_ARP_NOTIFY(in_dev)      IN_DEV_MAXCONF((in_dev),
>>>> ARP_NOTIFY)
>>>> +#define IN_DEV_ARP_EVICT_NOCARRIER(in_dev)
>>>> IN_DEV_CONF_GET((in_dev), ARP_EVICT_NOCARRIER)  
>>>
>>> IN_DEV_ANDCONF() makes most sense, I'd think.  
>>
>> So given we want '1' as the default as well as the ability to toggle
>> this option per-netdev I thought this was more appropriate. One caviat
>> is this would not work for setting ALL netdev's, but I don't think this
>> is a valid use case; or at least I can't imagine why you'd want to ever
>> do this.
>>
>> This is a whole new area to me though, so if I'm understanding these
>> macros wrong please educate me :)
> 
> Yeah, TBH I'm not sure what the best practice is here. I know we
> weren't very consistent in the past. Having a knob for "all" which 
> is 100% ignored seems like the worst option. Let me CC Dave Ahern,
> please make sure you CC him on v2 as well.
> 

agreed. It's a matter of consistency in the use of the 'all' variant. I
would say support it even if it you do not believe it will be changed.