From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [PATCH net] ipv6: Do not consider linkdown nexthops during multipath Date: Tue, 21 Nov 2017 12:00:03 -0700 Message-ID: <77e45cd4-48a9-71f8-19dc-93b44ebb14b9@gmail.com> References: <20171121075012.11077-1-idosch@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: davem@davemloft.net, andy@greyhouse.net, mlxsw@mellanox.com To: Ido Schimmel , netdev@vger.kernel.org Return-path: Received: from mail-pg0-f65.google.com ([74.125.83.65]:36800 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751184AbdKUTAF (ORCPT ); Tue, 21 Nov 2017 14:00:05 -0500 Received: by mail-pg0-f65.google.com with SMTP id k190so6215612pga.3 for ; Tue, 21 Nov 2017 11:00:05 -0800 (PST) In-Reply-To: <20171121075012.11077-1-idosch@mellanox.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 11/21/17 12:50 AM, Ido Schimmel wrote: > When the 'ignore_routes_with_linkdown' sysctl is set, we should not > consider linkdown nexthops during route lookup. > > While the code correctly verifies that the initially selected route > ('match') has a carrier, it does not perform the same check in the > subsequent multipath selection, resulting in a potential packet loss. > > In case the chosen route does not have a carrier and the sysctl is set, > choose the initially selected route. > > Fixes: 35103d11173b ("net: ipv6 sysctl option to ignore routes when nexthop link is down") > Signed-off-by: Ido Schimmel > --- > net/ipv6/route.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/net/ipv6/route.c b/net/ipv6/route.c > index 05eb7bc36156..0363db914c7a 100644 > --- a/net/ipv6/route.c > +++ b/net/ipv6/route.c > @@ -472,6 +472,11 @@ static struct rt6_info *rt6_multipath_select(struct rt6_info *match, > &match->rt6i_siblings, rt6i_siblings) { > route_choosen--; > if (route_choosen == 0) { > + struct inet6_dev *idev = sibling->rt6i_idev; > + > + if (!netif_carrier_ok(sibling->dst.dev) && > + idev->cnf.ignore_routes_with_linkdown) > + break; > if (rt6_score_route(sibling, oif, strict) < 0) > break; > match = sibling; > Looks right to me. Acked-by: David Ahern