Merge pull request #2051 from donaldsharp/PBRD_EXTRA

Pbrd extra

7 files changed, 149 insertions, 137 deletions

diff --git a/pbrd/pbr_debug.c b/pbrd/pbr_debug.c
index e9b4a5295..82f045c46 100644
--- a/pbrd/pbr_debug.c
+++ b/pbrd/pbr_debug.c
@@ -43,16 +43,7 @@ const char *pbr_debugs_conflines[] = {
 	"debug pbr events",
 };
 
-/*
- * Set or unset flags on all debugs for pbrd.
- *
- * flags
- *    The flags to set
- *
- * set
- *    Whether to set or unset the specified flags
- */
-static void pbr_debug_set_all(uint32_t flags, bool set)
+void pbr_debug_set_all(uint32_t flags, bool set)
 {
 	for (unsigned int i = 0; i < array_size(pbr_debugs); i++) {
 		DEBUG_FLAGS_SET(pbr_debugs[i], flags, set);
@@ -63,36 +54,13 @@ static void pbr_debug_set_all(uint32_t flags, bool set)
 	}
 }
 
-/*
- * Check flags on all debugs for pbrd.
- *
- * flags
- *    The flags to set
- *
- * Returns:
- *    The subset of the given flags that were set in all pbrd debugs
- */
-static uint32_t pbr_debug_check_all(uint32_t flags)
-{
-	uint32_t mode = DEBUG_MODE_ALL;
-
-	for (unsigned int i = 0; i < array_size(pbr_debugs); i++)
-		mode &= DEBUG_MODE_CHECK(pbr_debugs[i], flags);
-	return mode;
-}
-
-static int pbr_debug_config_write_helper(struct vty *vty, bool config)
+int pbr_debug_config_write_helper(struct vty *vty, bool config)
 {
 	uint32_t mode = DEBUG_MODE_ALL;
 
 	if (config)
 		mode = DEBUG_MODE_CONF;
 
-	if (pbr_debug_check_all(DEBUG_MODE_CONF) == mode) {
-		vty_out(vty, "debug pbr\n");
-		return 0;
-	}
-
 	for (unsigned int i = 0; i < array_size(pbr_debugs); i++)
 		if (DEBUG_MODE_CHECK(pbr_debugs[i], mode))
 			vty_out(vty, "%s\n", pbr_debugs_conflines[i]);
@@ -104,70 +72,9 @@ int pbr_debug_config_write(struct vty *vty)
 	return pbr_debug_config_write_helper(vty, true);
 }
 
-/* PBR debugging CLI ------------------------------------------------------- */
-/* clang-format off */
-
-DEFPY(debug_pbr,
-      debug_pbr_cmd,
-      "[no] debug pbr [{map$map|zebra$zebra|nht$nht|events$events}]",
-      NO_STR
-      DEBUG_STR
-      "Policy Based Routing\n"
-      "Policy maps\n"
-      "PBRD <-> Zebra communications\n"
-      "Nexthop tracking\n"
-      "Events\n")
-{
-	uint32_t mode = DEBUG_NODE2MODE(vty->node);
-
-	if (map)
-		DEBUG_MODE_SET(&pbr_dbg_map, mode, !no);
-	if (zebra)
-		DEBUG_MODE_SET(&pbr_dbg_zebra, mode, !no);
-	if (nht)
-		DEBUG_MODE_SET(&pbr_dbg_nht, mode, !no);
-	if (events)
-		DEBUG_MODE_SET(&pbr_dbg_event, mode, !no);
-
-	/* no specific debug --> act on all of them */
-	if (strmatch(argv[argc - 1]->text, "pbr"))
-		pbr_debug_set_all(mode, !no);
-
-	return CMD_SUCCESS;
-}
-
-DEFUN_NOSH(show_debugging_pbr,
-	   show_debugging_pbr_cmd,
-	   "show debugging [pbr]",
-	   SHOW_STR
-	   DEBUG_STR
-	   "Policy Based Routing\n")
-{
-	vty_out(vty, "PBR debugging status:\n");
-
-	pbr_debug_config_write_helper(vty, false);
-
-	return CMD_SUCCESS;
-}
-
-/* clang-format on */
-/* ------------------------------------------------------------------------- */
-
-static struct cmd_node debug_node = {DEBUG_NODE, "", 1};
-
 struct debug_callbacks pbr_dbg_cbs = {.debug_set_all = pbr_debug_set_all};
 
 void pbr_debug_init(void)
 {
 	debug_init(&pbr_dbg_cbs);
 }
