Message-ID: <f8f40c760f274a7780c5ab491e7eb75e9ca0098b.camel@redhat.com>
Date: Thu, 08 Feb 2024 12:55:54 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: 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: sinquersw@...il.com, kuifeng@...a.com
Subject: Re: [PATCH net-next v5 0/5] Remove expired routes with a separated
 list of routes.

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

Powered by blists - more mailing lists