From 094dcc3cdac19d3da65b38effc45aa88d960909f Mon Sep 17 00:00:00 2001 From: Francois Dumontet Date: Tue, 23 Apr 2024 11:16:24 +0200 Subject: [PATCH] bgpd: fix "bgp as-pah access-list" with "set aspath exclude" set/unset 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 --- bgpd/bgp_aspath.c | 47 ++++++++++++++++++++++++++++++++++++ bgpd/bgp_aspath.h | 7 ++++++ bgpd/bgp_filter.c | 41 +++++++++++++++++++------------ bgpd/bgp_filter.h | 13 +++++----- bgpd/bgp_routemap.c | 59 ++++++++++++++++++--------------------------- 5 files changed, 108 insertions(+), 59 deletions(-) diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c index 9dcd0ad1d621..4c1615a5c625 100644 --- 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 index 2a831c3a5510..f7e57fd66dda 100644 --- 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 /* 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 index a85117965aaa..002f054f5e41 100644 --- 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 index 2d9f07ce84a3..77a3f3c2f727 100644 --- 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 + #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 index df5c2557bd26..f938869b1c48 100644 --- 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; }