Zebra: Redistribute replace handling corner cases

Ticket: CM-7309
Reviewed By: CCR-3448
Testing Done: passing route_ospf_route_thrash, the new redist test

When zebra was modified to have redistributed routes follow a replace
logic instead of the del-add, one case was missed. When a route is
replaced with a change only to the source protocol (say from static
to bgp), its possible that the new source protocol is not selected
for redistribution by a client. Since we did not delete the route
initially, if the add is not allowed for the new source protocol
for a client, we need to send a delete of that redistributed route.
This is what the patch fixes.

Signed-off-by: Dinesh G Dutt <ddutt@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Vivek Venkatraman <vivek@cumulusnetworks.com>

diff --git a/zebra/redistribute.c b/zebra/redistribute.c
index f55b04ee0db88b6ae26738d98f78875a8cd7197d..5a2a0668b7ca52c4a39a3767a71ff468fdb2a54b 100644 (file)
--- a/zebra/redistribute.c
+++ b/zebra/redistribute.c
@@ -171,58 +171,73 @@ zebra_redistribute (struct zserv *client, int type, u_short instance)
 #endif /* HAVE_IPV6 */
 }
 
+/* Either advertise a route for redistribution to registered clients or */
+/* withdraw redistribution if add cannot be done for client */
 void
-redistribute_add (struct prefix *p, struct rib *rib)
+redistribute_update (struct prefix *p, struct rib *rib, struct rib *prev_rib)
 {
   struct listnode *node, *nnode;
   struct zserv *client;
+  int send_redistribute;
+  int afi;
+
+  afi = family2afi(p->family);
+  if (!afi)
+    {
+      zlog_warn("%s: Unknown AFI/SAFI prefix received\n", __FUNCTION__);
+      return;
+    }
 
   for (ALL_LIST_ELEMENTS (zebrad.client_list, node, nnode, client))
     {
-      if (is_default (p))
-        {
-          if ((p->family == AF_INET) &&
-              (client->redist_default ||
-              redist_check_instance(&client->redist[AFI_IP][rib->type],
-                                    rib->instance)))
+      send_redistribute = 0;
+
+      if (is_default(p) && client->redist_default)
+         send_redistribute = 1;
+
+      if (redist_check_instance(&client->redist[afi][rib->type],
+                               rib->instance))
+       send_redistribute = 1;
+
+      if (send_redistribute)
+       {
+         switch (afi)
            {
+           case AFI_IP:
              client->redist_v4_add_cnt++;
               zsend_redistribute_route (ZEBRA_REDISTRIBUTE_IPV4_ADD, client,
                                        p, rib);
-            }
-#ifdef HAVE_IPV6
-          if ((p->family == AF_INET6) &&
-              (client->redist_default ||
-              redist_check_instance(&client->redist[AFI_IP6][rib->type],
-                                    rib->instance)))
-            {
-              client->redist_v6_add_cnt++;
+             break;
+           case AFI_IP6:
+             client->redist_v6_add_cnt++;
               zsend_redistribute_route (ZEBRA_REDISTRIBUTE_IPV6_ADD, client,
                                     p, rib);
-            }
-#endif /* HAVE_IPV6 */   
-        }
-      else
-        {
-           if ((p->family == AF_INET) &&
-              redist_check_instance(&client->redist[AFI_IP][rib->type],
-                                    rib->instance))
+             break;
+           default:
+             zlog_warn("%s: Unknown AFI/SAFI prefix received\n", __FUNCTION__);
+             break;
+           }
+       }
+      else if (prev_rib &&
+              redist_check_instance(&client->redist[afi][prev_rib->type],
+                                    rib->instance))
+       {
+         switch (afi)
            {
-             client->redist_v4_add_cnt++;
-              zsend_redistribute_route (ZEBRA_REDISTRIBUTE_IPV4_ADD, client,
-                                       p, rib);
-            }
-#ifdef HAVE_IPV6
-          if ((p->family == AF_INET6) &&
-              redist_check_instance(&client->redist[AFI_IP6][rib->type],
-                                    rib->instance))
-            {
-              client->redist_v6_add_cnt++;
-              zsend_redistribute_route (ZEBRA_REDISTRIBUTE_IPV6_ADD, client,
-                                       p, rib);
-            }
-#endif /* HAVE_IPV6 */   
-        }
+           case AFI_IP:
+             client->redist_v4_del_cnt++;
+             zsend_redistribute_route (ZEBRA_REDISTRIBUTE_IPV4_DEL, client, p,
+                                       prev_rib);
+             break;
+           case AFI_IP6:
+             client->redist_v6_del_cnt++;
+             zsend_redistribute_route (ZEBRA_REDISTRIBUTE_IPV6_DEL, client, p,
+                                       prev_rib);
+             break;
+           default:
+             break;
+           }
+       }
     }
 }
 

