Re: [PATCH net-next v4] net: ipv6: Make address flushing on ifdown optional

[David Ahern <dsa@cumulusnetworks.com>]

Hi Hannes:

On 10/8/15 1:25 PM, Hannes Frederic Sowa wrote:
>> diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
>> index 1c8b6820b694..f190a14148ab 100644
>> --- a/include/net/if_inet6.h
>> +++ b/include/net/if_inet6.h
>> @@ -72,6 +72,7 @@ struct inet6_ifaddr {
>>   	int			regen_count;
>>
>>   	bool			tokenized;
>> +	bool			managed;
>
> IMHO the naming of the bool is a bit too vague. ;) Would you mind
> renaming it to something like puuh... user_managed, non_autoconf,
> manual_conf etc.?  'managed' seems so often used in the context of
> temporary addresses, I first thought about that.
>
> enum { USER_SPACE, KERNEL_AUTOCONF } managed_by;

I have no preference on naming; unless other preferences are stated I'll 
do v5 with it renamed to 'user_managed'.



>> @@ -2689,6 +2692,9 @@ static int inet6_addr_add(struct net *net, int ifindex,
>>   			    valid_lft, prefered_lft);
>>
>>   	if (!IS_ERR(ifp)) {
>> +		if (!expires)
>> +			ifp->managed = true;
>> +
>
> This assumes that user space managed addresses don't time out. This is
> in fact not true. I am not sure if it matters a lot, as most addresses
> added from user space with a timeout most probably will be added because
> of autoconf, but they are not managed by kernel autoconf. Not sure if we
> want to make this more explicit, certainly it would avoid surprises.

Not exactly. I'm taking the easy way out and saying only addresses with 
no expiration time fall into the 'user managed' category and retained on 
an ifdown. Trying to accommodate lifetimes is a PITA. I mentioned that 
in the documentation:
   "static global addresses with no expiration time are not flushed"

Thanks for the review,
David