From 3d30f6defb5d0c6bc4d016b8eca114c4198acbbe Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Wed, 27 Jan 2021 16:20:22 -0500 Subject: [PATCH] zebra: disallow resolution to duplicate nexthops Disallow the resolution to nexthops that are marked duplicate. When we are resolving to an ecmp group, it's possible this group has duplicates. I found this when I hit a bug where we can have groups resolving to each other and cause the resolved->next->next pointer to increase exponentially. Sufficiently large ecmp and zebra will grind to a hault. Like so: ``` D> 4.4.4.14/32 [150/0] via 1.1.1.1 (recursive), weight 1, 00:00:02 * via 1.1.1.1, dummy1 onlink, weight 1, 00:00:02 via 4.4.4.1 (recursive), weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 4.4.4.2 (recursive), weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 4.4.4.3 (recursive), weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 4.4.4.4 (recursive), weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 4.4.4.5 (recursive), weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 4.4.4.6 (recursive), weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 4.4.4.7 (recursive), weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 4.4.4.8 (recursive), weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 4.4.4.9 (recursive), weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 4.4.4.10 (recursive), weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 4.4.4.11 (recursive), weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 4.4.4.12 (recursive), weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 4.4.4.13 (recursive), weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 4.4.4.15 (recursive), weight 1, 00:00:02 via 1.1.1.1, dummy1 onlink, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1 onlink, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 4.4.4.16 (recursive), weight 1, 00:00:02 via 1.1.1.1, dummy1 onlink, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 via 1.1.1.1, dummy1, weight 1, 00:00:02 D> 4.4.4.15/32 [150/0] via 1.1.1.1 (recursive), weight 1, 00:00:09 * via 1.1.1.1, dummy1 onlink, weight 1, 00:00:09 via 4.4.4.1 (recursive), weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 4.4.4.2 (recursive), weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 4.4.4.3 (recursive), weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 4.4.4.4 (recursive), weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 4.4.4.5 (recursive), weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 4.4.4.6 (recursive), weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 4.4.4.7 (recursive), weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 4.4.4.8 (recursive), weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 4.4.4.9 (recursive), weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 4.4.4.10 (recursive), weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 4.4.4.11 (recursive), weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 4.4.4.12 (recursive), weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 4.4.4.13 (recursive), weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 4.4.4.14 (recursive), weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 4.4.4.16 (recursive), weight 1, 00:00:09 via 1.1.1.1, dummy1 onlink, weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 via 1.1.1.1, dummy1, weight 1, 00:00:09 D> 4.4.4.16/32 [150/0] via 1.1.1.1 (recursive), weight 1, 00:00:19 * via 1.1.1.1, dummy1 onlink, weight 1, 00:00:19 via 4.4.4.1 (recursive), weight 1, 00:00:19 via 1.1.1.1, dummy1, weight 1, 00:00:19 via 4.4.4.2 (recursive), weight 1, 00:00:19 ............... ................ and on... ``` You can repro the above via: ``` kernel routes: 1.1.1.1 dev dummy1 scope link 4.4.4.0/24 via 1.1.1.1 dev dummy1 ============================== config: nexthop-group doof nexthop 1.1.1.1 nexthop 4.4.4.1 nexthop 4.4.4.10 nexthop 4.4.4.11 nexthop 4.4.4.12 nexthop 4.4.4.13 nexthop 4.4.4.14 nexthop 4.4.4.15 nexthop 4.4.4.16 nexthop 4.4.4.2 nexthop 4.4.4.3 nexthop 4.4.4.4 nexthop 4.4.4.5 nexthop 4.4.4.6 nexthop 4.4.4.7 nexthop 4.4.4.8 nexthop 4.4.4.9 ! =========================== Then use sharpd to install 4.4.4.16 -> 4.4.4.1 pointing to that nexthop group in decending order. ``` With these changes it prevents the growing ecmp above by disallowing duplicates to be in the resolution decision. These nexthops are not installed anyways so why should we be resolving to them? Signed-off-by: Stephen Worley --- zebra/zebra_nhg.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index 604480b68b..2864b96c83 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -1760,6 +1760,10 @@ static bool nexthop_valid_resolve(const struct nexthop *nexthop, if (!CHECK_FLAG(resolved->flags, NEXTHOP_FLAG_ACTIVE)) return false; + /* Must not be duplicate */ + if (CHECK_FLAG(resolved->flags, NEXTHOP_FLAG_DUPLICATE)) + return false; + switch (nexthop->type) { case NEXTHOP_TYPE_IPV4_IFINDEX: case NEXTHOP_TYPE_IPV6_IFINDEX: -- 2.39.5