summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--bgpd/bgp_open.c2
-rw-r--r--bgpd/bgp_vty.c10
-rw-r--r--bgpd/bgpd.c127
-rw-r--r--bgpd/bgpd.h7
-rw-r--r--tests/topotests/bgp_roles_capability/r1/bgpd.conf14
-rw-r--r--tests/topotests/bgp_roles_capability/r1/zebra.conf3
-rw-r--r--tests/topotests/bgp_roles_capability/r2/bgpd.conf2
-rw-r--r--tests/topotests/bgp_roles_capability/r3/bgpd.conf2
-rw-r--r--tests/topotests/bgp_roles_capability/r4/bgpd.conf2
-rw-r--r--tests/topotests/bgp_roles_capability/r5/bgpd.conf2
-rw-r--r--tests/topotests/bgp_roles_capability/r6/bgpd.conf4
-rw-r--r--tests/topotests/bgp_roles_capability/r6/zebra.conf6
-rw-r--r--tests/topotests/bgp_roles_capability/test_bgp_roles_capability.py20
13 files changed, 159 insertions, 42 deletions
diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c
index 28cacd6086..7248f034a5 100644
--- a/bgpd/bgp_open.c
+++ b/bgpd/bgp_open.c
@@ -1152,7 +1152,7 @@ static bool bgp_role_violation(struct peer *peer)
return true;
}
if (remote_role == ROLE_UNDEFINED &&
- CHECK_FLAG(peer->flags, PEER_FLAG_STRICT_MODE)) {
+ CHECK_FLAG(peer->flags, PEER_FLAG_ROLE_STRICT_MODE)) {
const char *err_msg =
"Strict mode. Please set the role on your side.";
bgp_notify_send_with_data(peer, BGP_NOTIFY_OPEN_ERR,
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index f8980ba102..1bff1c6b97 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -6449,7 +6449,7 @@ static int peer_role_set_vty(struct vty *vty, const char *ip_str,
{
struct peer *peer;
- peer = peer_lookup_vty(vty, ip_str);
+ peer = peer_and_group_lookup_vty(vty, ip_str);
if (!peer)
return CMD_WARNING_CONFIG_FAILED;
uint8_t role = get_role_by_name(role_str);
@@ -6463,7 +6463,7 @@ static int peer_role_unset_vty(struct vty *vty, const char *ip_str)
{
struct peer *peer;
- peer = peer_lookup_vty(vty, ip_str);
+ peer = peer_and_group_lookup_vty(vty, ip_str);
if (!peer)
return CMD_WARNING_CONFIG_FAILED;
return bgp_vty_return(vty, peer_role_unset(peer));
@@ -16785,13 +16785,13 @@ static void bgp_config_write_peer_global(struct vty *vty, struct bgp *bgp,
}
/* role */
- if (peer->local_role != ROLE_UNDEFINED) {
+ if (peergroup_flag_check(peer, PEER_FLAG_ROLE) &&
+ peer->local_role != ROLE_UNDEFINED)
vty_out(vty, " neighbor %s local-role %s%s\n", addr,
bgp_get_name_by_role(peer->local_role),
- CHECK_FLAG(peer->flags, PEER_FLAG_STRICT_MODE)
+ CHECK_FLAG(peer->flags, PEER_FLAG_ROLE_STRICT_MODE)
? " strict-mode"
: "");
- }
/* ttl-security hops */
if (peer->gtsm_hops != BGP_GTSM_HOPS_DISABLED) {
diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 8ece201198..b6f2a294a6 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -2731,6 +2731,9 @@ static void peer_group2peer_config_copy(struct peer_group *group,
}
}
+ /* role */
+ PEER_ATTR_INHERIT(peer, group, local_role);
+
/* Update GR flags for the peer. */
bgp_peer_gr_flags_update(peer);
@@ -4256,7 +4259,8 @@ static const struct peer_flag_action peer_flag_action_list[] = {
{PEER_FLAG_UPDATE_SOURCE, 0, peer_change_none},
{PEER_FLAG_DISABLE_LINK_BW_ENCODING_IEEE, 0, peer_change_none},
{PEER_FLAG_EXTENDED_OPT_PARAMS, 0, peer_change_reset},
- {PEER_FLAG_STRICT_MODE, 0, peer_change_reset},
+ {PEER_FLAG_ROLE_STRICT_MODE, 0, peer_change_reset},
+ {PEER_FLAG_ROLE, 0, peer_change_reset},
{0, 0, 0}};
static const struct peer_flag_action peer_af_flag_action_list[] = {
@@ -4929,36 +4933,109 @@ int peer_ebgp_multihop_unset(struct peer *peer)
/* Set Open Policy Role and check its correctness */
int peer_role_set(struct peer *peer, uint8_t role, bool strict_mode)
{
- if (peer->sort != BGP_PEER_EBGP)
- return BGP_ERR_INVALID_INTERNAL_ROLE;
-
- if (peer->local_role == role) {
- if (CHECK_FLAG(peer->flags, PEER_FLAG_STRICT_MODE) &&
- !strict_mode)
- /* TODO: Is session restart needed if it was down? */
- UNSET_FLAG(peer->flags, PEER_FLAG_STRICT_MODE);
- if (!CHECK_FLAG(peer->flags, PEER_FLAG_STRICT_MODE) &&
- strict_mode) {
- SET_FLAG(peer->flags, PEER_FLAG_STRICT_MODE);
- /* Restart session to throw Role Mismatch Notification
- */
- if (peer->remote_role == ROLE_UNDEFINED)
- bgp_session_reset(peer);
+ struct peer *member;
+ struct listnode *node, *nnode;
+
+ peer_flag_set(peer, PEER_FLAG_ROLE);
+
+ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) {
+ if (peer->sort != BGP_PEER_EBGP)
+ return BGP_ERR_INVALID_INTERNAL_ROLE;
+
+ if (peer->local_role == role) {
+ if (CHECK_FLAG(peer->flags,
+ PEER_FLAG_ROLE_STRICT_MODE) &&
+ !strict_mode)
+ /* TODO: Is session restart needed if it was
+ * down?
+ */
+ UNSET_FLAG(peer->flags,
+ PEER_FLAG_ROLE_STRICT_MODE);
+ if (!CHECK_FLAG(peer->flags,
+ PEER_FLAG_ROLE_STRICT_MODE) &&
+ strict_mode) {
+ SET_FLAG(peer->flags,
+ PEER_FLAG_ROLE_STRICT_MODE);
+ /* Restart session to throw Role Mismatch
+ * Notification
+ */
+ if (peer->remote_role == ROLE_UNDEFINED)
+ bgp_session_reset(peer);
+ }
+ } else {
+ peer->local_role = role;
+ if (strict_mode)
+ SET_FLAG(peer->flags,
+ PEER_FLAG_ROLE_STRICT_MODE);
+ else
+ UNSET_FLAG(peer->flags,
+ PEER_FLAG_ROLE_STRICT_MODE);
+ bgp_session_reset(peer);
}
- } else {
- peer->local_role = role;
- if (strict_mode)
- SET_FLAG(peer->flags, PEER_FLAG_STRICT_MODE);
- else
- UNSET_FLAG(peer->flags, PEER_FLAG_STRICT_MODE);
- bgp_session_reset(peer);
+
+ return CMD_SUCCESS;
}
- return 0;
+
+ peer->local_role = role;
+ for (ALL_LIST_ELEMENTS(peer->group->peer, node, nnode, member)) {
+ if (member->sort != BGP_PEER_EBGP)
+ return BGP_ERR_INVALID_INTERNAL_ROLE;
+
+ if (member->local_role == role) {
+ if (CHECK_FLAG(member->flags,
+ PEER_FLAG_ROLE_STRICT_MODE) &&
+ !strict_mode)
+ /* TODO: Is session restart needed if it was
+ * down?
+ */
+ UNSET_FLAG(member->flags,
+ PEER_FLAG_ROLE_STRICT_MODE);
+ if (!CHECK_FLAG(member->flags,
+ PEER_FLAG_ROLE_STRICT_MODE) &&
+ strict_mode) {
+ SET_FLAG(peer->flags,
+ PEER_FLAG_ROLE_STRICT_MODE);
+ SET_FLAG(member->flags,
+ PEER_FLAG_ROLE_STRICT_MODE);
+ /* Restart session to throw Role Mismatch
+ * Notification
+ */
+ if (member->remote_role == ROLE_UNDEFINED)
+ bgp_session_reset(member);
+ }
+ } else {
+ member->local_role = role;
+
+ if (strict_mode) {
+ SET_FLAG(peer->flags,
+ PEER_FLAG_ROLE_STRICT_MODE);
+ SET_FLAG(member->flags,
+ PEER_FLAG_ROLE_STRICT_MODE);
+ } else {
+ UNSET_FLAG(member->flags,
+ PEER_FLAG_ROLE_STRICT_MODE);
+ }
+ bgp_session_reset(member);
+ }
+ }
+
+ return CMD_SUCCESS;
}
int peer_role_unset(struct peer *peer)
{
- return peer_role_set(peer, ROLE_UNDEFINED, 0);
+ struct peer *member;
+ struct listnode *node, *nnode;
+
+ peer_flag_unset(peer, PEER_FLAG_ROLE);
+
+ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP))
+ return peer_role_set(peer, ROLE_UNDEFINED, 0);
+
+ for (ALL_LIST_ELEMENTS(peer->group->peer, node, nnode, member))
+ peer_role_set(member, ROLE_UNDEFINED, 0);
+
+ return CMD_SUCCESS;
}
/* Neighbor description. */
diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 6be05c180a..bc5f6cf6fd 100644
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -1337,9 +1337,12 @@ struct peer {
#define PEER_FLAG_EXTENDED_OPT_PARAMS (1ULL << 30)
/* BGP Open Policy flags.
- * Enforce using roles on both sides
+ * Enforce using roles on both sides:
+ * `local-role ROLE strict-mode` configured.
*/
-#define PEER_FLAG_STRICT_MODE (1ULL << 31)
+#define PEER_FLAG_ROLE_STRICT_MODE (1ULL << 31)
+ /* `local-role` configured */
+#define PEER_FLAG_ROLE (1ULL << 32)
/*
*GR-Disabled mode means unset PEER_FLAG_GRACEFUL_RESTART
diff --git a/tests/topotests/bgp_roles_capability/r1/bgpd.conf b/tests/topotests/bgp_roles_capability/r1/bgpd.conf
index 4ec83093dc..273f933d6e 100644
--- a/tests/topotests/bgp_roles_capability/r1/bgpd.conf
+++ b/tests/topotests/bgp_roles_capability/r1/bgpd.conf
@@ -2,18 +2,24 @@ router bgp 64501
bgp router-id 192.168.2.1
network 192.0.2.0/24
! Correct role pair
- neighbor 192.168.2.2 remote-as 64502
+ neighbor 192.168.2.2 remote-as external
neighbor 192.168.2.2 local-role provider
neighbor 192.168.2.2 timers 3 10
! Incorrect role pair
- neighbor 192.168.3.2 remote-as 64503
+ neighbor 192.168.3.2 remote-as external
neighbor 192.168.3.2 local-role provider
neighbor 192.168.3.2 timers 3 10
! Missed neighbor role
- neighbor 192.168.4.2 remote-as 64504
+ neighbor 192.168.4.2 remote-as external
neighbor 192.168.4.2 local-role provider
neighbor 192.168.4.2 timers 3 10
! Missed neighbor role with strict-mode
- neighbor 192.168.5.2 remote-as 64505
+ neighbor 192.168.5.2 remote-as external
neighbor 192.168.5.2 local-role provider strict-mode
neighbor 192.168.5.2 timers 3 10
+! Testing peer-groups
+ neighbor PG peer-group
+ neighbor PG remote-as external
+ neighbor PG local-role provider
+ neighbor PG timers 3 10
+ neighbor 192.168.6.2 peer-group PG
diff --git a/tests/topotests/bgp_roles_capability/r1/zebra.conf b/tests/topotests/bgp_roles_capability/r1/zebra.conf
index 3e90a261c1..301a1e8890 100644
--- a/tests/topotests/bgp_roles_capability/r1/zebra.conf
+++ b/tests/topotests/bgp_roles_capability/r1/zebra.conf
@@ -11,5 +11,8 @@ interface r1-eth2
interface r1-eth3
ip address 192.168.5.1/24
!
+interface r1-eth4
+ ip address 192.168.6.1/24
+!
ip forwarding
!
diff --git a/tests/topotests/bgp_roles_capability/r2/bgpd.conf b/tests/topotests/bgp_roles_capability/r2/bgpd.conf
index e741aa16ce..aa8ba72cad 100644
--- a/tests/topotests/bgp_roles_capability/r2/bgpd.conf
+++ b/tests/topotests/bgp_roles_capability/r2/bgpd.conf
@@ -1,5 +1,5 @@
router bgp 64502
bgp router-id 192.168.2.2
- neighbor 192.168.2.1 remote-as 64501
+ neighbor 192.168.2.1 remote-as external
neighbor 192.168.2.1 local-role customer
neighbor 192.168.2.1 timers 3 10
diff --git a/tests/topotests/bgp_roles_capability/r3/bgpd.conf b/tests/topotests/bgp_roles_capability/r3/bgpd.conf
index d1404260e6..5ed93d63e5 100644
--- a/tests/topotests/bgp_roles_capability/r3/bgpd.conf
+++ b/tests/topotests/bgp_roles_capability/r3/bgpd.conf
@@ -1,4 +1,4 @@
router bgp 64503
- neighbor 192.168.3.1 remote-as 64501
+ neighbor 192.168.3.1 remote-as external
neighbor 192.168.3.1 local-role peer
neighbor 192.168.3.1 timers 3 10
diff --git a/tests/topotests/bgp_roles_capability/r4/bgpd.conf b/tests/topotests/bgp_roles_capability/r4/bgpd.conf
index e6a0a9222a..118132d490 100644
--- a/tests/topotests/bgp_roles_capability/r4/bgpd.conf
+++ b/tests/topotests/bgp_roles_capability/r4/bgpd.conf
@@ -1,3 +1,3 @@
router bgp 64504
- neighbor 192.168.4.1 remote-as 64501
+ neighbor 192.168.4.1 remote-as external
neighbor 192.168.4.1 timers 3 10
diff --git a/tests/topotests/bgp_roles_capability/r5/bgpd.conf b/tests/topotests/bgp_roles_capability/r5/bgpd.conf
index eeeed7ed30..2f62fbf11a 100644
--- a/tests/topotests/bgp_roles_capability/r5/bgpd.conf
+++ b/tests/topotests/bgp_roles_capability/r5/bgpd.conf
@@ -1,3 +1,3 @@
router bgp 64505
- neighbor 192.168.5.1 remote-as 64501
+ neighbor 192.168.5.1 remote-as external
neighbor 192.168.5.1 timers 3 10
diff --git a/tests/topotests/bgp_roles_capability/r6/bgpd.conf b/tests/topotests/bgp_roles_capability/r6/bgpd.conf
new file mode 100644
index 0000000000..44b12febf2
--- /dev/null
+++ b/tests/topotests/bgp_roles_capability/r6/bgpd.conf
@@ -0,0 +1,4 @@
+router bgp 64506
+ neighbor 192.168.6.1 remote-as external
+ neighbor 192.168.6.1 local-role customer
+ neighbor 192.168.6.1 timers 3 10
diff --git a/tests/topotests/bgp_roles_capability/r6/zebra.conf b/tests/topotests/bgp_roles_capability/r6/zebra.conf
new file mode 100644
index 0000000000..c8428c4ba2
--- /dev/null
+++ b/tests/topotests/bgp_roles_capability/r6/zebra.conf
@@ -0,0 +1,6 @@
+!
+interface r6-eth0
+ ip address 192.168.6.2/24
+!
+ip forwarding
+!
diff --git a/tests/topotests/bgp_roles_capability/test_bgp_roles_capability.py b/tests/topotests/bgp_roles_capability/test_bgp_roles_capability.py
index f72b6d2cee..d72fd19b91 100644
--- a/tests/topotests/bgp_roles_capability/test_bgp_roles_capability.py
+++ b/tests/topotests/bgp_roles_capability/test_bgp_roles_capability.py
@@ -43,7 +43,7 @@ from lib.topolog import logger
pytestmark = [pytest.mark.bgpd]
-topodef = {f"s{i}": ("r1", f"r{i}") for i in range(2, 6)}
+topodef = {f"s{i}": ("r1", f"r{i}") for i in range(2, 7)}
@pytest.fixture(scope="module")
@@ -162,6 +162,24 @@ def test_role_strict_mode(tgen):
assert success, "Session between r1 and r5 was not correctly closed"
+def test_correct_pair_peer_group(tgen):
+ # provider-customer pair (using peer-groups)
+ router = tgen.gears["r1"]
+ neighbor_ip = "192.168.6.2"
+ check_r6_established = functools.partial(
+ check_session_established, router, neighbor_ip
+ )
+ success, _ = topotest.run_and_expect(check_r6_established, True, count=20, wait=3)
+ assert success, "Session with r6 is not Established"
+
+ neighbor_status = find_neighbor_status(router, neighbor_ip)
+ assert neighbor_status["localRole"] == "provider"
+ assert neighbor_status["remoteRole"] == "customer"
+ assert (
+ neighbor_status["neighborCapabilities"].get("role") == "advertisedAndReceived"
+ )
+
+
if __name__ == "__main__":
args = ["-s"] + sys.argv[1:]
sys.exit(pytest.main(args))