From 1d30d1f4a8241ca45df4eb70181211f5af0ff487 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 23 Aug 2018 16:05:02 -0400 Subject: [PATCH 1/2] zebra: When registering a nexthop, we do not always need to re-eval The code prior to this change, was allowing clients to register for nexthop tracking. Then zebra would look up the rnh and send to that particular client any known data. Additionally zebra was blindly re-evaluating the rnh for every registration. This leads to interesting behavior in that all people registered for that nexthop will get callbacks even if nothing changes. Modify the code to know if we have evaluated the rnh or not and if so limit the re-evaluation to when absolutely necessary This is of particular importance to do because of nht callbacks for protocols cause those protocols to do not insignificant work and as more protocols are registering for nht callbacks we will cause more work than is necessary. Signed-off-by: Donald Sharp --- zebra/zapi_msg.c | 10 ++++++++-- zebra/zebra_rnh.c | 22 +++++++++++++++++----- zebra/zebra_rnh.h | 2 +- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index 6990fd95f5..d95f78109c 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -1022,6 +1022,7 @@ static void zread_rnh_register(ZAPI_HANDLER_ARGS) unsigned short l = 0; uint8_t flags = 0; uint16_t type = cmd2type[hdr->command]; + bool exist; if (IS_ZEBRA_DEBUG_NHT) zlog_debug( @@ -1064,7 +1065,10 @@ static void zread_rnh_register(ZAPI_HANDLER_ARGS) p.family); return; } - rnh = zebra_add_rnh(&p, zvrf_id(zvrf), type); + rnh = zebra_add_rnh(&p, zvrf_id(zvrf), type, &exist); + if (!rnh) + return; + if (type == RNH_NEXTHOP_TYPE) { if (flags && !CHECK_FLAG(rnh->flags, ZEBRA_NHT_CONNECTED)) @@ -1084,7 +1088,9 @@ static void zread_rnh_register(ZAPI_HANDLER_ARGS) zebra_add_rnh_client(rnh, client, type, zvrf_id(zvrf)); /* Anything not AF_INET/INET6 has been filtered out above */ - zebra_evaluate_rnh(zvrf_id(zvrf), p.family, 1, type, &p); + if (!exist) + zebra_evaluate_rnh(zvrf_id(zvrf), p.family, 1, type, + &p); } stream_failure: diff --git a/zebra/zebra_rnh.c b/zebra/zebra_rnh.c index 156600c105..0b585af6a0 100644 --- a/zebra/zebra_rnh.c +++ b/zebra/zebra_rnh.c @@ -103,7 +103,8 @@ char *rnh_str(struct rnh *rnh, char *buf, int size) return buf; } -struct rnh *zebra_add_rnh(struct prefix *p, vrf_id_t vrfid, rnh_type_t type) +struct rnh *zebra_add_rnh(struct prefix *p, vrf_id_t vrfid, rnh_type_t type, + bool *exists) { struct route_table *table; struct route_node *rn; @@ -119,6 +120,7 @@ struct rnh *zebra_add_rnh(struct prefix *p, vrf_id_t vrfid, rnh_type_t type) prefix2str(p, buf, sizeof(buf)); zlog_warn("%u: Add RNH %s type %d - table not found", vrfid, buf, type); + exists = false; return NULL; } @@ -136,7 +138,9 @@ struct rnh *zebra_add_rnh(struct prefix *p, vrf_id_t vrfid, rnh_type_t type) route_lock_node(rn); rn->info = rnh; rnh->node = rn; - } + *exists = false; + } else + *exists = true; route_unlock_node(rn); return (rn->info); @@ -190,6 +194,14 @@ void zebra_delete_rnh(struct rnh *rnh, rnh_type_t type) route_unlock_node(rn); } +/* + * This code will send to the registering client + * the looked up rnh. + * For a rnh that was created, there is no data + * so it will send an empty nexthop group + * If rnh exists then we know it has been evaluated + * and as such it will have a resolved rnh. + */ void zebra_add_rnh_client(struct rnh *rnh, struct zserv *client, rnh_type_t type, vrf_id_t vrf_id) { @@ -201,8 +213,7 @@ void zebra_add_rnh_client(struct rnh *rnh, struct zserv *client, } if (!listnode_lookup(rnh->client_list, client)) { listnode_add(rnh->client_list, client); - send_client(rnh, client, type, - vrf_id); // Pending: check if its needed + send_client(rnh, client, type, vrf_id); } } @@ -247,9 +258,10 @@ void zebra_register_rnh_pseudowire(vrf_id_t vrf_id, struct zebra_pw *pw) { struct prefix nh; struct rnh *rnh; + bool exists; addr2hostprefix(pw->af, &pw->nexthop, &nh); - rnh = zebra_add_rnh(&nh, vrf_id, RNH_NEXTHOP_TYPE); + rnh = zebra_add_rnh(&nh, vrf_id, RNH_NEXTHOP_TYPE, &exists); if (rnh && !listnode_lookup(rnh->zebra_pseudowire_list, pw)) { listnode_add(rnh->zebra_pseudowire_list, pw); pw->rnh = rnh; diff --git a/zebra/zebra_rnh.h b/zebra/zebra_rnh.h index 9e09a1bc6f..f4b4b56390 100644 --- a/zebra/zebra_rnh.h +++ b/zebra/zebra_rnh.h @@ -68,7 +68,7 @@ static inline int rnh_resolve_via_default(int family) } extern struct rnh *zebra_add_rnh(struct prefix *p, vrf_id_t vrfid, - rnh_type_t type); + rnh_type_t type, bool *exists); extern struct rnh *zebra_lookup_rnh(struct prefix *p, vrf_id_t vrfid, rnh_type_t type); extern void zebra_free_rnh(struct rnh *rnh); From 74f0a94efdfca6c1b3a03d14bf095b6a8f8a2f1f Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 24 Aug 2018 20:42:45 -0400 Subject: [PATCH 2/2] staticd: refcount the nht add/removal When we add / remove a nexthop that we need to track, keep track of the number of times we have done this for each nexthop. Consequently keep track of the number of available nexthops, so that we can just install new routes when we get one that uses a pre-existing nexthop. Deletion of nexthops is done on refcount going to 0. Removal of routes is handled elsewhere for removal. Signed-off-by: Donald Sharp --- staticd/static_zebra.c | 92 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 90 insertions(+), 2 deletions(-) diff --git a/staticd/static_zebra.c b/staticd/static_zebra.c index 56ba70eaf8..a87dc074df 100644 --- a/staticd/static_zebra.c +++ b/staticd/static_zebra.c @@ -34,6 +34,7 @@ #include "log.h" #include "nexthop.h" #include "nexthop_group.h" +#include "hash.h" #include "static_vrf.h" #include "static_routes.h" @@ -43,6 +44,7 @@ /* Zebra structure to hold current status. */ struct zclient *zclient; +static struct hash *static_nht_hash; static struct interface *zebra_interface_if_lookup(struct stream *s) { @@ -176,10 +178,16 @@ static void zebra_connected(struct zclient *zclient) zclient_send_reg_requests(zclient, VRF_DEFAULT); } +struct static_nht_data { + struct prefix *nh; + uint32_t refcount; + uint8_t nh_num; +}; static int static_zebra_nexthop_update(int command, struct zclient *zclient, zebra_size_t length, vrf_id_t vrf_id) { + struct static_nht_data *nhtd, lookup; struct zapi_route nhr; afi_t afi = AFI_IP; @@ -191,6 +199,14 @@ static int static_zebra_nexthop_update(int command, struct zclient *zclient, if (nhr.prefix.family == AF_INET6) afi = AFI_IP6; + memset(&lookup, 0, sizeof(lookup)); + lookup.nh = &nhr.prefix; + + nhtd = hash_lookup(static_nht_hash, &lookup); + if (nhtd) + nhtd->nh_num = nhr.nexthop_num; + + static_nht_update(&nhr.prefix, nhr.nexthop_num, afi, vrf_id); return 1; } @@ -200,10 +216,50 @@ static void static_zebra_capabilities(struct zclient_capabilities *cap) mpls_enabled = cap->mpls_enabled; } +static unsigned int static_nht_hash_key(void *data) +{ + struct static_nht_data *nhtd = data; + + return prefix_hash_key(nhtd->nh); +} + +static int static_nht_hash_cmp(const void *d1, const void *d2) +{ + const struct static_nht_data *nhtd1 = d1; + const struct static_nht_data *nhtd2 = d2; + + return prefix_same(nhtd1->nh, nhtd2->nh); +} + +static void *static_nht_hash_alloc(void *data) +{ + struct static_nht_data *copy = data; + struct static_nht_data *new; + + new = XMALLOC(MTYPE_TMP, sizeof(*new)); + + new->nh = prefix_new(); + prefix_copy(new->nh, copy->nh); + new->refcount = 0; + new->nh_num = 0; + + return new; +} + +static void static_nht_hash_free(void *data) +{ + struct static_nht_data *nhtd = data; + + prefix_free(nhtd->nh); + XFREE(MTYPE_TMP, nhtd); +} + void static_zebra_nht_register(struct static_route *si, bool reg) { + struct static_nht_data *nhtd, lookup; uint32_t cmd; struct prefix p; + afi_t afi = AFI_IP; cmd = (reg) ? ZEBRA_NEXTHOP_REGISTER : ZEBRA_NEXTHOP_UNREGISTER; @@ -224,20 +280,48 @@ void static_zebra_nht_register(struct static_route *si, bool reg) p.family = AF_INET; p.prefixlen = IPV4_MAX_BITLEN; p.u.prefix4 = si->addr.ipv4; + afi = AFI_IP; break; case STATIC_IPV6_GATEWAY: case STATIC_IPV6_GATEWAY_IFNAME: p.family = AF_INET6; p.prefixlen = IPV6_MAX_BITLEN; p.u.prefix6 = si->addr.ipv6; + afi = AFI_IP6; break; } + memset(&lookup, 0, sizeof(lookup)); + lookup.nh = &p; + + si->nh_registered = reg; + + if (reg) { + nhtd = hash_get(static_nht_hash, &lookup, + static_nht_hash_alloc); + nhtd->refcount++; + + if (nhtd->refcount > 1) { + static_nht_update(nhtd->nh, nhtd->nh_num, + afi, si->nh_vrf_id); + return; + } + } else { + nhtd = hash_lookup(static_nht_hash, &lookup); + if (!nhtd) + return; + + nhtd->refcount--; + if (nhtd->refcount >= 1) + return; + + hash_release(static_nht_hash, nhtd); + static_nht_hash_free(nhtd); + } + if (zclient_send_rnh(zclient, cmd, &p, false, si->nh_vrf_id) < 0) zlog_warn("%s: Failure to send nexthop to zebra", __PRETTY_FUNCTION__); - - si->nh_registered = reg; } extern void static_zebra_route_add(struct route_node *rn, @@ -373,4 +457,8 @@ void static_zebra_init(void) zclient->interface_address_delete = interface_address_delete; zclient->route_notify_owner = route_notify_owner; zclient->nexthop_update = static_zebra_nexthop_update; + + static_nht_hash = hash_create(static_nht_hash_key, + static_nht_hash_cmp, + "Static Nexthop Tracking hash"); }