Re: [PATCH net-next v7 04/11] ipv4: Dump route exceptions if requested

From: David Ahern <dsahern@gmail.com>
To: Stefano Brivio <sbrivio@redhat.com>, David Miller <davem@davemloft.net>
Cc: Jianlin Shi <jishi@redhat.com>, Wei Wang <weiwan@google.com>,
	Martin KaFai Lau <kafai@fb.com>,
	Eric Dumazet <edumazet@google.com>,
	Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>,
	netdev@vger.kernel.org
Subject: Re: [PATCH net-next v7 04/11] ipv4: Dump route exceptions if requested
Date: Fri, 21 Jun 2019 16:19:21 -0600	[thread overview]
Message-ID: <542d653a-37c8-66b3-df34-71a0e0273f8b@gmail.com> (raw)
In-Reply-To: <8d3b68cd37fb5fddc470904cdd6793fcf480c6c1.1561131177.git.sbrivio@redhat.com>

On 6/21/19 9:45 AM, Stefano Brivio wrote:
> Since commit 4895c771c7f0 ("ipv4: Add FIB nexthop exceptions."), cached
> exception routes are stored as a separate entity, so they are not dumped
> on a FIB dump, even if the RTM_F_CLONED flag is passed.
> 
> This implies that the command 'ip route list cache' doesn't return any
> result anymore.
> 
> If the RTM_F_CLONED is passed, and strict checking requested, retrieve
> nexthop exception routes and dump them. If no strict checking is
> requested, filtering can't be performed consistently: dump everything in
> that case.
> 
> With this, we need to add an argument to the netlink callback in order to
> track how many entries were already dumped for the last leaf included in
> a partial netlink dump.
> 
> A single additional argument is sufficient, even if we traverse logically
> nested structures (nexthop objects, hash table buckets, bucket chains): it
> doesn't matter if we stop in the middle of any of those, because they are
> always traversed the same way. As an example, s_i values in [], s_fa
> values in ():
> 
>   node (fa) #1 [1]
>     nexthop #1
>     bucket #1 -> #0 in chain (1)
>     bucket #2 -> #0 in chain (2) -> #1 in chain (3) -> #2 in chain (4)
>     bucket #3 -> #0 in chain (5) -> #1 in chain (6)
> 
>     nexthop #2
>     bucket #1 -> #0 in chain (7) -> #1 in chain (8)
>     bucket #2 -> #0 in chain (9)
>   --
>   node (fa) #2 [2]
>     nexthop #1
>     bucket #1 -> #0 in chain (1) -> #1 in chain (2)
>     bucket #2 -> #0 in chain (3)
> 
> it doesn't matter if we stop at (3), (4), (7) for "node #1", or at (2)
> for "node #2": walking flattens all that.
> 
> It would even be possible to drop the distinction between the in-tree
> (s_i) and in-node (s_fa) counter, but a further improvement might
> advise against this. This is only as accurate as the existing tracking
> mechanism for leaves: if a partial dump is restarted after exceptions
> are removed or expired, we might skip some non-dumped entries.

...

> 
> Fixes: 4895c771c7f0 ("ipv4: Add FIB nexthop exceptions.")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>  include/net/route.h |  4 +++
>  net/ipv4/fib_trie.c | 44 +++++++++++++++++++--------
>  net/ipv4/route.c    | 73 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 108 insertions(+), 13 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@gmail.com>