diff --git a/zebra/redistribute.h b/zebra/redistribute.h
index 3ee6b9cd4df0f33ae4f4be9e39935eb4b3833d86..5bd427515549ef52e07429050637a200a902d2d1 100644 (file)
--- a/zebra/redistribute.h
+++ b/zebra/redistribute.h
@@ -33,7 +33,7 @@ extern void zebra_redistribute_delete (int, struct zserv *, int);
 extern void zebra_redistribute_default_add (int, struct zserv *, int);
 extern void zebra_redistribute_default_delete (int, struct zserv *, int);
 
-extern void redistribute_add (struct prefix *, struct rib *);
+extern void redistribute_update (struct prefix *, struct rib *, struct rib *);
 extern void redistribute_delete (struct prefix *, struct rib *);
 
 extern void zebra_interface_up_update (struct interface *);

diff --git a/zebra/redistribute_null.c b/zebra/redistribute_null.c
index 04a36ab8b2d45c974b20cd33005d37709307ccc5..3fd97a754f10ca38574b13ce5e9b9daaef8f1265 100644 (file)
--- a/zebra/redistribute_null.c
+++ b/zebra/redistribute_null.c
@@ -19,10 +19,10 @@ void zebra_redistribute_default_delete (int a, struct zserv *b, int c)
 { return; }
 #endif
 
-void redistribute_add (struct prefix *a, struct rib *b)
+void redistribute_update (struct prefix *a, struct rib *b, struct rib *c)
 { return; }
 #ifdef HAVE_SYS_WEAK_ALIAS_PRAGMA
-#pragma weak redistribute_delete = redistribute_add
+#pragma weak redistribute_delete = redistribute_update
 #else
 void redistribute_delete (struct prefix *a, struct rib *b)
 { return; }

diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index 4e4306b9fb8f03c1c991408e34d051cdf52e2a69..518cdeb81f5df756acfb313f50e591737b56b64a 100644 (file)
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -1709,7 +1709,7 @@ rib_process (struct route_node *rn)
                rib_install_kernel (rn, select, update_ok);
 
              /* assuming that the receiver knows how to dedup */
-             redistribute_add (&rn->p, select);
+              redistribute_update (&rn->p, select, NULL);
            }
          else
            {
@@ -1808,7 +1808,10 @@ rib_process (struct route_node *rn)
            rib_install_kernel (rn, select, update_ok);
          SET_FLAG (select->flags, ZEBRA_FLAG_SELECTED);
          /* Unconditionally announce, this part is exercised by new routes */
-         redistribute_add (&rn->p, select);
+         /* If we cannot add, for example route added is learnt by the */
+         /* protocol we're trying to redistribute to, delete the redist */
+         /* This is notified by setting the is_update to 1 */
+         redistribute_update (&rn->p, select, fib);
        }
       else
        {
@@ -2953,7 +2956,7 @@ static_uninstall_ipv4 (struct prefix *p, struct static_ipv4 *si)
           if (rib->nexthop_active_num > 1)
             {
               rib_install_kernel (rn, rib, 1);
-              redistribute_add (&rn->p, rib);
+              redistribute_update (&rn->p, rib, NULL);
             }
           else
            {
@@ -3725,7 +3728,7 @@ static_uninstall_ipv6 (struct prefix *p, struct static_ipv6 *si)
           if (rib->nexthop_active_num > 1)
             {
               rib_install_kernel (rn, rib, 0);
-              redistribute_add (&rn->p, rib);
+              redistribute_update (&rn->p, rib, NULL);
             }
         }