From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ido Schimmel Subject: Re: [PATCH net-next 1/4] ipv6: Calculate hash thresholds for IPv6 nexthops Date: Wed, 2 May 2018 21:53:10 +0300 Message-ID: <20180502185310.GA31998@splinter> References: <20180109144028.30133-1-idosch@mellanox.com> <20180109144028.30133-2-idosch@mellanox.com> <5550c628-5014-427b-60c9-71cf80462723@gmail.com> <20180502172106.GA12986@splinter> <20180502175244.GA14587@splinter> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: David Ahern , Ido Schimmel , netdev@vger.kernel.org, davem@davemloft.net, roopa@cumulusnetworks.com, nikolay@cumulusnetworks.com, pch@ordbogen.com, jkbs@redhat.com, yoshfuji@linux-ipv6.org, mlxsw@mellanox.com To: Eric Dumazet , Thomas.Winter@alliedtelesis.co.nz Return-path: Received: from out2-smtp.messagingengine.com ([66.111.4.26]:48769 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750939AbeEBSxP (ORCPT ); Wed, 2 May 2018 14:53:15 -0400 Content-Disposition: inline In-Reply-To: <20180502175244.GA14587@splinter> Sender: netdev-owner@vger.kernel.org List-ID: On Wed, May 02, 2018 at 08:52:44PM +0300, Ido Schimmel wrote: > On Wed, May 02, 2018 at 08:21:06PM +0300, Ido Schimmel wrote: > > On Wed, May 02, 2018 at 09:43:50AM -0700, Eric Dumazet wrote: > > > > > > > > > On 01/09/2018 07:43 PM, David Ahern wrote: > > > > On 1/9/18 7:40 AM, Ido Schimmel wrote: > > > >> Before we convert IPv6 to use hash-threshold instead of modulo-N, we > > > >> first need each nexthop to store its region boundary in the hash > > > >> function's output space. > > > >> > > > >> The boundary is calculated by dividing the output space equally between > > > >> the different active nexthops. That is, nexthops that are not dead or > > > >> linkdown. > > > >> > > > >> The boundaries are rebalanced whenever a nexthop is added or removed to > > > >> a multipath route and whenever a nexthop becomes active or inactive. > > > >> > > > >> Signed-off-by: Ido Schimmel > > > >> --- > > > >> include/net/ip6_fib.h | 1 + > > > >> include/net/ip6_route.h | 7 ++++ > > > >> net/ipv6/ip6_fib.c | 8 ++--- > > > >> net/ipv6/route.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++ > > > >> 4 files changed, 106 insertions(+), 6 deletions(-) > > > >> > > > > > > > > LGTM. > > > > Acked-by: David Ahern > > > > > > > > > > For some reason I have a divide by zero error booting my hosts with latest net tree. > > > > > > What guarantee do we have that total is not zero when rt6_upper_bound_set() is called ? > > > > Thanks for the report, Eric. I believe I didn't cover all the cases and > > 'rt6i_nh_weight' might be 0 is some cases. I'll try to reproduce and > > work on a fix. > > Hmmm, I think it's due to commit edd7ceb78296 ("ipv6: Allow non-gateway > ECMP for IPv6") which allows routes without a gateway (such as those > configured using slaac) to have siblings. > > Can you please check if reverting the patch / applying the below fixes > the issue? So this fixes the issue for me. To reproduce: # ip -6 address add 2001:db8::1/64 dev dummy0 # ip -6 address add 2001:db8::1/64 dev dummy1 This reproduces the issue because due to above commit both local routes are considered siblings... :/ local 2001:db8::1 proto kernel metric 0 nexthop dev dummy0 weight 1 nexthop dev dummy1 weight 1 pref medium I think it's best to revert the patch and have Thomas submit a fixed version to net-next. I was actually surprised to see it applied to net. > > diff --git a/net/ipv6/route.c b/net/ipv6/route.c > index f4d61736c41a..129dd4f4b264 100644 > --- a/net/ipv6/route.c > +++ b/net/ipv6/route.c > @@ -3606,6 +3606,7 @@ struct rt6_info *addrconf_dst_alloc(struct inet6_dev *idev, > rt->dst.input = ip6_input; > rt->dst.output = ip6_output; > rt->rt6i_idev = idev; > + rt->rt6i_nh_weight = 1; > > rt->rt6i_protocol = RTPROT_KERNEL; > rt->rt6i_flags = RTF_UP | RTF_NONEXTHOP;