From 40f321c000862f9d25f29c6003c53f21997bba80 Mon Sep 17 00:00:00 2001 From: Mark Stapp Date: Wed, 14 Aug 2019 13:30:24 -0400 Subject: [PATCH] zebra: revise redistribution delete to improve update case When selecting a new best route, zebra sends a redist update when the route is installed. There are cases where redist clients may not see that redist add - clients who are not subscribed to the new route type, e.g. In that case, attempt to send a redist delete for the old/previous route type. Revised the redist delete api to accomodate both cases; also tightened up the const-ness of a few internal redist apis. Signed-off-by: Mark Stapp --- zebra/redistribute.c | 94 +++++++++++++++++++++++++++++++++----------- zebra/redistribute.h | 18 ++++++--- zebra/zapi_msg.c | 3 +- zebra/zapi_msg.h | 3 +- zebra/zebra_rib.c | 17 ++++++-- 5 files changed, 100 insertions(+), 35 deletions(-) diff --git a/zebra/redistribute.c b/zebra/redistribute.c index 1ae2ba92b0..4f72e2414a 100644 --- a/zebra/redistribute.c +++ b/zebra/redistribute.c @@ -149,7 +149,8 @@ static void zebra_redistribute(struct zserv *client, int type, /* Either advertise a route for redistribution to registered clients or */ /* withdraw redistribution if add cannot be done for client */ void redistribute_update(const struct prefix *p, const struct prefix *src_p, - struct route_entry *re, struct route_entry *prev_re) + const struct route_entry *re, + const struct route_entry *prev_re) { struct listnode *node, *nnode; struct zserv *client; @@ -226,54 +227,99 @@ void redistribute_update(const struct prefix *p, const struct prefix *src_p, } } +/* + * During a route delete, where 'new_re' is NULL, redist a delete to all + * clients registered for the type of 'old_re'. + * During a route update, redist a delete to any clients who will not see + * an update when the new route is installed. There are cases when a client + * may have seen a redist for 'old_re', but will not see + * the redist for 'new_re'. + */ void redistribute_delete(const struct prefix *p, const struct prefix *src_p, - struct route_entry *re) + const struct route_entry *old_re, + const struct route_entry *new_re) { struct listnode *node, *nnode; struct zserv *client; - char buf[INET6_ADDRSTRLEN]; int afi; + char buf[PREFIX_STRLEN]; + vrf_id_t vrfid; + + if (old_re) + vrfid = old_re->vrf_id; + else if (new_re) + vrfid = new_re->vrf_id; + else + return; if (IS_ZEBRA_DEBUG_RIB) { - inet_ntop(p->family, &p->u.prefix, buf, INET6_ADDRSTRLEN); - zlog_debug("%u:%s/%d: Redist delete re %p (%s)", - re->vrf_id, buf, p->prefixlen, re, - zebra_route_string(re->type)); + zlog_debug( + "%u:%s: Redist del: re %p (%s), new re %p (%s)", + vrfid, prefix2str(p, buf, sizeof(buf)), + old_re, + old_re ? zebra_route_string(old_re->type) : "None", + new_re, + new_re ? zebra_route_string(new_re->type) : "None"); } /* Add DISTANCE_INFINITY check. */ - if (re->distance == DISTANCE_INFINITY) + if (old_re && (old_re->distance == DISTANCE_INFINITY)) return; afi = family2afi(p->family); if (!afi) { flog_warn(EC_ZEBRA_REDISTRIBUTE_UNKNOWN_AF, "%s: Unknown AFI/SAFI prefix received\n", - __FUNCTION__); + __func__); return; } + /* Skip invalid (e.g. linklocal) prefix */ if (!zebra_check_addr(p)) { - if (IS_ZEBRA_DEBUG_RIB) - zlog_debug("Redist delete filter prefix %s", - prefix2str(p, buf, sizeof(buf))); + if (IS_ZEBRA_DEBUG_RIB) { + zlog_debug( + "%u:%s: Redist del old: skipping invalid prefix", + vrfid, prefix2str(p, buf, sizeof(buf))); + } return; } for (ALL_LIST_ELEMENTS(zrouter.client_list, node, nnode, client)) { - if ((is_default_prefix(p) - && vrf_bitmap_check(client->redist_default[afi], - re->vrf_id)) - || vrf_bitmap_check(client->redist[afi][ZEBRA_ROUTE_ALL], - re->vrf_id) - || (re->instance - && redist_check_instance( - &client->mi_redist[afi][re->type], - re->instance)) - || vrf_bitmap_check(client->redist[afi][re->type], - re->vrf_id)) { + if (new_re) { + /* Skip this client if it will receive an update for the + * 'new' re + */ + if (is_default_prefix(p) + && vrf_bitmap_check(client->redist_default[afi], + new_re->vrf_id)) + continue; + else if (vrf_bitmap_check( + client->redist[afi][ZEBRA_ROUTE_ALL], + new_re->vrf_id)) + continue; + else if (new_re->instance + && redist_check_instance( + &client->mi_redist[afi][new_re->type], + new_re->instance)) + continue; + else if (vrf_bitmap_check( + client->redist[afi][new_re->type], + new_re->vrf_id)) + continue; + } + + /* Send a delete for the 'old' re to any subscribed client. */ + if (old_re + && ((old_re->instance + && redist_check_instance( + &client->mi_redist[afi] + [old_re->type], + old_re->instance)) + || vrf_bitmap_check( + client->redist[afi][old_re->type], + old_re->vrf_id))) { zsend_redistribute_route(ZEBRA_REDISTRIBUTE_ROUTE_DEL, - client, p, src_p, re); + client, p, src_p, old_re); } } } diff --git a/zebra/redistribute.h b/zebra/redistribute.h index 30ff6bcd09..2685458f96 100644 --- a/zebra/redistribute.h +++ b/zebra/redistribute.h @@ -42,11 +42,19 @@ extern void zebra_redistribute_default_delete(ZAPI_HANDLER_ARGS); extern void redistribute_update(const struct prefix *p, const struct prefix *src_p, - struct route_entry *re, - struct route_entry *prev_re); -extern void redistribute_delete(const struct prefix *p, - const struct prefix *src_p, - struct route_entry *re); + const struct route_entry *re, + const struct route_entry *prev_re); +/* + * During a route delete, where 'new_re' is NULL, redist a delete to all + * clients registered for the type of 'old_re'. + * During a route update, redist a delete to any clients who will not see + * an update when the new route is installed. There are cases when a client + * may have seen a redist for 'old_re', but will not see + * the redist for 'new_re'. + */ +void redistribute_delete(const struct prefix *p, const struct prefix *src_p, + const struct route_entry *old_re, + const struct route_entry *new_re); extern void zebra_interface_up_update(struct interface *); extern void zebra_interface_down_update(struct interface *); diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index fa6a2f62ec..0e7dc5ce9b 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -516,7 +516,8 @@ int zsend_interface_update(int cmd, struct zserv *client, struct interface *ifp) int zsend_redistribute_route(int cmd, struct zserv *client, const struct prefix *p, - const struct prefix *src_p, struct route_entry *re) + const struct prefix *src_p, + const struct route_entry *re) { struct zapi_route api; struct zapi_nexthop *api_nh; diff --git a/zebra/zapi_msg.h b/zebra/zapi_msg.h index 884edfef04..996a255ff4 100644 --- a/zebra/zapi_msg.h +++ b/zebra/zapi_msg.h @@ -65,7 +65,8 @@ extern int zsend_interface_update(int cmd, struct zserv *client, extern int zsend_redistribute_route(int cmd, struct zserv *zclient, const struct prefix *p, const struct prefix *src_p, - struct route_entry *re); + const struct route_entry *re); + extern int zsend_router_id_update(struct zserv *zclient, struct prefix *p, vrf_id_t vrf_id); extern int zsend_interface_vrf_update(struct zserv *zclient, diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 157c67fa62..ff4bfb10f7 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -691,7 +691,7 @@ static void rib_uninstall(struct route_node *rn, struct route_entry *re) srcdest_rnode_prefixes(rn, &p, &src_p); - redistribute_delete(p, src_p, re); + redistribute_delete(p, src_p, re, NULL); UNSET_FLAG(re->flags, ZEBRA_FLAG_SELECTED); } } @@ -1248,8 +1248,17 @@ static void rib_process(struct route_node *rn) SET_FLAG(new_selected->flags, ZEBRA_FLAG_SELECTED); if (old_selected) { - if (!new_selected) - redistribute_delete(p, src_p, old_selected); + /* + * If we're removing the old entry, we should tell + * redist subscribers about that *if* they aren't + * going to see a redist for the new entry. + */ + if (!new_selected || CHECK_FLAG(old_selected->status, + ROUTE_ENTRY_REMOVED)) + redistribute_delete(p, src_p, + old_selected, + new_selected); + if (old_selected != new_selected) UNSET_FLAG(old_selected->flags, ZEBRA_FLAG_SELECTED); @@ -2026,7 +2035,7 @@ static void rib_process_dplane_notify(struct zebra_dplane_ctx *ctx) dplane_route_notif_update(rn, re, DPLANE_OP_ROUTE_DELETE, ctx); /* Redistribute, lsp, and nht update */ - redistribute_delete(dest_pfx, src_pfx, re); + redistribute_delete(dest_pfx, src_pfx, re, NULL); zebra_rib_evaluate_rn_nexthops( rn, zebra_router_get_next_sequence()); -- 2.39.5