Date: Thu, 8 Feb 2024 08:07:18 -0800
From: Kui-Feng Lee <sinquersw@...il.com>
To: Paolo Abeni <pabeni@...hat.com>, thinker.li@...il.com,
 netdev@...r.kernel.org, ast@...nel.org, martin.lau@...ux.dev,
 kernel-team@...a.com, davem@...emloft.net, dsahern@...nel.org,
 edumazet@...gle.com, kuba@...nel.org, liuhangbin@...il.com
Cc: kuifeng@...a.com
Subject: Re: [PATCH net-next v5 0/5] Remove expired routes with a separated
 list of routes.



On 2/8/24 03:55, Paolo Abeni wrote:
> On Wed, 2024-02-07 at 11:29 -0800, thinker.li@...il.com wrote:
>> From: Kui-Feng Lee <thinker.li@...il.com>
>>
>> This patchset is resent due to previous reverting. [1]
>>
>> FIB6 GC walks trees of fib6_tables to remove expired routes. Walking a tree
>> can be expensive if the number of routes in a table is big, even if most of
>> them are permanent. Checking routes in a separated list of routes having
>> expiration will avoid this potential issue.
>>
>> Background
>> ==========
>>
>> The size of a Linux IPv6 routing table can become a big problem if not
>> managed appropriately.  Now, Linux has a garbage collector to remove
>> expired routes periodically.  However, this may lead to a situation in
>> which the routing path is blocked for a long period due to an
>> excessive number of routes.
>>
>> For example, years ago, there is a commit c7bb4b89033b ("ipv6: tcp:
>> drop silly ICMPv6 packet too big messages").  The root cause is that
>> malicious ICMPv6 packets were sent back for every small packet sent to
>> them. These packets add routes with an expiration time that prompts
>> the GC to periodically check all routes in the tables, including
>> permanent ones.
>>
>> Why Route Expires
>> =================
>>
>> Users can add IPv6 routes with an expiration time manually. However,
>> the Neighbor Discovery protocol may also generate routes that can
>> expire.  For example, Router Advertisement (RA) messages may create a
>> default route with an expiration time. [RFC 4861] For IPv4, it is not
>> possible to set an expiration time for a route, and there is no RA, so
>> there is no need to worry about such issues.
>>
>> Create Routes with Expires
>> ==========================
>>
>> You can create routes with expires with the  command.
>>
>> For example,
>>
>>      ip -6 route add 2001:b000:591::3 via fe80::5054:ff:fe12:3457 \
>>          dev enp0s3 expires 30
>>
>> The route that has been generated will be deleted automatically in 30
>> seconds.
>>
>> GC of FIB6
>> ==========
>>
>> The function called fib6_run_gc() is responsible for performing
>> garbage collection (GC) for the Linux IPv6 stack. It checks for the
>> expiration of every route by traversing the trees of routing
>> tables. The time taken to traverse a routing table increases with its
>> size. Holding the routing table lock during traversal is particularly
>> undesirable. Therefore, it is preferable to keep the lock for the
>> shortest possible duration.
>>
>> Solution
>> ========
>>
>> The cause of the issue is keeping the routing table locked during the
>> traversal of large trees. To solve this problem, we can create a separate
>> list of routes that have expiration. This will prevent GC from checking
>> permanent routes.
>>
>> Result
>> ======
>>
>> We conducted a test to measure the execution times of fib6_gc_timer_cb()
>> and observed that it enhances the GC of FIB6. During the test, we added
>> permanent routes with the following numbers: 1000, 3000, 6000, and
>> 9000. Additionally, we added a route with an expiration time.
>>
>> Here are the average execution times for the kernel without the patch.
>>   - 120020 ns with 1000 permanent routes
>>   - 308920 ns with 3000 ...
>>   - 581470 ns with 6000 ...
>>   - 855310 ns with 9000 ...
>>
>> The kernel with the patch consistently takes around 14000 ns to execute,
>> regardless of the number of permanent routes that are installed.
>>
>> Major changes from v4:
>>
>>   - Fix the comment of fib6_add_gc_list().
>>
>> Major changes from v3:
>>
>>   - Move the checks of f6i->fib6_node to fib6_add_gc_list().
>>
>>   - Make spin_lock_bh() and spin_unlock_bh() stands out.
>>
>>   - Explain the reason of the changes in the commit message of the
>>     patch 4.
>>
>> Major changes from v2:
>>
>>   - Refactory the boilerplate checks in the test case.
>>
>>     - check_rt_num() and check_rt_num_clean()
>>
>> Major changes from v1:
>>
>>   - Reduce the numbers of routes (5) in the test cases to work with
>>     slow environments. Due to the failure on patchwork.
>>
>>   - Remove systemd related commands in the test case.
>>
>> Major changes from the previous patchset [2]:
>>
>>   - Split helpers.
>>
>>     - fib6_set_expires() -> fib6_set_expires() and fib6_add_gc_list().
>>
>>     - fib6_clean_expires() -> fib6_clean_expires() and
>>       fib6_remove_gc_list().
>>
>>   - Fix rt6_add_dflt_router() to avoid racing of setting expires.
>>
>>   - Remove unnecessary calling to fib6_clean_expires() in
>>     ip6_route_info_create().
>>
>>   - Add test cases of toggling routes between permanent and temporary
>>     and handling routes from RA messages.
>>
>>     - Clean up routes by deleting the existing device and adding a new
>>       one.
>>
>>   - Fix a potential issue in modify_prefix_route().
> 
> Note that we have a selftest failure in the batch including this series
> for the fib_tests:
> 
> https://netdev-3.bots.linux.dev/vmksft-net/results/456022/6-fib-tests-sh/stdout
> 
> I haven't digged much, but I fear its related. Please have a look.
> 
> For more info on how to reproduce the selftest environment:
> 
> https://github.com/linux-netdev/nipa/wiki/How-to-run-netdev-selftests-CI-style
> 
> Thanks,
> 
> Paolo
> 
I will check it.

Powered by blists - more mailing lists