bgpd: conditional advertisement - comments addressed

Signed-off-by: Madhuri Kuruganti <k.madhuri@samsung.com>

bgpd/bgp_conditional_adv.c

@@ -54,7 +54,7 @@ bgp_check_rmap_prefixes_in_bgp_table(struct bgp_table *table,
 static void bgp_conditional_adv_routes(struct peer *peer, afi_t afi,
 				       safi_t safi, struct bgp_table *table,
 				       struct route_map *rmap,
-				       enum advertise advertise)
+				       enum update_type advertise)
 {
 	int addpath_capable;
 	const struct prefix *dest_p;

bgpd/bgp_route.c

@@ -2039,6 +2039,7 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi,
 			    || CONDITION_MAP_NAME(filter)) {
 				struct peer *temp_peer;
 				struct listnode *temp_node, *temp_nnode = NULL;
+
 				for (ALL_LIST_ELEMENTS(bgp->peer, temp_node,
 						       temp_nnode, temp_peer)) {
 					if (!CHECK_FLAG(peer->flags,

bgpd/bgp_vty.c

@@ -7046,8 +7046,9 @@ ALIAS_HIDDEN(no_neighbor_filter_list, no_neighbor_filter_list_hidden_cmd,
 /* Set advertise-map to the peer. */
 static int peer_advertise_map_set_vty(struct vty *vty, const char *ip_str,
 				      afi_t afi, safi_t safi,
-				      const char *advertise_str, bool condition,
+				      const char *advertise_str,
-				      const char *condition_str)
+				      const char *condition_str, bool condition,
+				      bool set)
 {
 	int ret = CMD_WARNING_CONFIG_FAILED;
 	struct peer *peer;
@@ -7061,42 +7062,22 @@ static int peer_advertise_map_set_vty(struct vty *vty, const char *ip_str,
 	condition_map = route_map_lookup_warn_noexist(vty, condition_str);
 	advertise_map = route_map_lookup_warn_noexist(vty, advertise_str);
 
-	ret = peer_advertise_map_set(peer, afi, safi, advertise_str,
+	if (set)
-				     advertise_map, condition, condition_str,
+		ret = peer_advertise_map_set(peer, afi, safi, advertise_str,
-				     condition_map);
+					     advertise_map, condition_str,
+					     condition_map, condition);
+	else
+		ret = peer_advertise_map_unset(peer, afi, safi, advertise_str,
+					       advertise_map, condition_str,
+					       condition_map, condition);
 
 	return bgp_vty_return(vty, ret);
 }
 
-static int peer_advertise_map_unset_vty(struct vty *vty, const char *ip_str,
+DEFPY (neighbor_advertise_map,
-					afi_t afi, safi_t safi,
-					const char *advertise_str,
-					bool condition,
-					const char *condition_str)
-{
-	int ret = CMD_WARNING_CONFIG_FAILED;
-	struct peer *peer;
-	struct route_map *advertise_map;
-	struct route_map *condition_map;
-
-
-	peer = peer_and_group_lookup_vty(vty, ip_str);
-	if (!peer)
-		return ret;
-
-	condition_map = route_map_lookup_warn_noexist(vty, condition_str);
-	advertise_map = route_map_lookup_warn_noexist(vty, advertise_str);
-
-	ret = peer_advertise_map_unset(peer, afi, safi, advertise_str,
-				       advertise_map, condition, condition_str,
-				       condition_map);
-
-	return bgp_vty_return(vty, ret);
-}
-
-DEFUN (neighbor_advertise_map,
        neighbor_advertise_map_cmd,
-       "neighbor <A.B.C.D|X:X::X:X|WORD> advertise-map WORD <exist-map WORD|non-exist-map WORD>",
+       "[no$no] neighbor <A.B.C.D|X:X::X:X|WORD> advertise-map WORD <exist-map WORD|non-exist-map WORD>",
+       NO_STR
        NEIGHBOR_STR
        NEIGHBOR_ADDR_STR2
        "Route-map to conditionally advertise routes\n"
@@ -7112,55 +7093,23 @@ DEFUN (neighbor_advertise_map,
 	int idx_condition_word = 5;
 	bool condition = CONDITION_EXIST;
 
+	if (no) {
+		idx_peer++;
+		idx_advertise_word++;
+		idx_condition_word++;
+	}
+
 	if (argv_find(argv, argc, "non-exist-map", &idx))
 		condition = CONDITION_NON_EXIST;
 
 	return peer_advertise_map_set_vty(
 		vty, argv[idx_peer]->arg, bgp_node_afi(vty), bgp_node_safi(vty),
-		argv[idx_advertise_word]->arg, condition,
+		argv[idx_advertise_word]->arg, argv[idx_condition_word]->arg,
-		argv[idx_condition_word]->arg);
+		condition, !no);
 }
 
 ALIAS_HIDDEN(neighbor_advertise_map, neighbor_advertise_map_hidden_cmd,
-	     "neighbor <A.B.C.D|X:X::X:X|WORD> advertise-map WORD <exist-map WORD|non-exist-map WORD>",
+	     "[no$no] neighbor <A.B.C.D|X:X::X:X|WORD> advertise-map WORD <exist-map WORD|non-exist-map WORD>",
-	     NEIGHBOR_STR NEIGHBOR_ADDR_STR2
-	     "Route-map to conditionally advertise routes\n"
-	     "Name of advertise map\n"
-	     "Advertise routes only if prefixes in exist-map are installed in BGP table\n"
-	     "Name of the exist map\n"
-	     "Advertise routes only if prefixes in non-exist-map are not installed in BGP table\n"
-	     "Name of the non exist map\n")
-
-DEFUN (no_neighbor_advertise_map,
-       no_neighbor_advertise_map_cmd,
-       "no neighbor <A.B.C.D|X:X::X:X|WORD> advertise-map WORD <exist-map WORD|non-exist-map WORD>",
-       NO_STR
-       NEIGHBOR_STR
-       NEIGHBOR_ADDR_STR2
-       "Route-map to conditionally advertise routes\n"
-       "Name of advertise map\n"
-       "Advertise routes only if prefixes in exist-map are installed in BGP table\n"
-       "Name of the exist map\n"
-       "Advertise routes only if prefixes in non-exist-map are not installed in BGP table\n"
-       "Name of the non exist map\n")
-{
-	int idx = 0;
-	int idx_peer = 2;
-	int idx_advertise_word = 4;
-	int idx_condition_word = 6;
-	bool condition = CONDITION_EXIST;
-
-	if (argv_find(argv, argc, "non-exist-map", &idx))
-		condition = CONDITION_NON_EXIST;
-
-	return peer_advertise_map_unset_vty(
-		vty, argv[idx_peer]->arg, bgp_node_afi(vty), bgp_node_safi(vty),
-		argv[idx_advertise_word]->arg, condition,
-		argv[idx_condition_word]->arg);
-}
-
-ALIAS_HIDDEN(no_neighbor_advertise_map, no_neighbor_advertise_map_hidden_cmd,
-	     "no neighbor <A.B.C.D|X:X::X:X|WORD> advertise-map WORD <exist-map WORD|non-exist-map WORD>",
 	     NO_STR NEIGHBOR_STR NEIGHBOR_ADDR_STR2
 	     "Route-map to conditionally advertise routes\n"
 	     "Name of advertise map\n"
@@ -17543,23 +17492,14 @@ void bgp_vty_init(void)
 
 	/* "neighbor advertise-map" commands. */
 	install_element(BGP_NODE, &neighbor_advertise_map_hidden_cmd);
-	install_element(BGP_NODE, &no_neighbor_advertise_map_hidden_cmd);
 	install_element(BGP_IPV4_NODE, &neighbor_advertise_map_cmd);
-	install_element(BGP_IPV4_NODE, &no_neighbor_advertise_map_cmd);
 	install_element(BGP_IPV4M_NODE, &neighbor_advertise_map_cmd);
-	install_element(BGP_IPV4M_NODE, &no_neighbor_advertise_map_cmd);
 	install_element(BGP_IPV4L_NODE, &neighbor_advertise_map_cmd);
-	install_element(BGP_IPV4L_NODE, &no_neighbor_advertise_map_cmd);
 	install_element(BGP_IPV6_NODE, &neighbor_advertise_map_cmd);
-	install_element(BGP_IPV6_NODE, &no_neighbor_advertise_map_cmd);
 	install_element(BGP_IPV6M_NODE, &neighbor_advertise_map_cmd);
-	install_element(BGP_IPV6M_NODE, &no_neighbor_advertise_map_cmd);
 	install_element(BGP_IPV6L_NODE, &neighbor_advertise_map_cmd);
-	install_element(BGP_IPV6L_NODE, &no_neighbor_advertise_map_cmd);
 	install_element(BGP_VPNV4_NODE, &neighbor_advertise_map_cmd);
-	install_element(BGP_VPNV4_NODE, &no_neighbor_advertise_map_cmd);
 	install_element(BGP_VPNV6_NODE, &neighbor_advertise_map_cmd);
-	install_element(BGP_VPNV6_NODE, &no_neighbor_advertise_map_cmd);
 
 	/* neighbor maximum-prefix-out commands. */
 	install_element(BGP_NODE, &neighbor_maximum_prefix_out_cmd);

bgpd/bgpd.c

@@ -6586,6 +6586,59 @@ int peer_unsuppress_map_unset(struct peer *peer, afi_t afi, safi_t safi)
 	return 0;
 }
 
+static void peer_update_rmap_filter_data(struct peer *peer, afi_t afi,
+					 safi_t safi, const char *rmap_name1,
+					 struct route_map *rmap1,
+					 const char *rmap_name2,
+					 struct route_map *rmap2,
+					 uint8_t config_flags)
+{
+	struct bgp_filter *filter;
+	bool filter_exists = false;
+
+	filter = &peer->filter[afi][safi];
+
+	if (CHECK_FLAG(config_flags, BGP_PEER_ADVERTISE_MAP)) {
+		/* advertise-map is already configured. */
+		if (filter->advmap.aname) {
+			filter_exists = true;
+			XFREE(MTYPE_BGP_FILTER_NAME, filter->advmap.aname);
+			XFREE(MTYPE_BGP_FILTER_NAME, filter->advmap.cname);
+		}
+		route_map_counter_decrement(filter->advmap.amap);
+
+		/* Removed advertise-map configuration */
+		if (!CHECK_FLAG(config_flags, BGP_PEER_RMAP_SET)) {
+			memset(filter, 0, sizeof(struct bgp_filter));
+
+			/* decrement condition_filter_count delete timer if last
+			 * one */
+			if (filter_exists)
+				bgp_conditional_adv_disable(peer, afi, safi);
+
+			return;
+		}
+
+		/* Update filter data with newly configured values. */
+		filter->advmap.aname =
+			XSTRDUP(MTYPE_BGP_FILTER_NAME, rmap_name1);
+		filter->advmap.cname =
+			XSTRDUP(MTYPE_BGP_FILTER_NAME, rmap_name2);
+		filter->advmap.amap = rmap1;
+		filter->advmap.cmap = rmap2;
+		filter->advmap.condition =
+			CHECK_FLAG(config_flags, BGP_PEER_CONDITION_EXIST);
+		route_map_counter_increment(filter->advmap.amap);
+		peer->advmap_config_change[afi][safi] = true;
+
+		/* Increment condition_filter_count and/or create timer. */
+		if (!filter_exists) {
+			filter->advmap.advertise = ADVERTISE;
+			bgp_conditional_adv_enable(peer, afi, safi);
+		}
+	}
+}
+
 /* Set advertise-map to the peer but do not process peer route updates here.  *
  * Hold filter changes until the conditional routes polling thread is called  *
  * AS we need to advertise/withdraw prefixes (in advertise-map) based on the  *
@@ -6595,50 +6648,29 @@ int peer_unsuppress_map_unset(struct peer *peer, afi_t afi, safi_t safi)
  */
 int peer_advertise_map_set(struct peer *peer, afi_t afi, safi_t safi,
 			   const char *advertise_name,
-			   struct route_map *advertise_map, bool condition,
+			   struct route_map *advertise_map,
 			   const char *condition_name,
-			   struct route_map *condition_map)
+			   struct route_map *condition_map, bool condition)
 {
-	bool filter_exists = false;
+	uint8_t config_flags = 0;
 	struct peer *member;
-	struct bgp_filter *filter;
 	struct listnode *node, *nnode;
 
+	SET_FLAG(config_flags, BGP_PEER_RMAP_SET);
+	SET_FLAG(config_flags, BGP_PEER_ADVERTISE_MAP);
+	if (condition)
+		SET_FLAG(config_flags, BGP_PEER_CONDITION_EXIST);
+
 	/* Set configuration on peer. */
-	filter = &peer->filter[afi][safi];
+	peer_update_rmap_filter_data(peer, afi, safi, advertise_name,
+				     advertise_map, condition_name,
+				     condition_map, config_flags);
 
-	if (filter->advmap.aname) {
+	/* Check if handling a regular peer & Skip peer-group mechanics. */
-		/* advertise-map filter is already configured on this peer */
-		filter_exists = true;
-
-		XFREE(MTYPE_BGP_FILTER_NAME, filter->advmap.aname);
-		XFREE(MTYPE_BGP_FILTER_NAME, filter->advmap.cname);
-		filter->advmap.condition = CONDITION_NON_EXIST;
-	}
-
-	route_map_counter_decrement(filter->advmap.amap);
-	filter->advmap.aname = XSTRDUP(MTYPE_BGP_FILTER_NAME, advertise_name);
-	filter->advmap.cname = XSTRDUP(MTYPE_BGP_FILTER_NAME, condition_name);
-	filter->advmap.amap = advertise_map;
-	filter->advmap.cmap = condition_map;
-	filter->advmap.condition = condition;
-	route_map_counter_increment(advertise_map);
-	peer->advmap_config_change[afi][safi] = true;
-
-	/* Check if handling a regular peer. */
 	if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) {
 		/* Set override-flag and process peer route updates. */
 		SET_FLAG(peer->filter_override[afi][safi][RMAP_OUT],
 			 PEER_FT_ADVERTISE_MAP);
-
-		/* Hold peer_on_policy_change() until timer thread is called.
-		 * Increment condition_filter_count and/or create timer.
-		 */
-		if (!filter_exists) {
-			filter->advmap.advertise = ADVERTISE;
-			bgp_conditional_adv_enable(peer, afi, safi);
-		}
-		/* Skip peer-group mechanics for regular peers. */
 		return 0;
 	}
 
@@ -6653,31 +6685,9 @@ int peer_advertise_map_set(struct peer *peer, afi_t afi, safi_t safi,
 			continue;
 
 		/* Set configuration on peer-group member. */
-		filter = &member->filter[afi][safi];
+		peer_update_rmap_filter_data(member, afi, safi, advertise_name,
-		if (filter->advmap.aname) {
+					     advertise_map, condition_name,
-			/* advertise-map filter is already configured. */
+					     condition_map, config_flags);
-			filter_exists = true;
-			XFREE(MTYPE_BGP_FILTER_NAME, filter->advmap.aname);
-			XFREE(MTYPE_BGP_FILTER_NAME, filter->advmap.cname);
-			filter->advmap.condition = CONDITION_NON_EXIST;
-		}
-		route_map_counter_decrement(filter->advmap.amap);
-		filter->advmap.aname =
-			XSTRDUP(MTYPE_BGP_FILTER_NAME, advertise_name);
-		filter->advmap.amap = advertise_map;
-		filter->advmap.cname =
-			XSTRDUP(MTYPE_BGP_FILTER_NAME, condition_name);
-		filter->advmap.cmap = condition_map;
-		filter->advmap.condition = condition;
-		route_map_counter_increment(advertise_map);
-
-		/* Hold peer_on_policy_change() until timer thread is called.
-		 * Increment condition_filter_count, create timer if 1st one
-		 */
-		if (!filter_exists) {
-			filter->advmap.advertise = ADVERTISE;
-			bgp_conditional_adv_enable(member, afi, safi);
-		}
 	}
 
 	return 0;
@@ -6686,15 +6696,18 @@ int peer_advertise_map_set(struct peer *peer, afi_t afi, safi_t safi,
 /* Unset advertise-map from the peer. */
 int peer_advertise_map_unset(struct peer *peer, afi_t afi, safi_t safi,
 			     const char *advertise_name,
-			     struct route_map *advertise_map, bool condition,
+			     struct route_map *advertise_map,
 			     const char *condition_name,
-			     struct route_map *condition_map)
+			     struct route_map *condition_map, bool condition)
 {
-	bool filter_exists = false;
+	uint8_t config_flags = 0;
 	struct peer *member;
-	struct bgp_filter *filter;
 	struct listnode *node, *nnode;
 
+	SET_FLAG(config_flags, BGP_PEER_ADVERTISE_MAP);
+	if (condition)
+		SET_FLAG(config_flags, BGP_PEER_CONDITION_EXIST);
+
 	/* Unset override-flag unconditionally. */
 	UNSET_FLAG(peer->filter_override[afi][safi][RMAP_OUT],
 		   PEER_FT_ADVERTISE_MAP);
@@ -6706,33 +6719,15 @@ int peer_advertise_map_unset(struct peer *peer, afi_t afi, safi_t safi,
 				      MTYPE_BGP_FILTER_NAME);
 		PEER_ATTR_INHERIT(peer, peer->group,
 				  filter[afi][safi].advmap.amap);
-	} else {
+	} else
-		/* Otherwise remove configuration from peer. */
+		peer_update_rmap_filter_data(peer, afi, safi, advertise_name,
-		filter = &peer->filter[afi][safi];
+					     advertise_map, condition_name,
-		if (filter->advmap.aname) {
+					     condition_map, config_flags);
-			/* advertise-map filter is already configured. */
-			filter_exists = true;
-			XFREE(MTYPE_BGP_FILTER_NAME, filter->advmap.aname);
-			XFREE(MTYPE_BGP_FILTER_NAME, filter->advmap.cname);
-		}
-		route_map_counter_decrement(filter->advmap.amap);
-		filter->advmap.aname = NULL;
-		filter->advmap.amap = NULL;
-		filter->advmap.cname = NULL;
-		filter->advmap.cmap = NULL;
-		filter->advmap.condition = CONDITION_NON_EXIST;
-	}
 
-	/* Check if handling a regular peer. */
+	/* Check if handling a regular peer and skip peer-group mechanics. */
 	if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) {
 		/* Process peer route updates. */
 		peer_on_policy_change(peer, afi, safi, 1);
-
-		/* decrement condition_filter_count delete timer if last one */
-		if (filter_exists)
-			bgp_conditional_adv_disable(peer, afi, safi);
-
-		/* Skip peer-group mechanics for regular peers. */
 		return 0;
 	}
 
@@ -6745,25 +6740,10 @@ int peer_advertise_map_unset(struct peer *peer, afi_t afi, safi_t safi,
 		if (CHECK_FLAG(member->filter_override[afi][safi][RMAP_OUT],
 			       PEER_FT_ADVERTISE_MAP))
 			continue;
-
 		/* Remove configuration on peer-group member. */
-		filter = &member->filter[afi][safi];
+		peer_update_rmap_filter_data(member, afi, safi, advertise_name,
-		if (filter->advmap.aname) {
+					     advertise_map, condition_name,
-			/* advertise-map filter is already configured. */
+					     condition_map, config_flags);
-			filter_exists = true;
-			XFREE(MTYPE_BGP_FILTER_NAME, filter->advmap.aname);
-			XFREE(MTYPE_BGP_FILTER_NAME, filter->advmap.cname);
-		}
-		route_map_counter_decrement(filter->advmap.amap);
-		filter->advmap.aname = NULL;
-		filter->advmap.amap = NULL;
-		filter->advmap.cname = NULL;
-		filter->advmap.cmap = NULL;
-		filter->advmap.condition = CONDITION_NON_EXIST;
-
-		/* decrement condition_filter_count delete timer if last one */
-		if (filter_exists)
-			bgp_conditional_adv_disable(peer, afi, safi);
 
 		/* Process peer route updates. */
 		peer_on_policy_change(member, afi, safi, 1);

bgpd/bgpd.h

@@ -764,10 +764,18 @@ struct bgp_nexthop {
 #define BGP_GTSM_HOPS_CONNECTED 1
 
 /* Advertise map */
-#define CONDITION_NON_EXIST 	false
+#define CONDITION_NON_EXIST	false
-#define CONDITION_EXIST     	true
+#define CONDITION_EXIST		true
 
-enum advertise { WITHDRAW, ADVERTISE };
+/* BGP peer RMAP options */
+#define BGP_PEER_ADVERTISE_MAP		(1 << 0)
+#define BGP_PEER_ROUTE_MAP		(1 << 1)
+#define BGP_PEER_UNSUPPRESS_MAP		(1 << 2)
+#define BGP_PEER_CONDITION_EXIST	(1 << 3)
+#define BGP_PEER_RMAP_DIRECTION		(1 << 4)
+#define BGP_PEER_RMAP_SET		(1 << 5)
+
+enum update_type { WITHDRAW, ADVERTISE };
 
 #include "filter.h"
 
@@ -813,7 +821,7 @@ struct bgp_filter {
 		char *cname;
 		struct route_map *cmap;
 
-		enum advertise advertise;
+		enum update_type advertise;
 	} advmap;
 };
 
@@ -1970,8 +1978,9 @@ extern int peer_unsuppress_map_set(struct peer *peer, afi_t afi, safi_t safi,
 extern int peer_advertise_map_set(struct peer *peer, afi_t afi, safi_t safi,
 				  const char *advertise_name,
 				  struct route_map *advertise_map,
-				  bool condition, const char *condition_name,
+				  const char *condition_name,
-				  struct route_map *condition_map);
+				  struct route_map *condition_map,
+				  bool condition);
 
 extern int peer_password_set(struct peer *, const char *);
 extern int peer_password_unset(struct peer *);
@@ -1981,8 +1990,9 @@ extern int peer_unsuppress_map_unset(struct peer *, afi_t, safi_t);
 extern int peer_advertise_map_unset(struct peer *peer, afi_t afi, safi_t safi,
 				    const char *advertise_name,
 				    struct route_map *advertise_map,
-				    bool condition, const char *condition_name,
+				    const char *condition_name,
-				    struct route_map *condition_map);
+				    struct route_map *condition_map,
+				    bool condition);
 
 extern int peer_maximum_prefix_set(struct peer *, afi_t, safi_t, uint32_t,
 				   uint8_t, int, uint16_t, bool force);