Merge pull request #15838 from fdumontet6WIND/fix_regexx_exclude

bgpd: fix "bgp as-pah access-list" with "set aspath exclude" set/unset issue
This commit is contained in:
Russ White 2024-06-24 17:08:17 -04:00 committed by GitHub
commit 7a87166178
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 175 additions and 72 deletions

View File

@ -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 */

View File

@ -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);

View File

@ -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;

View File

@ -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;
};

View File

@ -2322,7 +2322,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";
@ -2334,44 +2334,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);
}
@ -2406,16 +2399,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)
else if (ase->exclude_aspath_acl)
path->attr->aspath =
aspath_filter_exclude_acl(new_path,
ase->exclude_aspath_acl);
}
return RMAP_OKAY;
}

View File

@ -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
!

View File

@ -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
!

View File

@ -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 @@ clear bgp *
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__":