-
-void pbr_debug_init_vty(void)
-{
-	install_node(&debug_node, pbr_debug_config_write);
-
-	install_element(VIEW_NODE, &debug_pbr_cmd);
-	install_element(CONFIG_NODE, &debug_pbr_cmd);
-
-	install_element(VIEW_NODE, &show_debugging_pbr_cmd);
-}
diff --git a/pbrd/pbr_debug.h b/pbrd/pbr_debug.h
index 274472462..e72fb88be 100644
--- a/pbrd/pbr_debug.h
+++ b/pbrd/pbr_debug.h
@@ -38,9 +38,29 @@ extern struct debug pbr_dbg_event;
 void pbr_debug_init(void);
 
 /*
- * Install PBR debugging VTY commands.
+ * Set or unset flags on all debugs for pbrd.
+ *
+ * flags
+ *    The flags to set
+ *
+ * set
+ *    Whether to set or unset the specified flags
+ */
+void pbr_debug_set_all(uint32_t flags, bool set);
+
+/*
+ * Config write helper.
+ *
+ * vty
+ *    Vty to write to
+ *
+ * config
+ *    Whether we are writing to show run or saving config file
+ *
+ * Returns:
+ *    0 for convenience
  */
-void pbr_debug_init_vty(void);
+int pbr_debug_config_write_helper(struct vty *vty, bool config);
 
 /*
  * Print PBR debugging configuration.
diff --git a/pbrd/pbr_map.c b/pbrd/pbr_map.c
index 10940e154..eb2c082fb 100644
--- a/pbrd/pbr_map.c
+++ b/pbrd/pbr_map.c
@@ -152,8 +152,9 @@ void pbr_map_add_interface(struct pbr_map *pbrm, struct interface *ifp_add)
 	pmi->pbrm = pbrm;
 	listnode_add_sort(pbrm->incoming, pmi);
 
+	bf_assign_index(pbrm->ifi_bitfield, pmi->install_bit);
 	pbr_map_check_valid(pbrm->name);
-	if (pbrm->valid && !pbrm->installed)
+	if (pbrm->valid)
 		pbr_map_install(pbrm);
 }
 
@@ -193,6 +194,8 @@ extern void pbr_map_delete(struct pbr_map_sequence *pbrms)
 
 	if (pbrm->seqnumbers->count == 0) {
 		RB_REMOVE(pbr_map_entry_head, &pbr_maps, pbrm);
+
+		bf_free(pbrm->ifi_bitfield);
 		XFREE(MTYPE_PBR_MAP, pbrm);
 	}
 }
@@ -210,13 +213,12 @@ void pbr_map_delete_nexthop_group(struct pbr_map_sequence *pbrms)
 
 	pbrm->valid = false;
 	pbrms->nhs_installed = false;
-	pbrms->installed = false;
 	pbrms->reason |= PBR_MAP_INVALID_NO_NEXTHOPS;
 	pbrms->nhgrp_name = NULL;
 }
 
-struct pbr_map_sequence *pbrms_lookup_unique(uint32_t unique,
-					     ifindex_t ifindex)
+struct pbr_map_sequence *pbrms_lookup_unique(uint32_t unique, ifindex_t ifindex,
+					     struct pbr_map_interface **ppmi)
 {
 	struct pbr_map_sequence *pbrms;
 	struct listnode *snode, *inode;
@@ -228,6 +230,9 @@ struct pbr_map_sequence *pbrms_lookup_unique(uint32_t unique,
 			if (pmi->ifp->ifindex != ifindex)
 				continue;
 
+			if (ppmi)
+				*ppmi = pmi;
+
 			for (ALL_LIST_ELEMENTS_RO(pbrm->seqnumbers, snode,
 						  pbrms)) {
 				DEBUGD(&pbr_dbg_map, "%s: Comparing %u to %u",
@@ -284,6 +289,7 @@ struct pbr_map_sequence *pbrms_get(const char *name, uint32_t seqno)
 
 		RB_INSERT(pbr_map_entry_head, &pbr_maps, pbrm);
 
+		bf_init(pbrm->ifi_bitfield, 64);
 		pbr_map_add_interfaces(pbrm);
 	}
 
@@ -305,8 +311,6 @@ struct pbr_map_sequence *pbrms_get(const char *name, uint32_t seqno)
 
 		QOBJ_REG(pbrms, pbr_map_sequence);
 		listnode_add_sort(pbrm->seqnumbers, pbrms);
-
-		pbrm->installed = false;
 	}
 
 	return pbrms;
@@ -463,6 +467,8 @@ void pbr_map_policy_delete(struct pbr_map *pbrm, struct pbr_map_interface *pmi)
 
 	listnode_delete(pbrm->incoming, pmi);
 	pmi->pbrm = NULL;
+
+	bf_release_index(pbrm->ifi_bitfield, pmi->install_bit);
 	XFREE(MTYPE_PBR_MAP_INTERFACE, pmi);
 }
 
@@ -541,8 +547,9 @@ void pbr_map_check(struct pbr_map_sequence *pbrms)
 		       pbrms->seqno, pbrms->reason);
 	}
 
-	for (ALL_LIST_ELEMENTS_RO(pbrm->incoming, inode, pmi))
+	for (ALL_LIST_ELEMENTS_RO(pbrm->incoming, inode, pmi)) {
 		pbr_send_pbr_map(pbrms, pmi, install);
+	}
 }
 
 void pbr_map_install(struct pbr_map *pbrm)
@@ -557,8 +564,6 @@ void pbr_map_install(struct pbr_map *pbrm)
 	for (ALL_LIST_ELEMENTS_RO(pbrm->seqnumbers, node, pbrms))
 		for (ALL_LIST_ELEMENTS_RO(pbrm->incoming, inode, pmi))
 			pbr_send_pbr_map(pbrms, pmi, true);
-
-	pbrm->installed = true;
 }
 
 void pbr_map_init(void)
diff --git a/pbrd/pbr_map.h b/pbrd/pbr_map.h
index 5cb22d742..7cd079d16 100644
--- a/pbrd/pbr_map.h
+++ b/pbrd/pbr_map.h
@@ -20,6 +20,8 @@
 #ifndef __PBR_MAP_H__
 #define __PBR_MAP_H__
 
+#include <bitfield.h>
+
 struct pbr_map {
 	/*
 	 * RB Tree of the pbr_maps
@@ -40,20 +42,21 @@ struct pbr_map {
 	 */
 	struct list *incoming;
 
