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; } diff --git a/tests/topotests/bgp_set_aspath_exclude/r1/bgpd.conf b/tests/topotests/bgp_set_aspath_exclude/r1/bgpd.conf index 9bef24f93154..c70b4934a079 100644 --- a/tests/topotests/bgp_set_aspath_exclude/r1/bgpd.conf +++ b/tests/topotests/bgp_set_aspath_exclude/r1/bgpd.conf @@ -8,10 +8,19 @@ router bgp 65001 exit-address-family ! ip prefix-list p1 seq 5 permit 172.16.255.31/32 +ip prefix-list p2 seq 5 permit 172.16.255.32/32 +ip prefix-list p3 seq 5 permit 172.16.255.30/32 ! +bgp as-path access-list FIRST permit ^65 +bgp as-path access-list SECOND permit 2$ + +route-map r2 permit 6 + match ip address prefix-list p2 + set as-path exclude as-path-access-list SECOND route-map r2 permit 10 match ip address prefix-list p1 set as-path exclude 65003 route-map r2 permit 20 + match ip address prefix-list p3 set as-path exclude all ! diff --git a/tests/topotests/bgp_set_aspath_exclude/r3/zebra.conf b/tests/topotests/bgp_set_aspath_exclude/r3/zebra.conf index 3fa6c644844c..56893158a403 100644 --- a/tests/topotests/bgp_set_aspath_exclude/r3/zebra.conf +++ b/tests/topotests/bgp_set_aspath_exclude/r3/zebra.conf @@ -1,5 +1,6 @@ ! int lo + ip address 172.16.255.30/32 ip address 172.16.255.31/32 ip address 172.16.255.32/32 ! diff --git a/tests/topotests/bgp_set_aspath_exclude/test_bgp_set_aspath_exclude.py b/tests/topotests/bgp_set_aspath_exclude/test_bgp_set_aspath_exclude.py index d373a749fe87..85e7b9676d0e 100644 --- a/tests/topotests/bgp_set_aspath_exclude/test_bgp_set_aspath_exclude.py +++ b/tests/topotests/bgp_set_aspath_exclude/test_bgp_set_aspath_exclude.py @@ -64,29 +64,33 @@ def teardown_module(mod): expected_1 = { "routes": { + "172.16.255.30/32": [{"path": ""}], "172.16.255.31/32": [{"path": "65002"}], - "172.16.255.32/32": [{"path": ""}], + "172.16.255.32/32": [{"path": "65003"}], } } expected_2 = { "routes": { - "172.16.255.31/32": [{"path": ""}], + "172.16.255.30/32": [{"path": ""}], + "172.16.255.31/32": [{"path": "65002"}], "172.16.255.32/32": [{"path": ""}], } } expected_3 = { "routes": { - "172.16.255.31/32": [{"path": "65003"}], - "172.16.255.32/32": [{"path": "65003"}], + "172.16.255.30/32": [{"path": ""}], + "172.16.255.31/32": [{"path": "65002"}], + "172.16.255.32/32": [{"path": "65002 65003"}], } } expected_4 = { "routes": { - "172.16.255.31/32": [{"path": "65002 65003"}], - "172.16.255.32/32": [{"path": "65002 65003"}], + "172.16.255.30/32": [{"path": ""}], + "172.16.255.31/32": [{"path": "65002"}], + "172.16.255.32/32": [{"path": "65002"}], } } @@ -117,34 +121,42 @@ def test_bgp_set_aspath_exclude_access_list(): rname = "r1" r1 = tgen.gears[rname] + # tgen.mininet_cli() r1.vtysh_cmd( """ conf bgp as-path access-list FIRST permit ^65 route-map r2 permit 6 + no set as-path exclude as-path-access-list SECOND set as-path exclude as-path-access-list FIRST """ ) + # tgen.mininet_cli() + r1.vtysh_cmd( + """ +clear bgp * + """ + ) test_func = functools.partial(bgp_converge, tgen.gears["r1"], expected_2) _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) - assert result is None, "Failed overriding incoming AS-PATH with regex 1 route-map" + assert result is None, "Failed change of exclude rule in route map" r1.vtysh_cmd( """ conf - bgp as-path access-list SECOND permit 2 route-map r2 permit 6 + no set as-path exclude as-path-access-list FIRST set as-path exclude as-path-access-list SECOND """ ) # tgen.mininet_cli() - test_func = functools.partial(bgp_converge, tgen.gears["r1"], expected_3) + test_func = functools.partial(bgp_converge, tgen.gears["r1"], expected_1) _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) - assert result is None, "Failed overriding incoming AS-PATH with regex 2 route-map" + assert result is None, "Failed reverting exclude rule in route map" def test_no_bgp_set_aspath_exclude_access_list(): @@ -159,15 +171,28 @@ def test_no_bgp_set_aspath_exclude_access_list(): r1.vtysh_cmd( """ conf - no bgp as-path access-list SECOND permit 2 + no bgp as-path access-list SECOND permit 2$ + """ + ) + + r1.vtysh_cmd( + """ +clear bgp * """ ) test_func = functools.partial(bgp_converge, tgen.gears["r1"], expected_3) _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) - assert result is None, "Failed removing bgp as-path access-list" + assert result is None, "Failed to removing current accesslist" + # tgen.mininet_cli() + r1.vtysh_cmd( + """ +conf + bgp as-path access-list SECOND permit 3$ + """ + ) r1.vtysh_cmd( """ clear bgp * @@ -177,7 +202,26 @@ def test_no_bgp_set_aspath_exclude_access_list(): test_func = functools.partial(bgp_converge, tgen.gears["r1"], expected_4) _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) - assert result is None, "Failed to renegotiate with peers" + assert result is None, "Failed to renegotiate with peers 2" + + r1.vtysh_cmd( + """ +conf + route-map r2 permit 6 + no set as-path exclude as-path-access-list SECOND + """ + ) + + r1.vtysh_cmd( + """ +clear bgp * + """ + ) + + test_func = functools.partial(bgp_converge, tgen.gears["r1"], expected_3) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) + + assert result is None, "Failed to renegotiate with peers 2" if __name__ == "__main__":