From 2c722516c3b8cf3fe63853ed8cae2d518ec62f59 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 23 Feb 2023 22:54:12 +0200 Subject: [PATCH 1/4] 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 --- 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 8ef18a34d0..928a41e9ed 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -17154,7 +17154,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 0fd0dafa22..26da5d3e26 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -977,9 +977,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 2a7c7a3143..44da63bd54 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -2247,7 +2247,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 d782e3ffa24832b05aec2871332a3a8523ab3e97 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 23 Feb 2023 23:02:35 +0200 Subject: [PATCH 2/4] 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 --- 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 a32cdb1ba4..277492157f 100644 --- a/bgpd/bgp_updgrp.c +++ b/bgpd/bgp_updgrp.c @@ -317,7 +317,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; @@ -501,8 +501,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 928a41e9ed..cf9a05b198 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -5671,7 +5671,7 @@ DEFPY(neighbor_capability_software_version, } 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; @@ -5690,13 +5690,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 26da5d3e26..f97004dc12 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -2726,7 +2726,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); @@ -4518,7 +4518,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 47017b846f93bb17abee61262eeb02ca6ab570f5 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 23 Feb 2023 23:51:10 +0200 Subject: [PATCH 3/4] bgpd: Renumber peer->af_flags to be without any gaps Signed-off-by: Donatas Abraitis --- bgpd/bgpd.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 44da63bd54..bfb736e86f 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1441,14 +1441,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 5acfd822be556d2123cf8c0c9d36aa20a48b3329 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 23 Feb 2023 23:10:26 +0200 Subject: [PATCH 4/4] tests: Check if peer->af_flags can be higher than uint32_t Signed-off-by: Donatas Abraitis --- 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 bfb736e86f..c0dd8d5ef4 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1448,7 +1448,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 652aaa25d4..dde38c8693 100644 --- a/tests/bgpd/test_peer_attr.c +++ b/tests/bgpd/test_peer_attr.c @@ -639,6 +639,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")