bgpd: Send route update when modifying access/aspath/prefix lists

Handle ORF REMOVE_ALL events as well, because now we just silently return, and
a stale dynamic prefix-list is used instead of the new one.

Before this, soft clear/route refresh was needed. Don't know the reason, but
we didn't send updates when modifying the filters.

Probably due to a massive change of filters and to avoid automatic updates :/

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

5 files changed, 48 insertions, 32 deletions

diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 0bbf6f8a2..fe1887565 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -2283,17 +2283,26 @@ static int bgp_route_refresh_receive(struct peer *peer, bgp_size_t size)
 						peer, orf_type, orf_len);
 				}
 
+				/* ORF prefix-list name */
+				snprintf(name, sizeof(name), "%s.%d.%d",
+					 peer->host, afi, safi);
+
 				/* we're going to read at least 1 byte of common
 				 * ORF header,
 				 * and 7 bytes of ORF Address-filter entry from
 				 * the stream
 				 */
-				if (orf_len < 7)
+				if (*p_pnt & ORF_COMMON_PART_REMOVE_ALL) {
+					if (bgp_debug_neighbor_events(peer))
+						zlog_debug(
+							"%pBP rcvd Remove-All pfxlist ORF request",
+							peer);
+					prefix_bgp_orf_remove_all(afi, name);
 					break;
+				}
 
-				/* ORF prefix-list name */
-				snprintf(name, sizeof(name), "%s.%d.%d",
-					 peer->host, afi, safi);
+				if (orf_len < 7)
+					break;
 
 				while (p_pnt < p_end) {
 					/* If the ORF entry is malformed, want
@@ -2306,17 +2315,6 @@ static int bgp_route_refresh_receive(struct peer *peer, bgp_size_t size)
 					memset(&orfp, 0, sizeof(orfp));
 					common = *p_pnt++;
 					/* after ++: p_pnt <= p_end */
-					if (common
-					    & ORF_COMMON_PART_REMOVE_ALL) {
-						if (bgp_debug_neighbor_events(
-							    peer))
-							zlog_debug(
-								"%pBP rcvd Remove-All pfxlist ORF request",
-								peer);
-						prefix_bgp_orf_remove_all(afi,
-									  name);
-						break;
-					}
 					ok = ((uint32_t)(p_end - p_pnt)
 					      >= sizeof(uint32_t));
 					if (ok) {
diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index e94b633b3..33f68c9e8 100644
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -3867,7 +3867,7 @@ static void bgp_route_map_update_peer_group(const char *rmap_name,
  * network statements, etc looking to see if they use this route-map.
  */
 static void bgp_route_map_process_update(struct bgp *bgp, const char *rmap_name,
-					 int route_update)
+					 bool route_update)
 {
 	int i;
 	bool matched;
@@ -4080,7 +4080,7 @@ static void bgp_route_map_process_update_cb(char *rmap_name)
 	struct bgp *bgp;
 
 	for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp)) {
-		bgp_route_map_process_update(bgp, rmap_name, 1);
+		bgp_route_map_process_update(bgp, rmap_name, true);
 
 #ifdef ENABLE_BGP_VNC
 		vnc_routemap_update(bgp, __func__);
@@ -4116,12 +4116,11 @@ static void bgp_route_map_mark_update(const char *rmap_name)
 		/* Signal the groups that a route-map update event has
 		 * started */
 		for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp))
