bgpd: fix "bgp as-pah access-list" with "set aspath exclude" set/unse…

…t issues whith the following config router bgp 65001 no bgp ebgp-requires-policy neighbor 192.168.1.2 remote-as external neighbor 192.168.1.2 timers 3 10 ! address-family ipv4 unicast neighbor 192.168.1.2 route-map r2 in exit-address-family exit ! bgp as-path access-list FIRST seq 5 permit ^65 bgp as-path access-list SECOND seq 5 permit 2$ ! route-map r2 permit 6 match ip address prefix-list p2 set as-path exclude as-path-access-list SECOND exit ! route-map r2 permit 10 match ip address prefix-list p1 set as-path exclude 65003 exit ! route-map r2 permit 20 match ip address prefix-list p3 set as-path exclude all exit making some no bgp as-path access-list SECOND permit 2$ bgp as-path access-list SECOND permit 3$ clear bgp * no bgp as-path access-list SECOND permit 3$ bgp as-path access-list SECOND permit 2$ clear bgp * will induce some crashes thus we rework the links between aslists and aspath_exclude Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
donaldsharp · May 16, 2024 · 094dcc3 · 094dcc3
1 parent db1e2a0
commit 094dcc3
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 59 deletions.
diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
@@ -77,6 +77,9 @@ static struct hash *ashash;
 /* Stream for SNMP. See aspath_snmp_pathseg */
 static struct stream *snmp_stream;
 
+/* as-path orphan exclude list */
+static struct as_list_list_head as_exclude_list_orphan;
+
 /* Callers are required to initialize the memory */
 static as_t *assegment_data_new(int num)
 {
@@ -1558,6 +1561,38 @@ struct aspath *aspath_prepend(struct aspath *as1, struct aspath *as2)
 	/* Not reached */
 }
 
+/* insert aspath exclude in head of orphan exclude list*/
+void as_exclude_set_orphan(struct aspath_exclude *ase)
+{
+	ase->exclude_aspath_acl = NULL;
+	as_list_list_add_head(&as_exclude_list_orphan, ase);
+}
+
+void as_exclude_remove_orphan(struct aspath_exclude *ase)
+{
+	if (as_list_list_count(&as_exclude_list_orphan))
+		as_list_list_del(&as_exclude_list_orphan, ase);
+}
+
+/* currently provide only one exclude, not a list */
+struct aspath_exclude *as_exclude_lookup_orphan(const char *acl_name)
+{
+	struct aspath_exclude *ase = NULL;
+	char *name = NULL;
+
+	frr_each (as_list_list, &as_exclude_list_orphan, ase) {
+		if (ase->exclude_aspath_acl_name) {
+			name = ase->exclude_aspath_acl_name;
+			if (!strcmp(name, acl_name))
+				break;
+		}
+	}
+	if (ase)
+		as_exclude_remove_orphan(ase);
+
+	return ase;
+}
+
 /* Iterate over AS_PATH segments and wipe all occurrences of the
  * listed AS numbers. Hence some segments may lose some or even
  * all data on the way, the operation is implemented as a smarter
@@ -2236,14 +2271,26 @@ void aspath_init(void)
 {
 	ashash = hash_create_size(32768, aspath_key_make, aspath_cmp,
 				  "BGP AS Path");
+
+	as_list_list_init(&as_exclude_list_orphan);
 }
 
 void aspath_finish(void)
 {
+	struct aspath_exclude *ase;
+
 	hash_clean_and_free(&ashash, (void (*)(void *))aspath_free);
 
 	if (snmp_stream)
 		stream_free(snmp_stream);
+
+	while ((ase = as_list_list_pop(&as_exclude_list_orphan))) {
+		aspath_free(ase->aspath);
+		if (ase->exclude_aspath_acl_name)
+			XFREE(MTYPE_TMP, ase->exclude_aspath_acl_name);
+		XFREE(MTYPE_ROUTE_MAP_COMPILED, ase);
+	}
+	as_list_list_fini(&as_exclude_list_orphan);
 }
 
 /* return and as path value */

diff --git a/bgpd/bgp_aspath.h b/bgpd/bgp_aspath.h
@@ -9,6 +9,7 @@
 #include "lib/json.h"
 #include "bgpd/bgp_route.h"
 #include "bgpd/bgp_filter.h"
+#include <typesafe.h>
 
 /* AS path segment type.  */
 #define AS_SET                       1
