Skip to content

Commit

Permalink
Merge pull request #15435 from FRRouting/mergify/bp/dev/10.0/pr-15387
Browse files Browse the repository at this point in the history 
bgpd: fix no bgp as-path access-list issue  (backport #15387)
  • Loading branch information
@ton31337
ton31337 authored Feb 27, 2024
2 parents 3497d85 + 8acb85b commit 1d64549
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 42 deletions.
8 changes: 8 additions & 0 deletions bgpd/bgp_aspath.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ struct aspath {

#define ASPATH_STR_DEFAULT_LEN 32

/* `set as-path exclude ASn' */
struct aspath_exclude {
struct aspath *aspath;
bool exclude_all;
char *exclude_aspath_acl_name;
struct as_list *exclude_aspath_acl;
};

/* Prototypes. */
extern void aspath_init(void);
extern void aspath_finish(void);
Expand Down
20 changes: 18 additions & 2 deletions bgpd/bgp_filter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,17 @@ static struct as_list *as_list_new(void)

static void as_list_free(struct as_list *aslist)
{
XFREE(MTYPE_AS_STR, aslist->name);
XFREE(MTYPE_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);
}

/* Insert new AS list to list of as_list. Each as_list is sorted by
Expand Down Expand Up @@ -290,6 +299,7 @@ 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;
Expand All @@ -308,6 +318,12 @@ 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);
}

Expand Down
6 changes: 6 additions & 0 deletions bgpd/bgp_filter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ struct as_filter {
int64_t seq;
};

struct aspath_exclude_list {
struct aspath_exclude_list *next;
struct aspath_exclude *bp_as_excl;
};

/* AS path filter list. */
struct as_list {
char *name;
Expand All @@ -34,6 +39,7 @@ struct as_list {

struct as_filter *head;
struct as_filter *tail;
struct aspath_exclude_list *exclude_list;
};


Expand Down
34 changes: 26 additions & 8 deletions bgpd/bgp_routemap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2323,17 +2323,10 @@ static const struct route_map_rule_cmd route_set_aspath_prepend_cmd = {
route_set_aspath_prepend_free,
};

/* `set as-path exclude ASn' */
struct aspath_exclude {
struct aspath *aspath;
bool exclude_all;
char *exclude_aspath_acl_name;
struct as_list *exclude_aspath_acl;
};

static void *route_aspath_exclude_compile(const char *arg)
{
struct aspath_exclude *ase;
struct aspath_exclude_list *ael;
const char *str = arg;
static const char asp_acl[] = "as-path-access-list";

Expand All @@ -2348,16 +2341,41 @@ static void *route_aspath_exclude_compile(const char *arg)
ase->exclude_aspath_acl = as_list_lookup(str);
} 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;

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

Expand Down
104 changes: 72 additions & 32 deletions tests/topotests/bgp_set_aspath_exclude/test_bgp_set_aspath_exclude.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,23 +62,48 @@ def teardown_module(mod):
tgen.stop_topology()


expected_1 = {
"routes": {
"172.16.255.31/32": [{"path": "65002"}],
"172.16.255.32/32": [{"path": ""}],
}
}

expected_2 = {
"routes": {
"172.16.255.31/32": [{"path": ""}],
"172.16.255.32/32": [{"path": ""}],
}
}

expected_3 = {
"routes": {
"172.16.255.31/32": [{"path": "65003"}],
"172.16.255.32/32": [{"path": "65003"}],
}
}

expected_4 = {
"routes": {
"172.16.255.31/32": [{"path": "65002 65003"}],
"172.16.255.32/32": [{"path": "65002 65003"}],
}
}


def bgp_converge(router, expected):
output = json.loads(router.vtysh_cmd("show bgp ipv4 unicast json"))

return topotest.json_cmp(output, expected)


def test_bgp_set_aspath_exclude():
tgen = get_topogen()

if tgen.routers_have_failure():
pytest.skip(tgen.errors)

def _bgp_converge(router):
output = json.loads(router.vtysh_cmd("show bgp ipv4 unicast json"))
expected = {
"routes": {
"172.16.255.31/32": [{"path": "65002"}],
"172.16.255.32/32": [{"path": ""}],
}
}
return topotest.json_cmp(output, expected)

test_func = functools.partial(_bgp_converge, tgen.gears["r1"])
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 route-map"
Expand All @@ -102,19 +127,7 @@ def test_bgp_set_aspath_exclude_access_list():
"""
)

expected = {
"routes": {
"172.16.255.31/32": [{"path": ""}],
"172.16.255.32/32": [{"path": ""}],
}
}

def _bgp_regexp_1(router):
output = json.loads(router.vtysh_cmd("show bgp ipv4 unicast json"))

return topotest.json_cmp(output, expected)

test_func = functools.partial(_bgp_regexp_1, tgen.gears["r1"])
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"
Expand All @@ -127,19 +140,46 @@ def _bgp_regexp_1(router):
"""
)

expected = {
"routes": {
"172.16.255.31/32": [{"path": "65003"}],
"172.16.255.32/32": [{"path": "65003"}],
}
}

test_func = functools.partial(_bgp_regexp_1, tgen.gears["r1"])
# tgen.mininet_cli()
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 overriding incoming AS-PATH with regex 2 route-map"


def test_no_bgp_set_aspath_exclude_access_list():
tgen = get_topogen()

if tgen.routers_have_failure():
pytest.skip(tgen.errors)

rname = "r1"
r1 = tgen.gears[rname]

r1.vtysh_cmd(
"""
conf
no bgp as-path access-list SECOND permit 2
"""
)

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"

r1.vtysh_cmd(
"""
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"


if __name__ == "__main__":
args = ["-s"] + sys.argv[1:]
sys.exit(pytest.main(args))

0 comments on commit 1d64549

Please sign in to comment.