+	bitfield_t ifi_bitfield;
 	/*
 	 * If valid is true we think the pbr_map is valid,
 	 * If false, look in individual pbrms to see
 	 * what we think is the invalid reason
 	 */
 	bool valid;
-
-	bool installed;
 };
 
 RB_HEAD(pbr_map_entry_head, pbr_map);
 RB_PROTOTYPE(pbr_map_entry_head, pbr_map, pbr_map_entry, pbr_map_compare)
 
 struct pbr_map_interface {
+	uint32_t install_bit;
+
 	struct interface *ifp;
 
 	struct pbr_map *pbrm;
@@ -112,7 +115,7 @@ struct pbr_map_sequence {
 	/*
 	 * Are we installed
 	 */
-	bool installed;
+	uint64_t installed;
 
 	/*
 	 * A reason of 0 means we think the pbr_map_sequence is good to go
@@ -134,8 +137,9 @@ DECLARE_QOBJ_TYPE(pbr_map_sequence)
 extern struct pbr_map_entry_head pbr_maps;
 
 extern struct pbr_map_sequence *pbrms_get(const char *name, uint32_t seqno);
-extern struct pbr_map_sequence *pbrms_lookup_unique(uint32_t unique,
-						    ifindex_t ifindex);
+extern struct pbr_map_sequence *
+pbrms_lookup_unique(uint32_t unique, ifindex_t ifindex,
+		    struct pbr_map_interface **ppmi);
 
 extern struct pbr_map *pbrm_find(const char *name);
 extern void pbr_map_delete(struct pbr_map_sequence *pbrms);
diff --git a/pbrd/pbr_nht.c b/pbrd/pbr_nht.c
index 19bc1804d..1ccf3ebff 100644
--- a/pbrd/pbr_nht.c
+++ b/pbrd/pbr_nht.c
@@ -319,8 +319,16 @@ static void pbr_nht_find_nhg_from_table_install(struct hash_backet *b,
 	if (pnhgc->table_id == *table_id) {
 		DEBUGD(&pbr_dbg_nht, "%s: Table ID (%u) matches %s",
 		       __PRETTY_FUNCTION__, *table_id, pnhgc->name);
-		pnhgc->installed = true;
-		pbr_map_schedule_policy_from_nhg(pnhgc->name);
+
+		/*
+		 * 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);
+		}
 	}
 }
 
@@ -409,8 +417,6 @@ static void pbr_nht_install_nexthop_group(struct pbr_nexthop_group_cache *pnhgc,
 
 	install_afi = pbr_nht_which_afi(nhg, nh_afi);
 
-	pnhgc->installed = false;
-
 	route_add(pnhgc, nhg, install_afi);
 }
 
@@ -511,7 +517,6 @@ void pbr_nht_delete_individual_nexthop(struct pbr_map_sequence *pbrms)
 
 	pbrm->valid = false;
 	pbrms->nhs_installed = false;
-	pbrms->installed = false;
 	pbrms->reason |= PBR_MAP_INVALID_NO_NEXTHOPS;
 
 	memset(&find, 0, sizeof(find));
diff --git a/pbrd/pbr_vty.c b/pbrd/pbr_vty.c
index 16ae0ee2b..475ad86b5 100644
--- a/pbrd/pbr_vty.c
+++ b/pbrd/pbr_vty.c
@@ -251,8 +251,14 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd,
 					intf, vrf->name);
 				return CMD_WARNING_CONFIG_FAILED;
 			}
-		} else
+		} else {
+			if (IN6_IS_ADDR_LINKLOCAL(&nhop.gate.ipv6)) {
+				vty_out(vty,
+					"Specified a v6 LL with no interface, rejecting\n");
+				return CMD_WARNING_CONFIG_FAILED;
+			}
 			nhop.type = NEXTHOP_TYPE_IPV6;
+		}
 	}
 
 	if (pbrms->nhg)
@@ -383,7 +389,7 @@ DEFPY (show_pbr_map,
 				pbr_map_reason_string(pbrms->reason, rbuf,
 						      sizeof(rbuf));
 			vty_out(vty,
-				"    Seq: %u rule: %u Installed: %d(%u) Reason: %s\n",
+				"    Seq: %u rule: %u Installed: %" PRIu64 "(%u) Reason: %s\n",
 				pbrms->seqno, pbrms->ruleno, pbrms->installed,
 				pbrms->unique, pbrms->reason ? rbuf : "Valid");
 
@@ -477,6 +483,58 @@ DEFPY (show_pbr_interface,
 	return CMD_SUCCESS;
 }
 
+/* PBR debugging CLI ------------------------------------------------------- */
+/* clang-format off */
+
+static struct cmd_node debug_node = {DEBUG_NODE, "", 1};
+
+DEFPY(debug_pbr,
+      debug_pbr_cmd,
+      "[no] debug pbr [{map$map|zebra$zebra|nht$nht|events$events}]",
+      NO_STR
+      DEBUG_STR
+      "Policy Based Routing\n"
+      "Policy maps\n"
+      "PBRD <-> Zebra communications\n"
+      "Nexthop tracking\n"
+      "Events\n")
+{
+	uint32_t mode = DEBUG_NODE2MODE(vty->node);
+
+	if (map)
+		DEBUG_MODE_SET(&pbr_dbg_map, mode, !no);
+	if (zebra)
+		DEBUG_MODE_SET(&pbr_dbg_zebra, mode, !no);
+	if (nht)
+		DEBUG_MODE_SET(&pbr_dbg_nht, mode, !no);
+	if (events)
+		DEBUG_MODE_SET(&pbr_dbg_event, mode, !no);
+
+	/* no specific debug --> act on all of them */
+	if (strmatch(argv[argc - 1]->text, "pbr"))
+		pbr_debug_set_all(mode, !no);
+
+	return CMD_SUCCESS;
+}
+
+DEFUN_NOSH(show_debugging_pbr,
+	   show_debugging_pbr_cmd,
+	   "show debugging [pbr]",
+	   SHOW_STR
+	   DEBUG_STR
+	   "Policy Based Routing\n")
+{
+	vty_out(vty, "PBR debugging status:\n");
+
+	pbr_debug_config_write_helper(vty, false);
+
+	return CMD_SUCCESS;
+}
+
+/* clang-format on */
+/* ------------------------------------------------------------------------- */
+
+
 static struct cmd_node interface_node = {
 	INTERFACE_NODE, "%s(config-if)# ", 1 /* vtysh ? yes */
 };
@@ -561,6 +619,12 @@ void pbr_vty_init(void)
 	install_node(&pbr_map_node,
 		     pbr_vty_map_config_write);
 
+	/* debug */
+	install_node(&debug_node, pbr_debug_config_write);
+	install_element(VIEW_NODE, &debug_pbr_cmd);
+	install_element(CONFIG_NODE, &debug_pbr_cmd);
+	install_element(VIEW_NODE, &show_debugging_pbr_cmd);
+
 	install_default(PBRMAP_NODE);
 
 	install_element(CONFIG_NODE, &pbr_map_cmd);
@@ -574,6 +638,4 @@ void pbr_vty_init(void)
 	install_element(VIEW_NODE, &show_pbr_map_cmd);
 	install_element(VIEW_NODE, &show_pbr_interface_cmd);
 	install_element(VIEW_NODE, &show_pbr_nexthop_group_cmd);
-
-	pbr_debug_init_vty();
 }
diff --git a/pbrd/pbr_zebra.c b/pbrd/pbr_zebra.c
index a1a2d34ac..4e5b5f3dd 100644
--- a/pbrd/pbr_zebra.c
+++ b/pbrd/pbr_zebra.c
@@ -45,17 +45,6 @@ DEFINE_MTYPE_STATIC(PBRD, PBR_INTERFACE, "PBR Interface")
 /* Zebra structure to hold current status. */
 struct zclient *zclient;
 
-static struct interface *zebra_interface_if_lookup(struct stream *s)
-{
-	char ifname_tmp[INTERFACE_NAMSIZ];
-
-	/* Read interface name. */
-	stream_get(ifname_tmp, s, INTERFACE_NAMSIZ);
-
-	/* And look it up. */
-	return if_lookup_by_name(ifname_tmp, VRF_DEFAULT);
-}
-
 struct pbr_interface *pbr_if_new(struct interface *ifp)
 {
 	struct pbr_interface *pbr_ifp;
@@ -140,7 +129,7 @@ static int interface_state_up(int command, struct zclient *zclient,
 			      zebra_size_t length, vrf_id_t vrf_id)
 {
 
-	zebra_interface_if_lookup(zclient->ibuf);
+	zebra_interface_state_read(zclient->ibuf, vrf_id);
 
 	return 0;
 }
@@ -206,13 +195,16 @@ static int rule_notify_owner(int command, struct zclient *zclient,
 	uint32_t seqno, priority, unique;
 	enum zapi_rule_notify_owner note;
 	struct pbr_map_sequence *pbrms;
+	struct pbr_map_interface *pmi;
 	ifindex_t ifi;
+	uint64_t installed;
 
 	if (!zapi_rule_notify_decode(zclient->ibuf, &seqno, &priority, &unique,
 				     &ifi, &note))
 		return -1;
 
-	pbrms = pbrms_lookup_unique(unique, ifi);
+	pmi = NULL;
+	pbrms = pbrms_lookup_unique(unique, ifi, &pmi);
 	if (!pbrms) {
 		DEBUGD(&pbr_dbg_zebra,
 		       "%s: Failure to lookup pbrms based upon %u",
@@ -220,18 +212,21 @@ static int rule_notify_owner(int command, struct zclient *zclient,
 		return 0;
 	}
 
+	installed = 1 << pmi->install_bit;
+
 	switch (note) {
 	case ZAPI_RULE_FAIL_INSTALL:
 		DEBUGD(&pbr_dbg_zebra, "%s: Recieved RULE_FAIL_INSTALL",
 		       __PRETTY_FUNCTION__);
-		pbrms->installed = false;
+		pbrms->installed &= ~installed;
 		break;
 	case ZAPI_RULE_INSTALLED:
-		pbrms->installed = true;
+		pbrms->installed |= installed;
 		DEBUGD(&pbr_dbg_zebra, "%s: Recived RULE_INSTALLED",
 		       __PRETTY_FUNCTION__);
 		break;
 	case ZAPI_RULE_REMOVED:
+		pbrms->installed &= ~installed;
 		DEBUGD(&pbr_dbg_zebra, "%s: Received RULE REMOVED",
 		       __PRETTY_FUNCTION__);
 		break;
@@ -499,9 +494,23 @@ void pbr_send_pbr_map(struct pbr_map_sequence *pbrms,
 {
 	struct pbr_map *pbrm = pbrms->parent;
 	struct stream *s;
+	uint64_t is_installed = 1 << pmi->install_bit;
 
-	DEBUGD(&pbr_dbg_zebra, "%s: for %s %d", __PRETTY_FUNCTION__, pbrm->name,
-	       install);
+	is_installed &= pbrms->installed;
+
+	DEBUGD(&pbr_dbg_zebra, "%s: for %s %d(%" PRIu64 ")",
+	       __PRETTY_FUNCTION__, pbrm->name, install, is_installed);
+
+	/*
+	 * If we are installed and asked to do so again
+	 * just return.  If we are not installed and asked
+	 * and asked to delete just return;
+	 */
+	if (install && is_installed)
+		return;
+
+	if (!install && !is_installed)
+		return;
 
 	s = zclient->obuf;
 	stream_reset(s);