Thread (24 messages) flat view 24 messages, 6 authors, 2018-05-04

Re: [PATCH net-next 1/4] ipv6: Calculate hash thresholds for IPv6 nexthops

From: Ido Schimmel <hidden>
Date: 2018-05-02 18:53:15

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:
quoted
On Wed, May 02, 2018 at 09:43:50AM -0700, Eric Dumazet wrote:
quoted

On 01/09/2018 07:43 PM, David Ahern wrote:
quoted
On 1/9/18 7:40 AM, Ido Schimmel wrote:
quoted
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 <redacted>
---
 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 <redacted>
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.
quoted hunk ↗ jump to hunk
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;
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help