Re: [PATCH iproute2] ip route: ignore ENOENT during save if RT_TABLE_MAIN is being dumped

[David Ahern <dsahern@gmail.com>]

On 6/22/21 9:03 AM, Alexander Mikhalitsyn wrote:
> We started to use in-kernel filtering feature which allows to get only needed
> tables (see iproute_dump_filter()). From the kernel side it's implemented in
> net/ipv4/fib_frontend.c (inet_dump_fib), net/ipv6/ip6_fib.c (inet6_dump_fib).
> The problem here is that behaviour of "ip route save" was changed after
> c7e6371bc ("ip route: Add protocol, table id and device to dump request").
> If filters are used, then kernel returns ENOENT error if requested table is absent,
> but in newly created net namespace even RT_TABLE_MAIN table doesn't exist.
> It is really allocated, for instance, after issuing "ip l set lo up".
> 
> Reproducer is fairly simple:
> $ unshare -n ip route save > dump
> Error: ipv4: FIB table does not exist.
> Dump terminated

The above command on 5.4 kernel with corresponding iproute2 does not
show that error. Is your kernel compiled with CONFIG_IP_MULTIPLE_TABLES
enabled?

> 
> Expected result here is to get empty dump file (as it was before this change).
> 
> This affects on CRIU [1] because we use ip route save in dump process, to workaround
> problem in tests we just put loopback interface up in each net namespace.
> Other users also met this problem [2].
> 
> Links:
> [1] https://github.com/checkpoint-restore/criu/issues/747
> [2] https://www.spinics.net/lists/netdev/msg559739.html
> 
> Fixes: c7e6371bc ("ip route: Add protocol, table id and device to dump request")
> 
> Cc: David Ahern <dsahern@kernel.org>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Cc: Andrei Vagin <avagin@gmail.com>
> Cc: Alexander Mikhalitsyn <alexander@mihalicyn.com>
> Signed-off-by: Alexander Mikhalitsyn <alexander.mikhalitsyn@virtuozzo.com>
> ---
>  ip/iproute.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/ip/iproute.c b/ip/iproute.c
> index 5853f026..b70acc00 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -1734,6 +1734,7 @@ static int iproute_list_flush_or_save(int argc, char **argv, int action)
>  	char *od = NULL;
>  	unsigned int mark = 0;
>  	rtnl_filter_t filter_fn;
> +	int ret;
>  
>  	if (action == IPROUTE_SAVE) {
>  		if (save_route_prep())
> @@ -1939,7 +1940,11 @@ static int iproute_list_flush_or_save(int argc, char **argv, int action)
>  
>  	new_json_obj(json);
>  
> -	if (rtnl_dump_filter(&rth, filter_fn, stdout) < 0) {
> +	ret = rtnl_dump_filter(&rth, filter_fn, stdout);
> +
> +	/* Let's ignore ENOENT error if we want to dump RT_TABLE_MAIN table */
> +	if (ret < 0 &&

ret temp variable is not needed; just add the extra checks.

> +	    !(errno == ENOENT && filter.tb == RT_TABLE_MAIN)) {
>  		fprintf(stderr, "Dump terminated\n");
>  		return -2;
>  	}
> 

This looks fine to me, but I want clarification on the kernel config. As
I recall with multiple tables and fib rules tables are created when net
namespace is created.