@@ -67,11 +68,14 @@ struct aspath {
 
 /* `set as-path exclude ASn' */
 struct aspath_exclude {
+	struct as_list_list_item exclude_list;
 	struct aspath *aspath;
 	bool exclude_all;
 	char *exclude_aspath_acl_name;
 	struct as_list *exclude_aspath_acl;
 };
+DECLARE_DLIST(as_list_list, struct aspath_exclude, exclude_list);
+
 
 /* Prototypes. */
 extern void aspath_init(void);
@@ -83,6 +87,9 @@ extern struct aspath *aspath_parse(struct stream *s, size_t length,
 extern struct aspath *aspath_dup(struct aspath *aspath);
 extern struct aspath *aspath_aggregate(struct aspath *as1, struct aspath *as2);
 extern struct aspath *aspath_prepend(struct aspath *as1, struct aspath *as2);
+extern void as_exclude_set_orphan(struct aspath_exclude *ase);
+extern void as_exclude_remove_orphan(struct aspath_exclude *ase);
+extern struct aspath_exclude *as_exclude_lookup_orphan(const char *acl_name);
 extern struct aspath *aspath_filter_exclude(struct aspath *source,
 					    struct aspath *exclude_list);
 extern struct aspath *aspath_filter_exclude_all(struct aspath *source);

diff --git a/bgpd/bgp_filter.c b/bgpd/bgp_filter.c
@@ -16,7 +16,7 @@
 #include "bgpd/bgp_aspath.h"
 #include "bgpd/bgp_regex.h"
 
-/* List of AS filter list. */
+/* List of AS list. */
 struct as_list_list {
 	struct as_list *head;
 	struct as_list *tail;
@@ -205,14 +205,6 @@ static struct as_list *as_list_new(void)
 
 static void as_list_free(struct as_list *aslist)
 {
-	struct aspath_exclude_list *cur_bp = aslist->exclude_list;
-	struct aspath_exclude_list *next_bp = NULL;
-
-	while (cur_bp) {
-		next_bp = cur_bp->next;
-		XFREE(MTYPE_ROUTE_MAP_COMPILED, cur_bp);
-		cur_bp = next_bp;
-	}
 
 	XFREE (MTYPE_AS_STR, aslist->name);
 	XFREE (MTYPE_AS_LIST, aslist);
@@ -299,7 +291,6 @@ static void as_list_delete(struct as_list *aslist)
 {
 	struct as_list_list *list;
 	struct as_filter *filter, *next;
-	struct aspath_exclude_list *cur_bp;
 
 	for (filter = aslist->head; filter; filter = next) {
 		next = filter->next;
@@ -318,12 +309,6 @@ static void as_list_delete(struct as_list *aslist)
 	else
 		list->head = aslist->next;
 
-	cur_bp = aslist->exclude_list;
-	while (cur_bp) {
-		cur_bp->bp_as_excl->exclude_aspath_acl = NULL;
-		cur_bp = cur_bp->next;
-	}
-
 	as_list_free(aslist);
 }
 
@@ -431,6 +416,7 @@ DEFUN(as_path, bgp_as_path_cmd,
 	enum as_filter_type type;
 	struct as_filter *asfilter;
 	struct as_list *aslist;
+	struct aspath_exclude *ase;
 	regex_t *regex;
 	char *regstr;
 	int64_t seqnum = ASPATH_SEQ_NUMBER_AUTO;
@@ -482,6 +468,22 @@ DEFUN(as_path, bgp_as_path_cmd,
 	else
 		as_list_filter_add(aslist, asfilter);
 
+	/* init the exclude rule list*/
+	as_list_list_init(&aslist->exclude_rule);
+
+	/* get aspath orphan exclude that are using this acl */
+	ase = as_exclude_lookup_orphan(alname);
+	if (ase) {
+		as_list_list_add_head(&aslist->exclude_rule, ase);
+		/* set reverse pointer */
+		ase->exclude_aspath_acl = aslist;
+		/* set list of aspath excludes using that acl */
+		while ((ase = as_exclude_lookup_orphan(alname))) {
+			as_list_list_add_head(&aslist->exclude_rule, ase);
+			ase->exclude_aspath_acl = aslist;
+		}
+	}
+
 	return CMD_SUCCESS;
 }
 
@@ -502,6 +504,7 @@ DEFUN(no_as_path, no_bgp_as_path_cmd,
 	enum as_filter_type type;
 	struct as_filter *asfilter;
 	struct as_list *aslist;
+	struct aspath_exclude *ase;
 	char *regstr;
 	regex_t *regex;
 
@@ -556,6 +559,12 @@ DEFUN(no_as_path, no_bgp_as_path_cmd,
 
 	XFREE(MTYPE_TMP, regstr);
 
+	/* put aspath exclude list into orphan */
+	if (as_list_list_count(&aslist->exclude_rule))
+		while ((ase = as_list_list_pop(&aslist->exclude_rule)))
+			as_exclude_set_orphan(ase);
+
+	as_list_list_fini(&aslist->exclude_rule);
 	as_list_filter_delete(aslist, asfilter);
 
 	return CMD_SUCCESS;

diff --git a/bgpd/bgp_filter.h b/bgpd/bgp_filter.h
@@ -6,11 +6,12 @@
 #ifndef _QUAGGA_BGP_FILTER_H
 #define _QUAGGA_BGP_FILTER_H
 
+#include <typesafe.h>
+
 #define ASPATH_SEQ_NUMBER_AUTO -1
 
 enum as_filter_type { AS_FILTER_DENY, AS_FILTER_PERMIT };
 
-
 /* Element of AS path filter. */
 struct as_filter {
 	struct as_filter *next;
@@ -25,11 +26,7 @@ struct as_filter {
 	int64_t seq;
 };
 
-struct aspath_exclude_list {
-	struct aspath_exclude_list *next;
-	struct aspath_exclude *bp_as_excl;
-};
-
+PREDECL_DLIST(as_list_list);
 /* AS path filter list. */
 struct as_list {
 	char *name;
@@ -39,7 +36,9 @@ struct as_list {
 
 	struct as_filter *head;
 	struct as_filter *tail;
-	struct aspath_exclude_list *exclude_list;
+
+	/* Changes in AS path */
+	struct as_list_list_head exclude_rule;
 };
 
 

diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
@@ -2325,7 +2325,7 @@ static const struct route_map_rule_cmd route_set_aspath_prepend_cmd = {
 static void *route_aspath_exclude_compile(const char *arg)
 {
 	struct aspath_exclude *ase;
-	struct aspath_exclude_list *ael;
+	struct as_list *aux_aslist;
 	const char *str = arg;
 	static const char asp_acl[] = "as-path-access-list";
 
@@ -2337,44 +2337,37 @@ static void *route_aspath_exclude_compile(const char *arg)
 		while (*str == ' ')
 			str++;
 		ase->exclude_aspath_acl_name = XSTRDUP(MTYPE_TMP, str);
-		ase->exclude_aspath_acl = as_list_lookup(str);
+		aux_aslist = as_list_lookup(str);
+		if (!aux_aslist)
+			/* new orphan filter */
+			as_exclude_set_orphan(ase);
+		else
+			as_list_list_add_head(&aux_aslist->exclude_rule, ase);
+
+		ase->exclude_aspath_acl = aux_aslist;
 	} else
 		ase->aspath = aspath_str2aspath(str, bgp_get_asnotation(NULL));
 
-	if (ase->exclude_aspath_acl) {
-		ael = XCALLOC(MTYPE_ROUTE_MAP_COMPILED,
-				sizeof(struct aspath_exclude_list));
-		ael->bp_as_excl = ase;
-		ael->next = ase->exclude_aspath_acl->exclude_list;
-		ase->exclude_aspath_acl->exclude_list = ael;
-	}
-
 	return ase;
 }
 
 static void route_aspath_exclude_free(void *rule)
 {
 	struct aspath_exclude *ase = rule;
-	struct aspath_exclude_list *cur_ael = NULL;
-	struct aspath_exclude_list *prev_ael = NULL;
+	struct as_list *acl;
+
+	/* manage references to that rule*/
+	if (ase->exclude_aspath_acl) {
+		acl = ase->exclude_aspath_acl;
+		as_list_list_del(&acl->exclude_rule, ase);
+	} else {
+		/* no ref to acl, this aspath exclude is orphan */
+		as_exclude_remove_orphan(ase);
+	}
 
 	aspath_free(ase->aspath);
 	if (ase->exclude_aspath_acl_name)
 		XFREE(MTYPE_TMP, ase->exclude_aspath_acl_name);
-	if (ase->exclude_aspath_acl)
-		cur_ael = ase->exclude_aspath_acl->exclude_list;
-	while (cur_ael) {
-		if (cur_ael->bp_as_excl == ase) {
-			if (prev_ael)
-				prev_ael->next = cur_ael->next;
-			else
-				ase->exclude_aspath_acl->exclude_list = NULL;
-			XFREE(MTYPE_ROUTE_MAP_COMPILED, cur_ael);
-			break;
-		}
-		prev_ael = cur_ael;
-		cur_ael = cur_ael->next;
-	}
 	XFREE(MTYPE_ROUTE_MAP_COMPILED, ase);
 }
 
@@ -2409,16 +2402,10 @@ route_set_aspath_exclude(void *rule, const struct prefix *dummy, void *object)
 	else if (ase->exclude_all)
 		path->attr->aspath = aspath_filter_exclude_all(new_path);
 
-	else if (ase->exclude_aspath_acl_name) {
-		if (!ase->exclude_aspath_acl)
-			ase->exclude_aspath_acl =
-				as_list_lookup(ase->exclude_aspath_acl_name);
-		if (ase->exclude_aspath_acl)
-			path->attr->aspath =
-				aspath_filter_exclude_acl(new_path,
-							  ase->exclude_aspath_acl);
-	}
-
+	else if (ase->exclude_aspath_acl)
+		path->attr->aspath =
+			aspath_filter_exclude_acl(new_path,
+						  ase->exclude_aspath_acl);
 	return RMAP_OKAY;
 }