Merge pull request #4731 from mjstapp/fix_redist_update

zebra: redistribute deletes when updating selected route

diff --git a/zebra/redistribute.c b/zebra/redistribute.c
index 1ae2ba92b0cd513a337216bef39b82eea4938148..4f72e2414af2768db30d5fbfce39f8905195851a 100644 (file)
--- 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 30ff6bcd094c0a69bbbce6c3937158c195740520..2685458f968753a032199a0a22bf3ad9446b00b8 100644 (file)
--- 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 826d31ef371c43557515cbd9f96a659c25a1a5ca..ffdc4dc51252349c16666ed3aa2c0ab272df1d99 100644 (file)
--- 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 884edfef049500c109c5eef6cf176847c0e90efc..996a255ff4beacb1bf2e37cbd73e0d33f8b029e5 100644 (file)
--- 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 d8fb9ae3cf6e00c4bfa20a54794f7122301df816..5428b6eaef2050e8c0424380779b3539863e3098 100644 (file)
--- 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());