Re: [PATCH net 2/2] ipv6: Do not consider link down nexthops in path selection

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Ido Schimmel wrote:
> Nexthops whose link is down are not supposed to be considered during
> path selection when the "ignore_routes_with_linkdown" sysctl is set.
> This is done by assigning them a negative region boundary.
> 
> However, when comparing the computed hash (unsigned) with the region
> boundary (signed), the negative region boundary is treated as unsigned,
> resulting in incorrect nexthop selection.
> 
> Fix by treating the computed hash as signed. Note that the computed hash
> is always in range of [0, 2^31 - 1].
> 
> Fixes: 3d709f69a3e7 ("ipv6: Use hash-threshold instead of modulo-N")
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> ---
>  net/ipv6/route.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 864f0002034b..ab12b816ab94 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -442,6 +442,7 @@ void fib6_select_path(const struct net *net, struct fib6_result *res,
>  {
>  	struct fib6_info *first, *match = res->f6i;
>  	struct fib6_info *sibling;
> +	int hash;
>  
>  	if (!match->nh && (!match->fib6_nsiblings || have_oif_match))
>  		goto out;
> @@ -468,7 +469,8 @@ void fib6_select_path(const struct net *net, struct fib6_result *res,
>  	if (!first)
>  		goto out;
>  
> -	if (fl6->mp_hash <= atomic_read(&first->fib6_nh->fib_nh_upper_bound) &&
> +	hash = fl6->mp_hash;
> +	if (hash <= atomic_read(&first->fib6_nh->fib_nh_upper_bound) &&

The combined upper bounds add up to the total weights of the paths.

Should hash be scaled (using reciprocal_scale) to that bound to have
a uniform random distribution across all weights?

Else a hash in the range [0, 2^31 - 1] is unlikely to fall within the
total weights range.

>  	    rt6_score_route(first->fib6_nh, first->fib6_flags, oif,
>  			    strict) >= 0) {
>  		match = first;
> @@ -481,7 +483,7 @@ void fib6_select_path(const struct net *net, struct fib6_result *res,
>  		int nh_upper_bound;
>  
>  		nh_upper_bound = atomic_read(&nh->fib_nh_upper_bound);
> -		if (fl6->mp_hash > nh_upper_bound)
> +		if (hash > nh_upper_bound)
>  			continue;
>  		if (rt6_score_route(nh, sibling->fib6_flags, oif, strict) < 0)
>  			break;
> -- 
> 2.49.0
>