From 8587e5434edbb901d9f5d37e43c45a4fa5894ceb Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 23 Feb 2023 22:54:12 +0200 Subject: [PATCH 1/6] bgpd: Convert peer_af_flag_check() to bool Since we increased peer->af_flags from uint32_t to uint64_t, peer_af_flag_check() was historically returning integer, and not bool as should be. The bug was that if we have af_flags higher than uint32_t it will never returned a right value. Signed-off-by: Donatas Abraitis (cherry picked from commit 2c722516c3b8cf3fe63853ed8cae2d518ec62f59) --- bgpd/bgp_vty.c | 2 +- bgpd/bgpd.c | 5 +++-- bgpd/bgpd.h | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index d837601f32..3acd72f210 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -16912,7 +16912,7 @@ static bool peergroup_af_flag_check(struct peer *peer, afi_t afi, safi_t safi, if (CHECK_FLAG(peer->af_flags_invert[afi][safi], flag)) return !peer_af_flag_check(peer, afi, safi, flag); else - return !!peer_af_flag_check(peer, afi, safi, flag); + return peer_af_flag_check(peer, afi, safi, flag); } return !!CHECK_FLAG(peer->af_flags_override[afi][safi], flag); diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index a24c75d0a5..0610ff4f1c 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -980,9 +980,10 @@ void peer_flag_inherit(struct peer *peer, uint64_t flag) COND_FLAG(peer->flags, flag, group_val); } -int peer_af_flag_check(struct peer *peer, afi_t afi, safi_t safi, uint32_t flag) +bool peer_af_flag_check(struct peer *peer, afi_t afi, safi_t safi, + uint64_t flag) { - return CHECK_FLAG(peer->af_flags[afi][safi], flag); + return !!CHECK_FLAG(peer->af_flags[afi][safi], flag); } void peer_af_flag_inherit(struct peer *peer, afi_t afi, safi_t safi, diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 4c4c81f997..aff3a99bbc 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -2227,7 +2227,8 @@ extern int peer_af_flag_set(struct peer *peer, afi_t afi, safi_t safi, uint64_t flag); extern int peer_af_flag_unset(struct peer *peer, afi_t afi, safi_t safi, uint64_t flag); -extern int peer_af_flag_check(struct peer *, afi_t, safi_t, uint32_t); +extern bool peer_af_flag_check(struct peer *peer, afi_t afi, safi_t safi, + uint64_t flag); extern void peer_af_flag_inherit(struct peer *peer, afi_t afi, safi_t safi, uint64_t flag); extern void peer_change_action(struct peer *peer, afi_t afi, safi_t safi, From f6c5c830bc65554ad67db1b0077179384678cab6 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 23 Feb 2023 23:02:35 +0200 Subject: [PATCH 2/6] bgpd: Convert missing uint32_t to uint64_t for for af_flags/flags It was hard to catch those unless using higher values than uint32_t, but already hit, it's time to fix completely. Signed-off-by: Donatas Abraitis (cherry picked from commit d782e3ffa24832b05aec2871332a3a8523ab3e97) --- bgpd/bgp_updgrp.c | 6 +++--- bgpd/bgp_vty.c | 6 +++--- bgpd/bgpd.c | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/bgpd/bgp_updgrp.c b/bgpd/bgp_updgrp.c index 9ca57f08da..a91c9411fe 100644 --- a/bgpd/bgp_updgrp.c +++ b/bgpd/bgp_updgrp.c @@ -332,7 +332,7 @@ static unsigned int updgrp_hash_key_make(const void *p) const struct update_group *updgrp; const struct peer *peer; const struct bgp_filter *filter; - uint32_t flags; + uint64_t flags; uint32_t key; afi_t afi; safi_t safi; @@ -516,8 +516,8 @@ static bool updgrp_hash_cmp(const void *p1, const void *p2) const struct update_group *grp2; const struct peer *pe1; const struct peer *pe2; - uint32_t flags1; - uint32_t flags2; + uint64_t flags1; + uint64_t flags2; const struct bgp_filter *fl1; const struct bgp_filter *fl2; afi_t afi; diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 3acd72f210..4ec0876411 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -5555,7 +5555,7 @@ DEFUN (no_neighbor_capability_enhe, } static int peer_af_flag_modify_vty(struct vty *vty, const char *peer_str, - afi_t afi, safi_t safi, uint32_t flag, + afi_t afi, safi_t safi, uint64_t flag, int set) { int ret; @@ -5574,13 +5574,13 @@ static int peer_af_flag_modify_vty(struct vty *vty, const char *peer_str, } static int peer_af_flag_set_vty(struct vty *vty, const char *peer_str, - afi_t afi, safi_t safi, uint32_t flag) + afi_t afi, safi_t safi, uint64_t flag) { return peer_af_flag_modify_vty(vty, peer_str, afi, safi, flag, 1); } static int peer_af_flag_unset_vty(struct vty *vty, const char *peer_str, - afi_t afi, safi_t safi, uint32_t flag) + afi_t afi, safi_t safi, uint64_t flag) { return peer_af_flag_modify_vty(vty, peer_str, afi, safi, flag, 0); } diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 0610ff4f1c..1abd8e9a87 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -2704,7 +2704,7 @@ struct peer_group *peer_group_get(struct bgp *bgp, const char *name) static void peer_group2peer_config_copy(struct peer_group *group, struct peer *peer) { - uint32_t flags_tmp; + uint64_t flags_tmp; struct peer *conf; bool config_node = !!CHECK_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE); @@ -4449,7 +4449,7 @@ static int peer_flag_action_set(const struct peer_flag_action *action_list, return found; } -static void peer_flag_modify_action(struct peer *peer, uint32_t flag) +static void peer_flag_modify_action(struct peer *peer, uint64_t flag) { if (flag == PEER_FLAG_SHUTDOWN) { if (CHECK_FLAG(peer->flags, flag)) { From c95bfed068b60c24ccc143c887bda971901056e2 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 23 Feb 2023 23:51:10 +0200 Subject: [PATCH 3/6] bgpd: Renumber peer->af_flags to be without any gaps Signed-off-by: Donatas Abraitis (cherry picked from commit 47017b846f93bb17abee61262eeb02ca6ab570f5) --- bgpd/bgpd.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index aff3a99bbc..682a410c58 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1435,14 +1435,14 @@ struct peer { #define PEER_FLAG_REMOVE_PRIVATE_AS_REPLACE (1ULL << 19) #define PEER_FLAG_AS_OVERRIDE (1ULL << 20) #define PEER_FLAG_REMOVE_PRIVATE_AS_ALL_REPLACE (1ULL << 21) -#define PEER_FLAG_WEIGHT (1ULL << 24) -#define PEER_FLAG_ALLOWAS_IN_ORIGIN (1ULL << 25) -#define PEER_FLAG_SEND_LARGE_COMMUNITY (1ULL << 26) -#define PEER_FLAG_MAX_PREFIX_OUT (1ULL << 27) -#define PEER_FLAG_MAX_PREFIX_FORCE (1ULL << 28) -#define PEER_FLAG_DISABLE_ADDPATH_RX (1ULL << 29) -#define PEER_FLAG_SOO (1ULL << 30) -#define PEER_FLAG_ACCEPT_OWN (1ULL << 31) +#define PEER_FLAG_WEIGHT (1ULL << 22) +#define PEER_FLAG_ALLOWAS_IN_ORIGIN (1ULL << 23) +#define PEER_FLAG_SEND_LARGE_COMMUNITY (1ULL << 24) +#define PEER_FLAG_MAX_PREFIX_OUT (1ULL << 25) +#define PEER_FLAG_MAX_PREFIX_FORCE (1ULL << 26) +#define PEER_FLAG_DISABLE_ADDPATH_RX (1ULL << 27) +#define PEER_FLAG_SOO (1ULL << 28) +#define PEER_FLAG_ACCEPT_OWN (1ULL << 29) enum bgp_addpath_strat addpath_type[AFI_MAX][SAFI_MAX]; From b0d906ea153dd5481d69cfc912538720506f82db Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 23 Feb 2023 23:10:26 +0200 Subject: [PATCH 4/6] tests: Check if peer->af_flags can be higher than uint32_t Signed-off-by: Donatas Abraitis (cherry picked from commit 5acfd822be556d2123cf8c0c9d36aa20a48b3329) --- bgpd/bgpd.h | 2 +- tests/bgpd/test_peer_attr.c | 8 ++++++++ tests/bgpd/test_peer_attr.py | 2 ++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 682a410c58..72b5b50fb4 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1442,7 +1442,7 @@ struct peer { #define PEER_FLAG_MAX_PREFIX_FORCE (1ULL << 26) #define PEER_FLAG_DISABLE_ADDPATH_RX (1ULL << 27) #define PEER_FLAG_SOO (1ULL << 28) -#define PEER_FLAG_ACCEPT_OWN (1ULL << 29) +#define PEER_FLAG_ACCEPT_OWN (1ULL << 63) enum bgp_addpath_strat addpath_type[AFI_MAX][SAFI_MAX]; diff --git a/tests/bgpd/test_peer_attr.c b/tests/bgpd/test_peer_attr.c index cc4f71e688..e4f1e73043 100644 --- a/tests/bgpd/test_peer_attr.c +++ b/tests/bgpd/test_peer_attr.c @@ -640,6 +640,14 @@ static struct test_peer_attr test_peer_attrs[] = { .u.flag = PEER_FLAG_WEIGHT, .handlers[0] = TEST_HANDLER(weight), }, + { + .cmd = "accept-own", + .peer_cmd = "accept-own", + .group_cmd = "accept-own", + .families[0] = {.afi = AFI_IP, .safi = SAFI_MPLS_VPN}, + .families[1] = {.afi = AFI_IP6, .safi = SAFI_MPLS_VPN}, + .u.flag = PEER_FLAG_ACCEPT_OWN, + }, {NULL} }; /* clang-format on */ diff --git a/tests/bgpd/test_peer_attr.py b/tests/bgpd/test_peer_attr.py index 16b441b25d..eb57618434 100644 --- a/tests/bgpd/test_peer_attr.py +++ b/tests/bgpd/test_peer_attr.py @@ -196,3 +196,5 @@ TestFlag.okfail("peer\\ipv4-unicast\\weight") TestFlag.okfail("peer\\ipv4-multicast\\weight") TestFlag.okfail("peer\\ipv6-unicast\\weight") TestFlag.okfail("peer\\ipv6-multicast\\weight") +TestFlag.okfail("peer\\ipv4-vpn\\accept-own") +TestFlag.okfail("peer\\ipv6-vpn\\accept-own") From 8575ad3a6a61e45fac992a15c74f69477a8bfa46 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Mon, 20 Feb 2023 17:59:09 +0200 Subject: [PATCH 5/6] tests: Cover all enum values for unit tests Signed-off-by: Donatas Abraitis --- tests/bgpd/test_peer_attr.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/tests/bgpd/test_peer_attr.c b/tests/bgpd/test_peer_attr.c index e4f1e73043..02cd282a70 100644 --- a/tests/bgpd/test_peer_attr.c +++ b/tests/bgpd/test_peer_attr.c @@ -659,21 +659,14 @@ static const char *str_from_afi(afi_t afi) return "ipv4"; case AFI_IP6: return "ipv6"; - default: - return ""; + case AFI_L2VPN: + return "l2vpn"; + case AFI_MAX: + case AFI_UNSPEC: + return "bad-value"; } -} -static const char *str_from_safi(safi_t safi) -{ - switch (safi) { - case SAFI_UNICAST: - return "unicast"; - case SAFI_MULTICAST: - return "multicast"; - default: - return ""; - } + assert(!"Reached end of function we should never reach"); } static const char *str_from_attr_type(enum test_peer_attr_type at) @@ -1159,7 +1152,7 @@ static void test_peer_attr(struct test *test, struct test_peer_attr *pa) test_log(test, "prepare: switch address-family to [%s]", get_afi_safi_str(pa->afi, pa->safi, false)); test_execute(test, "address-family %s %s", - str_from_afi(pa->afi), str_from_safi(pa->safi)); + str_from_afi(pa->afi), safi2str(pa->safi)); test_execute(test, "neighbor %s activate", g->name); test_execute(test, "neighbor %s activate", p->host); } @@ -1226,7 +1219,7 @@ static void test_peer_attr(struct test *test, struct test_peer_attr *pa) test_log(test, "prepare: switch address-family to [%s]", get_afi_safi_str(pa->afi, pa->safi, false)); test_execute(test, "address-family %s %s", - str_from_afi(pa->afi), str_from_safi(pa->safi)); + str_from_afi(pa->afi), safi2str(pa->safi)); test_execute(test, "neighbor %s activate", g->name); test_execute(test, "neighbor %s activate", p->host); } @@ -1274,7 +1267,7 @@ static void test_peer_attr(struct test *test, struct test_peer_attr *pa) test_log(test, "prepare: switch address-family to [%s]", get_afi_safi_str(pa->afi, pa->safi, false)); test_execute(test, "address-family %s %s", - str_from_afi(pa->afi), str_from_safi(pa->safi)); + str_from_afi(pa->afi), safi2str(pa->safi)); test_execute(test, "neighbor %s activate", g->name); test_execute(test, "neighbor %s activate", p->host); } @@ -1473,7 +1466,7 @@ int main(void) if (pa->afi && pa->safi) desc = asprintfrr(MTYPE_TMP, "peer\\%s-%s\\%s", str_from_afi(pa->afi), - str_from_safi(pa->safi), pa->cmd); + safi2str(pa->safi), pa->cmd); else desc = asprintfrr(MTYPE_TMP, "peer\\%s", pa->cmd); From 982097c360a85183f4527206e918cb221b69bd28 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 27 Jan 2023 13:49:16 +0200 Subject: [PATCH 6/6] tests: Increase flags from uint32_t to uint64_t Missed this part when increasing in the past. Signed-off-by: Donatas Abraitis --- tests/bgpd/test_peer_attr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bgpd/test_peer_attr.c b/tests/bgpd/test_peer_attr.c index 02cd282a70..3e25faacb5 100644 --- a/tests/bgpd/test_peer_attr.c +++ b/tests/bgpd/test_peer_attr.c @@ -176,9 +176,9 @@ struct test_peer_attr { enum test_peer_attr_type type; union { - uint32_t flag; + uint64_t flag; struct { - uint32_t flag; + uint64_t flag; size_t direct; } filter; } u;