diff options
| -rw-r--r-- | bgpd/bgp_fsm.c | 96 | ||||
| -rw-r--r-- | bgpd/bgp_vty.c | 49 | ||||
| -rw-r--r-- | bgpd/bgpd.c | 1 | ||||
| -rw-r--r-- | tests/topotests/bgp_peer_group/r1/bgpd.conf | 12 | ||||
| -rw-r--r-- | tests/topotests/bgp_peer_group/r1/frr.conf | 21 | ||||
| -rw-r--r-- | tests/topotests/bgp_peer_group/r1/zebra.conf | 9 | ||||
| -rw-r--r-- | tests/topotests/bgp_peer_group/r2/bgpd.conf | 11 | ||||
| -rw-r--r-- | tests/topotests/bgp_peer_group/r2/frr.conf | 19 | ||||
| -rw-r--r-- | tests/topotests/bgp_peer_group/r2/zebra.conf | 9 | ||||
| -rw-r--r-- | tests/topotests/bgp_peer_group/r3/frr.conf (renamed from tests/topotests/bgp_peer_group/r3/bgpd.conf) | 5 | ||||
| -rw-r--r-- | tests/topotests/bgp_peer_group/r3/zebra.conf | 6 | ||||
| -rw-r--r-- | tests/topotests/bgp_peer_group/test_bgp_peer-group.py | 41 |
12 files changed, 151 insertions, 128 deletions
diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 94fca23e18..e5692b6b48 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -2724,33 +2724,55 @@ static void bgp_gr_update_mode_of_all_peers(struct bgp *bgp, struct listnode *node = {0}; struct listnode *nnode = {0}; enum peer_mode peer_old_state = PEER_INVALID; - - /* TODO: Need to handle peer-groups. */ + struct peer_group *group; + struct peer *member; for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { - peer_old_state = bgp_peer_gr_mode_get(peer); - if (peer_old_state != PEER_GLOBAL_INHERIT) - continue; + if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + peer_old_state = bgp_peer_gr_mode_get(peer); + if (peer_old_state != PEER_GLOBAL_INHERIT) + continue; - bgp_peer_inherit_global_gr_mode(peer, global_new_state); - bgp_peer_gr_flags_update(peer); + bgp_peer_inherit_global_gr_mode(peer, global_new_state); + bgp_peer_gr_flags_update(peer); - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("%pBP: Inherited Global GR mode, GR flags 0x%x peer flags 0x%" PRIx64 - "...resetting session", - peer, peer->peer_gr_new_status_flag, - peer->flags); + if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) + zlog_debug("%pBP: Inherited Global GR mode, GR flags 0x%x peer flags 0x%" PRIx64 + "...resetting session", + peer, peer->peer_gr_new_status_flag, peer->flags); - peer->last_reset = PEER_DOWN_CAPABILITY_CHANGE; + peer->last_reset = PEER_DOWN_CAPABILITY_CHANGE; - /* Reset session to match with behavior for other peer - * configs that require the session to be re-setup. - */ - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); - else - bgp_session_reset_safe(peer, &nnode); + if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) + bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_CONFIG_CHANGE); + else + bgp_session_reset_safe(peer, &nnode); + } else { + group = peer->group; + for (ALL_LIST_ELEMENTS(group->peer, node, nnode, member)) { + peer_old_state = bgp_peer_gr_mode_get(member); + if (peer_old_state != PEER_GLOBAL_INHERIT) + continue; + + bgp_peer_inherit_global_gr_mode(member, global_new_state); + bgp_peer_gr_flags_update(member); + + if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) + zlog_debug("%pBP: Inherited Global GR mode, GR flags 0x%x peer flags 0x%" PRIx64 + "...resetting session", + member, member->peer_gr_new_status_flag, + member->flags); + + member->last_reset = PEER_DOWN_CAPABILITY_CHANGE; + + if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) + bgp_notify_send(member->connection, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_CONFIG_CHANGE); + else + bgp_session_reset(member); + } + } } } @@ -2936,6 +2958,9 @@ unsigned int bgp_peer_gr_action(struct peer *peer, enum peer_mode old_state, { enum global_mode global_gr_mode; bool session_reset = true; + struct peer_group *group; + struct peer *member; + struct listnode *node, *nnode; if (old_state == new_state) return BGP_GR_NO_OPERATION; @@ -2970,16 +2995,27 @@ unsigned int bgp_peer_gr_action(struct peer *peer, enum peer_mode old_state, bgp_peer_move_to_gr_mode(peer, new_state); if (session_reset) { - peer->last_reset = PEER_DOWN_CAPABILITY_CHANGE; + if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + peer->last_reset = PEER_DOWN_CAPABILITY_CHANGE; - /* Reset session to match with behavior for other peer - * configs that require the session to be re-setup. - */ - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); - else - bgp_session_reset(peer); + if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) + bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_CONFIG_CHANGE); + else + bgp_session_reset(peer); + } else { + group = peer->group; + for (ALL_LIST_ELEMENTS(group->peer, node, nnode, member)) { + member->last_reset = PEER_DOWN_CAPABILITY_CHANGE; + bgp_peer_move_to_gr_mode(member, new_state); + + if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) + bgp_notify_send(member->connection, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_CONFIG_CHANGE); + else + bgp_session_reset(member); + } + } } return BGP_GR_SUCCESS; diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 68ced14de2..c153d38386 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -3517,11 +3517,6 @@ DEFUN (bgp_neighbor_graceful_restart_set, peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); if (!peer) return CMD_WARNING_CONFIG_FAILED; - if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { - vty_out(vty, - "Per peer-group graceful-restart configuration is not yet supported\n"); - return CMD_WARNING_CONFIG_FAILED; - } result = bgp_neighbor_graceful_restart(peer, PEER_GR_CMD); if (result == BGP_GR_SUCCESS) { @@ -3552,11 +3547,6 @@ DEFUN (no_bgp_neighbor_graceful_restart, peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); if (!peer) return CMD_WARNING_CONFIG_FAILED; - if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { - vty_out(vty, - "Per peer-group graceful-restart configuration is not yet supported\n"); - return CMD_WARNING_CONFIG_FAILED; - } result = bgp_neighbor_graceful_restart(peer, NO_PEER_GR_CMD); if (ret == BGP_GR_SUCCESS) { @@ -3586,11 +3576,6 @@ DEFUN (bgp_neighbor_graceful_restart_helper_set, peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); if (!peer) return CMD_WARNING_CONFIG_FAILED; - if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { - vty_out(vty, - "Per peer-group graceful-restart configuration is not yet supported\n"); - return CMD_WARNING_CONFIG_FAILED; - } ret = bgp_neighbor_graceful_restart(peer, PEER_HELPER_CMD); if (ret == BGP_GR_SUCCESS) { @@ -3621,11 +3606,6 @@ DEFUN (no_bgp_neighbor_graceful_restart_helper, peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); if (!peer) return CMD_WARNING_CONFIG_FAILED; - if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { - vty_out(vty, - "Per peer-group graceful-restart configuration is not yet supported\n"); - return CMD_WARNING_CONFIG_FAILED; - } ret = bgp_neighbor_graceful_restart(peer, NO_PEER_HELPER_CMD); if (ret == BGP_GR_SUCCESS) { @@ -3655,11 +3635,6 @@ DEFUN (bgp_neighbor_graceful_restart_disable_set, peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); if (!peer) return CMD_WARNING_CONFIG_FAILED; - if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { - vty_out(vty, - "Per peer-group graceful-restart configuration is not yet supported\n"); - return CMD_WARNING_CONFIG_FAILED; - } ret = bgp_neighbor_graceful_restart(peer, PEER_DISABLE_CMD); if (ret == BGP_GR_SUCCESS) { @@ -3691,11 +3666,6 @@ DEFUN (no_bgp_neighbor_graceful_restart_disable, peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); if (!peer) return CMD_WARNING_CONFIG_FAILED; - if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { - vty_out(vty, - "Per peer-group graceful-restart configuration is not yet supported\n"); - return CMD_WARNING_CONFIG_FAILED; - } ret = bgp_neighbor_graceful_restart(peer, NO_PEER_DISABLE_CMD); if (ret == BGP_GR_SUCCESS) { @@ -14935,22 +14905,31 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, bool use_json, if (CHECK_FLAG(p->cap, PEER_CAP_RESTART_RCV) || CHECK_FLAG(p->cap, PEER_CAP_RESTART_ADV)) { if (CHECK_FLAG(p->cap, PEER_CAP_RESTART_ADV) && - CHECK_FLAG(p->cap, PEER_CAP_RESTART_RCV)) + CHECK_FLAG(p->cap, PEER_CAP_RESTART_RCV)) { json_object_string_add( json_cap, "gracefulRestart", "advertisedAndReceived"); - else if (CHECK_FLAG(p->cap, - PEER_CAP_RESTART_ADV)) + } else if (CHECK_FLAG(p->cap, PEER_CAP_RESTART_ADV)) { + json_object_string_add(json_cap, "gracefulRestart", + "advertised"); +#if CONFDATE > 20250525 +CPP_NOTICE("Remove `gracefulRestartCapability` JSON field") +#endif json_object_string_add( json_cap, "gracefulRestartCapability", "advertised"); - else if (CHECK_FLAG(p->cap, - PEER_CAP_RESTART_RCV)) + } else if (CHECK_FLAG(p->cap, PEER_CAP_RESTART_RCV)) { + json_object_string_add(json_cap, "gracefulRestart", + "received"); +#if CONFDATE > 20250525 +CPP_NOTICE("Remove `gracefulRestartCapability` JSON field") +#endif json_object_string_add( json_cap, "gracefulRestartCapability", "received"); + } if (CHECK_FLAG(p->cap, PEER_CAP_RESTART_RCV)) { int restart_af_count = 0; diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index d65aac672c..6a788d752a 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3027,6 +3027,7 @@ static void peer_group2peer_config_copy(struct peer_group *group, PEER_ATTR_INHERIT(peer, group, local_role); /* Update GR flags for the peer. */ + PEER_ATTR_INHERIT(peer, group, peer_gr_new_status_flag); bgp_peer_gr_flags_update(peer); /* Apply BFD settings from group to peer if it exists. */ diff --git a/tests/topotests/bgp_peer_group/r1/bgpd.conf b/tests/topotests/bgp_peer_group/r1/bgpd.conf deleted file mode 100644 index 68d8e61a59..0000000000 --- a/tests/topotests/bgp_peer_group/r1/bgpd.conf +++ /dev/null @@ -1,12 +0,0 @@ -! -router bgp 65001 - neighbor PG peer-group - neighbor PG remote-as external - neighbor PG timers 3 10 - neighbor 192.168.255.3 peer-group PG - neighbor r1-eth0 interface peer-group PG - neighbor PG1 peer-group - neighbor PG1 remote-as external - neighbor PG1 timers 3 20 - neighbor 192.168.251.2 peer-group PG1 -! diff --git a/tests/topotests/bgp_peer_group/r1/frr.conf b/tests/topotests/bgp_peer_group/r1/frr.conf new file mode 100644 index 0000000000..035c8e4cf1 --- /dev/null +++ b/tests/topotests/bgp_peer_group/r1/frr.conf @@ -0,0 +1,21 @@ +! +interface r1-eth0 + ip address 192.168.255.1/24 +! +interface r1-eth1 + ip address 192.168.251.1/30 +! +ip forwarding +! +router bgp 65001 + neighbor PG peer-group + neighbor PG remote-as external + neighbor PG timers 3 10 + neighbor 192.168.255.3 peer-group PG + neighbor r1-eth0 interface peer-group PG + neighbor PG1 peer-group + neighbor PG1 remote-as external + neighbor PG1 timers 3 20 + neighbor PG1 graceful-restart-disable + neighbor 192.168.251.2 peer-group PG1 +! diff --git a/tests/topotests/bgp_peer_group/r1/zebra.conf b/tests/topotests/bgp_peer_group/r1/zebra.conf deleted file mode 100644 index 16fd8c538c..0000000000 --- a/tests/topotests/bgp_peer_group/r1/zebra.conf +++ /dev/null @@ -1,9 +0,0 @@ -! -interface r1-eth0 - ip address 192.168.255.1/24 -! -interface r1-eth1 - ip address 192.168.251.1/30 -! -ip forwarding -! diff --git a/tests/topotests/bgp_peer_group/r2/bgpd.conf b/tests/topotests/bgp_peer_group/r2/bgpd.conf deleted file mode 100644 index d0e8f017d1..0000000000 --- a/tests/topotests/bgp_peer_group/r2/bgpd.conf +++ /dev/null @@ -1,11 +0,0 @@ -! -router bgp 65002 - neighbor PG peer-group - neighbor PG remote-as external - neighbor PG timers 3 10 - neighbor r2-eth0 interface peer-group PG - neighbor PG1 peer-group - neighbor PG1 remote-as external - neighbor PG1 timers 3 20 - neighbor 192.168.251.1 peer-group PG1 -! diff --git a/tests/topotests/bgp_peer_group/r2/frr.conf b/tests/topotests/bgp_peer_group/r2/frr.conf new file mode 100644 index 0000000000..4713789f15 --- /dev/null +++ b/tests/topotests/bgp_peer_group/r2/frr.conf @@ -0,0 +1,19 @@ +! +interface r2-eth0 + ip address 192.168.255.2/24 +! +interface r2-eth1 + ip address 192.168.251.2/30 +! +ip forwarding +! +router bgp 65002 + neighbor PG peer-group + neighbor PG remote-as external + neighbor PG timers 3 10 + neighbor r2-eth0 interface peer-group PG + neighbor PG1 peer-group + neighbor PG1 remote-as external + neighbor PG1 timers 3 20 + neighbor 192.168.251.1 peer-group PG1 +! diff --git a/tests/topotests/bgp_peer_group/r2/zebra.conf b/tests/topotests/bgp_peer_group/r2/zebra.conf deleted file mode 100644 index c2ad956c9c..0000000000 --- a/tests/topotests/bgp_peer_group/r2/zebra.conf +++ /dev/null @@ -1,9 +0,0 @@ -! -interface r2-eth0 - ip address 192.168.255.2/24 -! -interface r2-eth1 - ip address 192.168.251.2/30 -! -ip forwarding -! diff --git a/tests/topotests/bgp_peer_group/r3/bgpd.conf b/tests/topotests/bgp_peer_group/r3/frr.conf index 5a1340fb0b..e8bffaab51 100644 --- a/tests/topotests/bgp_peer_group/r3/bgpd.conf +++ b/tests/topotests/bgp_peer_group/r3/frr.conf @@ -1,4 +1,9 @@ ! +interface r3-eth0 + ip address 192.168.255.3/24 +! +ip forwarding +! router bgp 65003 no bgp ebgp-requires-policy neighbor PG peer-group diff --git a/tests/topotests/bgp_peer_group/r3/zebra.conf b/tests/topotests/bgp_peer_group/r3/zebra.conf deleted file mode 100644 index e9fdfb70c5..0000000000 --- a/tests/topotests/bgp_peer_group/r3/zebra.conf +++ /dev/null @@ -1,6 +0,0 @@ -! -interface r3-eth0 - ip address 192.168.255.3/24 -! -ip forwarding -! diff --git a/tests/topotests/bgp_peer_group/test_bgp_peer-group.py b/tests/topotests/bgp_peer_group/test_bgp_peer-group.py index 7d476b0538..45f713b8a2 100644 --- a/tests/topotests/bgp_peer_group/test_bgp_peer-group.py +++ b/tests/topotests/bgp_peer_group/test_bgp_peer-group.py @@ -2,12 +2,14 @@ # SPDX-License-Identifier: ISC # -# Copyright (c) 2021 by +# Copyright (c) 2021-2024 by # Donatas Abraitis <donatas.abraitis@gmail.com> +# Donatas Abraitis <donatas@opensourcerouting.org> # """ -Test if peer-group works for numbered and unnumbered configurations. +Test if various random settings with peer-group works for +numbered and unnumbered configurations. """ import os @@ -21,7 +23,7 @@ sys.path.append(os.path.join(CWD, "../")) # pylint: disable=C0413 from lib import topotest -from lib.topogen import Topogen, TopoRouter, get_topogen +from lib.topogen import Topogen, get_topogen from lib.topolog import logger pytestmark = [pytest.mark.bgpd] @@ -48,12 +50,7 @@ def setup_module(mod): router_list = tgen.routers() for _, (rname, router) in enumerate(router_list.items(), 1): - router.load_config( - TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname)) - ) - router.load_config( - TopoRouter.RD_BGP, os.path.join(CWD, "{}/bgpd.conf".format(rname)) - ) + router.load_frr_config(os.path.join(CWD, "{}/frr.conf".format(rname))) tgen.start_router() @@ -72,14 +69,26 @@ def test_bgp_peer_group(): def _bgp_peer_group_configured(): output = json.loads(tgen.gears["r1"].vtysh_cmd("show ip bgp neighbor json")) expected = { - "r1-eth0": {"peerGroup": "PG", "bgpState": "Established"}, - "192.168.255.3": {"peerGroup": "PG", "bgpState": "Established"}, - "192.168.251.2": {"peerGroup": "PG1", "bgpState": "Established"}, + "r1-eth0": { + "peerGroup": "PG", + "bgpState": "Established", + "neighborCapabilities": {"gracefulRestart": "advertisedAndReceived"}, + }, + "192.168.255.3": { + "peerGroup": "PG", + "bgpState": "Established", + "neighborCapabilities": {"gracefulRestart": "advertisedAndReceived"}, + }, + "192.168.251.2": { + "peerGroup": "PG1", + "bgpState": "Established", + "neighborCapabilities": {"gracefulRestart": "received"}, + }, } return topotest.json_cmp(output, expected) test_func = functools.partial(_bgp_peer_group_configured) - _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) assert result is None, "Failed bgp convergence in r1" def _bgp_peer_group_check_advertised_routes(): @@ -97,7 +106,7 @@ def test_bgp_peer_group(): return topotest.json_cmp(output, expected) test_func = functools.partial(_bgp_peer_group_check_advertised_routes) - _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) assert result is None, "Failed checking advertised routes from r3" @@ -122,7 +131,7 @@ def test_bgp_peer_group_remote_as_del_readd(): return topotest.json_cmp(output, expected) test_func = functools.partial(_bgp_peer_group_remoteas_del) - _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) assert result is None, "Failed bgp convergence in r1" logger.info("Re-add bgp peer-group PG1 remote-as neighbor should be established") @@ -139,7 +148,7 @@ def test_bgp_peer_group_remote_as_del_readd(): return topotest.json_cmp(output, expected) test_func = functools.partial(_bgp_peer_group_remoteas_add) - _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) assert result is None, "Failed bgp convergence in r1" |