bgpd: Fix graceful-restart for peer-groups

Slipped somehow that peer-groups with GR is just completely broken, but it was
working before.

Strikes again, that we MUST have more and more topotests.

Fixes: 15403f521a12b668e87ef8961c78e0ed97c6ff92 ("bgpd: Streamline GR config, act on change immediately")
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
(cherry picked from commit 0a85b1ba04f6463e300aa6d5e064a5e5d79bec53)

diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c
index 94fca23e1849aedfde1ef961c5ffcc489eb6bdbf..e5692b6b48656540733e0ef13954ef06ed662482 100644 (file)
--- 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 68ced14de23167e7d1b6a8826ca9b58093b5bdd3..f1363ee60dfd9b49d885f4c865cda1e576f5d249 100644 (file)
--- 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) {

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index d65aac672c1d10aa9a0058e8c555a5a9a32ff962..6a788d752af67ce57abde796ec5bc65941ab5a10 100644 (file)
--- 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. */