bgpd: flowspec pbr entries listed on the bgp information entry

Because one flowspec entry can create 1-N bgp pbr entries, the list is
now updated and visible. Also, because the bgp_extra structure is used,
this list is flushed when the bgp_extra structure is deleted.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>

5 files changed, 48 insertions, 19 deletions

diff --git a/bgpd/bgp_flowspec_vty.c b/bgpd/bgp_flowspec_vty.c
index c695e7f125..3f450fc4ed 100644
--- a/bgpd/bgp_flowspec_vty.c
+++ b/bgpd/bgp_flowspec_vty.c
@@ -332,16 +332,32 @@ void route_vty_out_flowspec(struct vty *vty, struct prefix *p,
 		struct bgp_info_extra *extra = bgp_info_extra_get(binfo);
 
 		if (extra->bgp_fs_pbr) {
+			struct listnode *node;
 			struct bgp_pbr_match_entry *bpme;
 			struct bgp_pbr_match *bpm;
+			int unit = 0;
+			struct list *list_bpm;
 
-			bpme = (struct bgp_pbr_match_entry *)extra->bgp_fs_pbr;
-			bpm = bpme->backpointer;
-			vty_out(vty, "\tinstalled in PBR");
-			if (bpm)
-				vty_out(vty, " (%s)\n", bpm->ipset_name);
-			else
-				vty_out(vty, "\n");
+			list_bpm = list_new();
+			if (listcount(extra->bgp_fs_pbr))
+				vty_out(vty, "\tinstalled in PBR");
+			for (ALL_LIST_ELEMENTS_RO(extra->bgp_fs_pbr,
+						  node, bpme)) {
+				bpm = bpme->backpointer;
+				if (listnode_lookup(list_bpm, bpm))
+					continue;
+				listnode_add(list_bpm, bpm);
+				if (unit == 0)
+					vty_out(vty, " (");
+				else
+					vty_out(vty, ", ");
+				vty_out(vty, "%s", bpm->ipset_name);
+				unit++;
+			}
+			if (unit)
+				vty_out(vty, ")");
+			vty_out(vty, "\n");
+			list_delete_all_node(list_bpm);
 		} else
 			vty_out(vty, "\tnot installed in PBR\n");
 	}
diff --git a/bgpd/bgp_pbr.c b/bgpd/bgp_pbr.c
index b5ddfd4b21..9152307c99 100644
--- a/bgpd/bgp_pbr.c
+++ b/bgpd/bgp_pbr.c
@@ -1225,7 +1225,8 @@ static void bgp_pbr_flush_entry(struct bgp *bgp, struct bgp_pbr_action *bpa,
 			/* unlink bgp_info to bpme */
 			bgp_info = (struct bgp_info *)bpme->bgp_info;
 			extra = bgp_info_extra_get(bgp_info);
-			extra->bgp_fs_pbr = NULL;
+			if (extra->bgp_fs_pbr)
+				listnode_delete(extra->bgp_fs_pbr, bpme);
 			bpme->bgp_info = NULL;
 		}
 	}
@@ -1597,6 +1598,7 @@ static void bgp_pbr_policyroute_add_to_zebra_unit(struct bgp *bgp,
 	struct bgp_pbr_range_port *src_port;
 	struct bgp_pbr_range_port *dst_port;
 	struct bgp_pbr_range_port *pkt_len;
+	bool bpme_found = false;
 
 	if (!bpf)
 		return;
@@ -1817,8 +1819,21 @@ static void bgp_pbr_policyroute_add_to_zebra_unit(struct bgp *bgp,
 		bpme->install_in_progress = false;
 		/* link bgp info to bpme */
 		bpme->bgp_info = (void *)binfo;
-	}
+	} else
+		bpme_found = true;
 
+	/* already installed */
+	if (bpme_found && bpme) {
+		struct bgp_info_extra *extra = bgp_info_extra_get(binfo);
+
+		if (extra && extra->bgp_fs_pbr &&
+		    listnode_lookup(extra->bgp_fs_pbr, bpme)) {
+			if (BGP_DEBUG(pbr, PBR_ERROR))
+				zlog_err("%s: entry %p/%p already installed in bgp pbr",
+					 __func__, binfo, bpme);
+			return;
+		}
+	}
 	/* BGP FS: append entry to zebra
 	 * - policies are not routing entries and as such
 	 * route replace semantics don't necessarily follow
@@ -2195,7 +2210,6 @@ void bgp_pbr_update_entry(struct bgp *bgp, struct prefix *p,
 			 bool nlri_update)
 {
 	struct bgp_pbr_entry_main api;
-	struct bgp_info_extra *extra = bgp_info_extra_get(info);
 
 	if (afi == AFI_IP6)
 		return; /* IPv6 not supported */
@@ -2212,13 +2226,6 @@ void bgp_pbr_update_entry(struct bgp *bgp, struct prefix *p,
 				 __func__);
 		return;
 	}
-	/* already installed */
-	if (nlri_update && extra->bgp_fs_pbr) {
-		if (BGP_DEBUG(pbr, PBR_ERROR))
-			zlog_err("%s: entry %p already installed in bgp pbr",
-				 __func__, info);
-		return;
-	}
 
 	if (bgp_pbr_build_and_validate_entry(p, info, &api) < 0) {
 		if (BGP_DEBUG(pbr, PBR_ERROR))
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index f10f7425c6..d54a5da1fb 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -196,6 +196,10 @@ static void bgp_info_extra_free(struct bgp_info_extra **extra)
 
 	if (e->bgp_orig)
 		bgp_unlock(e->bgp_orig);
+
+	if ((*extra)->bgp_fs_pbr)
+		list_delete_all_node((*extra)->bgp_fs_pbr);
+	(*extra)->bgp_fs_pbr = NULL;
 	XFREE(MTYPE_BGP_ROUTE_EXTRA, *extra);
 
 	*extra = NULL;
diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h
index 72923901b8..dfef9a8f79 100644
--- a/bgpd/bgp_route.h
+++ b/bgpd/bgp_route.h
@@ -147,7 +147,7 @@ struct bgp_info_extra {
 	 */
 	struct prefix nexthop_orig;
 	/* presence of FS pbr entry */
-	void *bgp_fs_pbr;
+	struct list *bgp_fs_pbr;
 };
 
 struct bgp_info {
diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
index 714f6791c1..7123875f77 100644
--- a/bgpd/bgp_zebra.c
+++ b/bgpd/bgp_zebra.c
@@ -2074,7 +2074,9 @@ static int ipset_entry_notify_owner(int command, struct zclient *zclient,
 			/* link bgp_info to bpme */
 			bgp_info = (struct bgp_info *)bgp_pbime->bgp_info;
 			extra = bgp_info_extra_get(bgp_info);
-			extra->bgp_fs_pbr = (void *)bgp_pbime;
+			if (extra->bgp_fs_pbr == NULL)
+				extra->bgp_fs_pbr = list_new();
+			listnode_add(extra->bgp_fs_pbr, bgp_pbime);
 		}
 		break;
 	case ZAPI_IPSET_ENTRY_FAIL_REMOVE: