Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <dae3fc04-5e59-dff5-db77-ea7d0a3d154e@kernel.org>
Date:   Thu, 20 Jul 2023 09:18:17 -0600
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.13.0
Subject: Re: [PATCH net-next v2 3/4] nexthop: Do not return invalid nexthop
 object during multipath selection
Content-Language: en-US
To:     Benjamin Poirier <bpoirier@nvidia.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, Shuah Khan <shuah@kernel.org>
Cc:     "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-kselftest@vger.kernel.org" <linux-kselftest@vger.kernel.org>,
        Ido Schimmel <idosch@nvidia.com>
References: <20230719-nh_select-v2-0-04383e89f868@nvidia.com>
 <20230719-nh_select-v2-3-04383e89f868@nvidia.com>
From:   David Ahern <dsahern@kernel.org>
In-Reply-To: <20230719-nh_select-v2-3-04383e89f868@nvidia.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 7/19/23 7:57 AM, Benjamin Poirier wrote:
> With legacy nexthops, when net.ipv4.fib_multipath_use_neigh is set,
> fib_select_multipath() will never set res->nhc to a nexthop that is not
> good (as per fib_good_nh()). OTOH, with nexthop objects,
> nexthop_select_path_hthr() may return a nexthop that failed the
> nexthop_is_good_nh() test even if there was one that passed. Refactor
> nexthop_select_path_hthr() to follow a selection logic more similar to
> fib_select_multipath().
> 
> The issue can be demonstrated with the following sequence of commands. The
> first block shows that things work as expected with legacy nexthops. The
> last sequence of `ip rou get` in the second block shows the problem case -
> some routes still use the .2 nexthop.
> 
> sysctl net.ipv4.fib_multipath_use_neigh=1
> ip link add dummy1 up type dummy
> ip rou add 198.51.100.0/24 nexthop via 192.0.2.1 dev dummy1 onlink nexthop via 192.0.2.2 dev dummy1 onlink
> for i in {10..19}; do ip -o rou get 198.51.100.$i; done
> ip neigh add 192.0.2.1 dev dummy1 nud failed
> echo ".1 failed:"  # results should not use .1
> for i in {10..19}; do ip -o rou get 198.51.100.$i; done
> ip neigh del 192.0.2.1 dev dummy1
> ip neigh add 192.0.2.2 dev dummy1 nud failed
> echo ".2 failed:"  # results should not use .2
> for i in {10..19}; do ip -o rou get 198.51.100.$i; done
> ip link del dummy1
> 
> ip link add dummy1 up type dummy
> ip nexthop add id 1 via 192.0.2.1 dev dummy1 onlink
> ip nexthop add id 2 via 192.0.2.2 dev dummy1 onlink
> ip nexthop add id 1001 group 1/2
> ip rou add 198.51.100.0/24 nhid 1001
> for i in {10..19}; do ip -o rou get 198.51.100.$i; done
> ip neigh add 192.0.2.1 dev dummy1 nud failed
> echo ".1 failed:"  # results should not use .1
> for i in {10..19}; do ip -o rou get 198.51.100.$i; done
> ip neigh del 192.0.2.1 dev dummy1
> ip neigh add 192.0.2.2 dev dummy1 nud failed
> echo ".2 failed:"  # results should not use .2
> for i in {10..19}; do ip -o rou get 198.51.100.$i; done
> ip link del dummy1
> 
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
> ---
>  net/ipv4/nexthop.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>