netdev - Re: [PATCH v2 net-next] neighbor: Improve garbage collection

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]

Date:   Sun, 9 Dec 2018 21:12:59 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     David Miller <davem@...emloft.net>, dsahern@...nel.org
Cc:     netdev@...r.kernel.org, roopa@...ulusnetworks.com,
        dsahern@...il.com
Subject: Re: [PATCH v2 net-next] neighbor: Improve garbage collection



On 12/07/2018 04:03 PM, David Miller wrote:
> From: David Ahern <dsahern@...nel.org>
> Date: Fri,  7 Dec 2018 12:24:57 -0800
> 
>> From: David Ahern <dsahern@...il.com>
>>
>> The existing garbage collection algorithm has a number of problems:
>  ...
>> This patch addresses these problems as follows:
>>
>> 1. Use of a separate list_head to track entries that can be garbage
>>    collected along with a separate counter. PERMANENT entries are not
>>    added to this list.
>>
>>    The gc_thresh parameters are only compared to the new counter, not the
>>    total entries in the table. The forced_gc function is updated to only
>>    walk this new gc_list looking for entries to evict.
>>
>> 2. Entries are added to the list head at the tail and removed from the
>>    front.
>>
>> 3. Entries are only evicted if they were last updated more than 5 seconds
>>    ago, adhering to the original intent of gc_thresh2.
>>
>> 4. Forced gc is stopped once the number of gc_entries drops below
>>    gc_thresh2.
>>
>> 5. Since gc checks do not apply to PERMANENT entries, gc levels are skipped
>>    when allocating a new neighbor for a PERMANENT entry. By extension this
>>    means there are no explicit limits on the number of PERMANENT entries
>>    that can be created, but this is no different than FIB entries or FDB
>>    entries.
>>
>> Signed-off-by: David Ahern <dsahern@...il.com>
>> ---
>> v2
>> - remove on_gc_list boolean in favor of !list_empty
>> - fix neigh_alloc to add new entry to tail of list_head
> 
> Again, looks great, applied.
> 

What protects gc_list linkage ?

We can not use list_del_init(&n->gc_list);                    or 
               list_add_tail(&n->gc_list, &n->tbl->gc_list);

if tbl->lock is not held.

It seems to me this patch needs more care.