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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5616C5AE.8010707@cumulusnetworks.com>
Date:	Thu, 8 Oct 2015 13:36:14 -0600
From:	David Ahern <dsa@...ulusnetworks.com>
To:	Hannes Frederic Sowa <hannes@...hat.com>, netdev@...r.kernel.org
Cc:	nicolas.dichtel@...nd.com
Subject: Re: [PATCH net-next v4] net: ipv6: Make address flushing on ifdown
 optional

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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html