From: vivek Date: Sun, 15 Nov 2015 18:21:12 +0000 (-0800) Subject: BGP: Fix nexthop registration churn X-Git-Tag: frr-2.0-rc1~1199^2~3 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=25c38b240e56e41c0742c5260e72c1dd30dffbc7;p=matthieu%2Ffrr.git BGP: Fix nexthop registration churn When a BGP nexthop is registered for resolution, if it is learnt from an EBGP peer and other conditions warrant (non-multihop peer and connected check is not disabled), the registration includes a flag that indicates that the nexthop must be resolved only if it is directly connected. In peculiar situations - e.g., third-party nexthop or policy configuration - the same nexthop could be learnt from an IBGP peer, and in general, nexthops learnt from IBGP peers can be resolved over any route. This scenario was causing a churn in the nexthop registration with the 'must-be-connected' flag being repeatedly toggled as routes are received from both peers. The registrations would in turn trigger significant processing. The fix is to treat 'must-be-connected' as an overriding condition. The repeated registration and related processing was also causing heavy memory usage by BGP - for memory buffers used to hold registration information. This fix will ensure that is no longer the case. Signed-off-by: Vivek Venkatraman Reviewed-by: Donald Sharp Ticket: CM-8005, CM-8013 Reviewed By: CCR-3772 Testing Done: Manual, bgpsmoke (on 2.5-br) --- diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index 197dd87585..25969da3d2 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -191,13 +191,23 @@ bgp_find_or_add_nexthop (struct bgp *bgp, afi_t afi, struct bgp_info *ri, UNSET_FLAG(bnc->flags, BGP_NEXTHOP_VALID); } } + /* When nexthop is already known, but now requires 'connected' resolution, + * re-register it. The reverse scenario where the nexthop currently requires + * 'connected' resolution does not need a re-register (i.e., we treat + * 'connected-required' as an override) except in the scenario where this + * is actually a case of tracking a peer for connectivity (e.g., after + * disable connected-check). + * NOTE: We don't track the number of paths separately for 'connected- + * required' vs 'connected-not-required' as this change is not a common + * scenario. + */ else if (connected && ! CHECK_FLAG(bnc->flags, BGP_NEXTHOP_CONNECTED)) { SET_FLAG(bnc->flags, BGP_NEXTHOP_CONNECTED); UNSET_FLAG(bnc->flags, BGP_NEXTHOP_REGISTERED); UNSET_FLAG(bnc->flags, BGP_NEXTHOP_VALID); } - else if (!connected && CHECK_FLAG(bnc->flags, BGP_NEXTHOP_CONNECTED)) + else if (peer && !connected && CHECK_FLAG(bnc->flags, BGP_NEXTHOP_CONNECTED)) { UNSET_FLAG(bnc->flags, BGP_NEXTHOP_CONNECTED); UNSET_FLAG(bnc->flags, BGP_NEXTHOP_REGISTERED);