-			update_group_policy_update(bgp,
-						   BGP_POLICY_ROUTE_MAP,
-						   rmap_name, 1, 1);
+			update_group_policy_update(bgp, BGP_POLICY_ROUTE_MAP,
+						   rmap_name, true, 1);
 	} else {
 		for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp)) {
-			bgp_route_map_process_update(bgp, rmap_name, 0);
+			bgp_route_map_process_update(bgp, rmap_name, false);
 #ifdef ENABLE_BGP_VNC
 			vnc_routemap_update(bgp, __func__);
 #endif
diff --git a/bgpd/bgp_updgrp.c b/bgpd/bgp_updgrp.c
index 405dd8f6e..13d5ec6b8 100644
--- a/bgpd/bgp_updgrp.c
+++ b/bgpd/bgp_updgrp.c
@@ -1547,7 +1547,7 @@ static int update_group_periodic_merge_walkcb(struct update_group *updgrp,
  *             update groups.
  */
 void update_group_policy_update(struct bgp *bgp, enum bgp_policy_type ptype,
-				const char *pname, int route_update,
+				const char *pname, bool route_update,
 				int start_event)
 {
 	struct updwalk_context ctx;
diff --git a/bgpd/bgp_updgrp.h b/bgpd/bgp_updgrp.h
index 473017c80..56289d399 100644
--- a/bgpd/bgp_updgrp.h
+++ b/bgpd/bgp_updgrp.h
@@ -298,7 +298,7 @@ struct updwalk_context {
 	enum bgp_policy_type policy_type;
 	const char *policy_name;
 	int policy_event_start_flag;
-	int policy_route_update;
+	bool policy_route_update;
 	updgrp_walkcb cb;
 	void *context;
 	uint8_t flags;
@@ -377,7 +377,7 @@ extern bool update_subgroup_trigger_merge_check(struct update_subgroup *,
 						int force);
 extern void update_group_policy_update(struct bgp *bgp,
 				       enum bgp_policy_type ptype,
-				       const char *pname, int route_update,
+				       const char *pname, bool route_update,
 				       int start_event);
 extern void update_group_af_walk(struct bgp *bgp, afi_t afi, safi_t safi,
 				 updgrp_walkcb cb, void *ctx);
diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index bd3e61377..219dee469 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -5497,12 +5497,23 @@ static void peer_on_policy_change(struct peer *peer, afi_t afi, safi_t safi,
 			return;
 
 		if (CHECK_FLAG(peer->af_flags[afi][safi],
-			       PEER_FLAG_SOFT_RECONFIG))
+			       PEER_FLAG_SOFT_RECONFIG)) {
 			bgp_soft_reconfig_in(peer, afi, safi);
-		else if (CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_OLD_RCV)
-			 || CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_NEW_RCV))
-			bgp_route_refresh_send(peer, afi, safi, 0, 0, 0,
-					       BGP_ROUTE_REFRESH_NORMAL);
+		} else if (CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_OLD_RCV) ||
+			   CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_NEW_RCV)) {
+			if (CHECK_FLAG(peer->af_cap[afi][safi],
+				       PEER_CAP_ORF_PREFIX_SM_ADV) &&
+			    (CHECK_FLAG(peer->af_cap[afi][safi],
+					PEER_CAP_ORF_PREFIX_RM_RCV) ||
+			     CHECK_FLAG(peer->af_cap[afi][safi],
+					PEER_CAP_ORF_PREFIX_RM_OLD_RCV)))
+				peer_clear_soft(peer, afi, safi,
+						BGP_CLEAR_SOFT_IN_ORF_PREFIX);
+			else
+				bgp_route_refresh_send(
+					peer, afi, safi, 0, 0, 0,
+					BGP_ROUTE_REFRESH_NORMAL);
+		}
 	}
 }
 
@@ -6546,7 +6557,7 @@ static void peer_distribute_update(struct access_list *access)
 		if (access->name)
 			update_group_policy_update(bgp,
 						   BGP_POLICY_DISTRIBUTE_LIST,
-						   access->name, 0, 0);
+						   access->name, true, 0);
 		for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) {
 			FOREACH_AFI_SAFI (afi, safi) {
 				filter = &peer->filter[afi][safi];
@@ -6735,7 +6746,7 @@ static void peer_prefix_list_update(struct prefix_list *plist)
 		 */
 		update_group_policy_update(
 			bgp, BGP_POLICY_PREFIX_LIST,
-			plist ? prefix_list_name(plist) : NULL, 0, 0);
+			plist ? prefix_list_name(plist) : NULL, true, 0);
 
 		for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) {
 			FOREACH_AFI_SAFI (afi, safi) {
@@ -6753,6 +6764,14 @@ static void peer_prefix_list_update(struct prefix_list *plist)
 						filter->plist[direct].plist =
 							NULL;
 				}
+
+				/* If we touch prefix-list, we need to process
+				 * new updates. This is important for ORF to
+				 * work correctly as well.
+				 */
+				if (peer->afc_nego[afi][safi])
+					peer_on_policy_change(peer, afi, safi,
+							      0);
 			}
 		}
 		for (ALL_LIST_ELEMENTS(bgp->group, node, nnode, group)) {
@@ -6912,7 +6931,7 @@ static void peer_aslist_update(const char *aslist_name)
 
 	for (ALL_LIST_ELEMENTS(bm->bgp, mnode, mnnode, bgp)) {
 		update_group_policy_update(bgp, BGP_POLICY_FILTER_LIST,
-					   aslist_name, 0, 0);
+					   aslist_name, true, 0);
 
 		for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) {
 			FOREACH_AFI_SAFI (afi, safi) {