pbrd: nhgc state improvements when installed/removed

Cleanup the marking of a nhc as installed/removed based on table
route installation.

We were not even handling the removal state at all.

We saw some timing issues with the routes being installed/removed
multiple times and then never resending the pbr map due to bad states
on the nhgc.

Dont worry about checking if its already marked installed before
scheduling the policy walk. We have a check in `pbr_send_map()`
to ensure we dont try to resend a map sequence already installed.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>

diff --git a/pbrd/pbr_map.c b/pbrd/pbr_map.c
index edc3f1d8daf605d836b3c9f41045b0ddea66d5f0..0ef8432e5e7073ebb0d8dea1fabcb417a5a4570f 100644 (file)
--- a/pbrd/pbr_map.c
+++ b/pbrd/pbr_map.c
@@ -603,7 +603,7 @@ bool pbr_map_check_valid(const char *name)
        return pbrm->valid;
 }
 
-void pbr_map_schedule_policy_from_nhg(const char *nh_group)
+void pbr_map_schedule_policy_from_nhg(const char *nh_group, bool installed)
 {
        struct pbr_map_sequence *pbrms;
        struct pbr_map *pbrm;
@@ -618,7 +618,7 @@ void pbr_map_schedule_policy_from_nhg(const char *nh_group)
 
                        if (pbrms->nhgrp_name
                            && (strcmp(nh_group, pbrms->nhgrp_name) == 0)) {
-                               pbrms->nhs_installed = true;
+                               pbrms->nhs_installed = installed;
 
                                pbr_map_check(pbrms, false);
                        }
@@ -626,7 +626,7 @@ void pbr_map_schedule_policy_from_nhg(const char *nh_group)
                        if (pbrms->nhg
                            && (strcmp(nh_group, pbrms->internal_nhg_name)
                                == 0)) {
-                               pbrms->nhs_installed = true;
+                               pbrms->nhs_installed = installed;
 
                                pbr_map_check(pbrms, false);
                        }

diff --git a/pbrd/pbr_map.h b/pbrd/pbr_map.h
index 41f1703954fc149a7ff63d71612a687d693b5b5a..704f1a0361fc14cd6195d13a0ff6e5afa7e63e45 100644 (file)
--- a/pbrd/pbr_map.h
+++ b/pbrd/pbr_map.h
@@ -194,7 +194,8 @@ extern void pbr_map_check(struct pbr_map_sequence *pbrms, bool changed);
 extern void pbr_map_check_nh_group_change(const char *nh_group);
 extern void pbr_map_reason_string(unsigned int reason, char *buf, int size);
 
-extern void pbr_map_schedule_policy_from_nhg(const char *nh_group);
+extern void pbr_map_schedule_policy_from_nhg(const char *nh_group,
+                                            bool installed);
 
 extern void pbr_map_install(struct pbr_map *pbrm);
 

diff --git a/pbrd/pbr_nht.c b/pbrd/pbr_nht.c
index d7a6751ff1da09be8d25ab457de93bbd20417574..31da65679335be4a1bf36e49e90ad881c45e7a82 100644 (file)
--- a/pbrd/pbr_nht.c
+++ b/pbrd/pbr_nht.c
@@ -328,27 +328,29 @@ static struct pbr_nexthop_cache *pbr_nht_lookup_nexthop(struct nexthop *nexthop)
 }
 #endif
 
+static void
+pbr_nht_find_nhg_from_table_update(struct pbr_nexthop_group_cache *pnhgc,
+                                  uint32_t table_id, bool installed)
+{
+       if (pnhgc->table_id == table_id) {
+               DEBUGD(&pbr_dbg_nht, "%s: %s: Table ID (%u) matches %s",
+                      __func__, (installed ? "install" : "remove"), table_id,
+                      pnhgc->name);
+
+               pnhgc->installed = installed;
+               pnhgc->valid = installed;
+               pbr_map_schedule_policy_from_nhg(pnhgc->name, pnhgc->installed);
+       }
+}
+
 static void pbr_nht_find_nhg_from_table_install(struct hash_bucket *b,
                                                void *data)
 {
        struct pbr_nexthop_group_cache *pnhgc =
                (struct pbr_nexthop_group_cache *)b->data;
-       uint32_t *table_id = (uint32_t *)data;
-
-       if (pnhgc->table_id == *table_id) {
-               DEBUGD(&pbr_dbg_nht, "%s: Table ID (%u) matches %s", __func__,
-                      *table_id, pnhgc->name);
+       uint32_t table_id = *(uint32_t *)data;
 
-               /*
-                * If the table has been re-handled by zebra
-                * and we are already installed no need to do
-                * anything here.
-                */
-               if (!pnhgc->installed) {
-                       pnhgc->installed = true;
-                       pbr_map_schedule_policy_from_nhg(pnhgc->name);
-               }
-       }
+       pbr_nht_find_nhg_from_table_update(pnhgc, table_id, true);
 }
 
 void pbr_nht_route_installed_for_table(uint32_t table_id)
@@ -360,7 +362,11 @@ void pbr_nht_route_installed_for_table(uint32_t table_id)
 static void pbr_nht_find_nhg_from_table_remove(struct hash_bucket *b,
                                               void *data)
 {
-       ;
+       struct pbr_nexthop_group_cache *pnhgc =
+               (struct pbr_nexthop_group_cache *)b->data;
+       uint32_t table_id = *(uint32_t *)data;
+
+       pbr_nht_find_nhg_from_table_update(pnhgc, table_id, false);
 }
 
 void pbr_nht_route_removed_for_table(uint32_t table_id)