bgpd: send notify in OpenSent when stopping manually
The issue it fixes is that the notification message is not sent to a
second peer when bgp is stopped manually.
According to BGP RFC4271, section 8.2.2, regarding the FSM transitions,
in OpenSent state:
If a ManualStop event (Event 2) is issued in the OpenSent state, the
local system:
* sends the NOTIFICATION with a Cease,
* sets the ConnectRetryTimer to zero,
* releases all BGP resources,
* drops the TCP connection,
* sets the ConnectRetryCounter to zero, and
* changes its state to Idle.
I've added a check for OpenSent state when the notification is sent from
the functions which are called from the CLI commands which
directly/indirectly stop/restart BGP.
Acked-by: Pradosh Mohapatra <pmohapat@cumulusnetworks.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 4b26993..5e2a5e1 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -175,7 +175,7 @@
{
IPV4_ADDR_COPY (&peer->local_id, id);
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_RID_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -205,7 +205,7 @@
if (peer->sort != BGP_PEER_IBGP)
continue;
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_CLID_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -233,7 +233,7 @@
if (peer->sort != BGP_PEER_IBGP)
continue;
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_CLID_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -302,7 +302,7 @@
if (peer_sort (peer) == BGP_PEER_EBGP)
{
peer->local_as = as;
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_CONFED_ID_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -322,7 +322,7 @@
/* Reset the local_as to be our EBGP one */
if (peer_sort (peer) == BGP_PEER_EBGP)
peer->local_as = as;
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_CONFED_ID_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -351,7 +351,7 @@
if (peer_sort (peer) != BGP_PEER_IBGP)
{
peer->local_as = bgp->as;
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_CONFED_ID_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -415,7 +415,7 @@
if (peer->as == as)
{
peer->local_as = bgp->as;
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_CONFED_PEER_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -471,7 +471,7 @@
if (peer->as == as)
{
peer->local_as = bgp->confed_id;
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_CONFED_PEER_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -911,7 +911,7 @@
/* Stop peer. */
if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP))
{
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_REMOTE_AS_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -1869,7 +1869,7 @@
peer_group2peer_config_copy (group, peer, afi, safi);
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_RMAP_BIND;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -1912,7 +1912,7 @@
peer_global_config_reset (peer);
}
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_RMAP_UNBIND;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -2095,10 +2095,28 @@
bgp_redistribute_unset (bgp, afi, i);
for (ALL_LIST_ELEMENTS (bgp->peer, node, next, peer))
- peer_delete (peer);
+ {
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
+ {
+ /* Send notify to remote peer. */
+ bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_ADMIN_SHUTDOWN);
+ }
+
+ peer_delete (peer);
+ }
for (ALL_LIST_ELEMENTS (bgp->group, node, next, group))
- peer_group_delete (group);
+ {
+ for (ALL_LIST_ELEMENTS (group->peer, node, next, peer))
+ {
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
+ {
+ /* Send notify to remote peer. */
+ bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_ADMIN_SHUTDOWN);
+ }
+ }
+ peer_group_delete (group);
+ }
assert (listcount (bgp->rsclient) == 0);
@@ -2406,7 +2424,7 @@
if (CHECK_FLAG (peer->sflags, PEER_STATUS_NSF_WAIT))
peer_nsf_stop (peer);
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
BGP_NOTIFY_CEASE_ADMIN_SHUTDOWN);
else
@@ -2418,7 +2436,7 @@
BGP_EVENT_ADD (peer, BGP_Stop);
}
}
- else if (peer->status == Established)
+ else if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
if (flag == PEER_FLAG_DYNAMIC_CAPABILITY)
peer->last_reset = PEER_DOWN_CAPABILITY_CHANGE;
@@ -2821,7 +2839,7 @@
if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP))
{
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -2853,7 +2871,7 @@
peer->update_if = XSTRDUP (MTYPE_PEER_UPDATE_SOURCE, ifname);
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -2890,7 +2908,7 @@
if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP))
{
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -2921,7 +2939,7 @@
peer->update_source = sockunion_dup (su);
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -2972,7 +2990,7 @@
if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP))
{
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -3002,7 +3020,7 @@
peer->update_if = NULL;
}
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -3441,7 +3459,7 @@
if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP))
{
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_LOCAL_AS_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -3467,7 +3485,7 @@
else
UNSET_FLAG (peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS);
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_LOCAL_AS_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -3498,7 +3516,7 @@
if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP))
{
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_LOCAL_AS_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -3517,7 +3535,7 @@
UNSET_FLAG (peer->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND);
UNSET_FLAG (peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS);
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
{
peer->last_reset = PEER_DOWN_LOCAL_AS_CHANGE;
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
@@ -3551,8 +3569,8 @@
if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP))
{
- if (peer->status == Established)
- bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE);
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
+ bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE);
else
BGP_EVENT_ADD (peer, BGP_Stop);
@@ -3569,7 +3587,7 @@
peer->password = XSTRDUP(MTYPE_PEER_PASSWORD, password);
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE);
else
BGP_EVENT_ADD (peer, BGP_Stop);
@@ -3597,7 +3615,7 @@
&& strcmp (peer->group->conf->password, peer->password) == 0)
return BGP_ERR_PEER_GROUP_HAS_THE_FLAG;
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE);
else
BGP_EVENT_ADD (peer, BGP_Stop);
@@ -3620,7 +3638,7 @@
if (!peer->password)
continue;
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE);
else
BGP_EVENT_ADD (peer, BGP_Stop);
@@ -4543,7 +4561,7 @@
}
peer->v_start = BGP_INIT_START_TIMER;
- if (peer->status == Established)
+ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status))
bgp_notify_send (peer, BGP_NOTIFY_CEASE,
BGP_NOTIFY_CEASE_ADMIN_RESET);
else
diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 024fedc..3d516d3 100644
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -778,6 +778,8 @@
/* Macros. */
#define BGP_INPUT(P) ((P)->ibuf)
#define BGP_INPUT_PNT(P) (STREAM_PNT(BGP_INPUT(P)))
+#define BGP_IS_VALID_STATE_FOR_NOTIF(S)\
+ (((S) == OpenSent) || ((S) == OpenConfirm) || ((S) == Established))
/* Count prefix size from mask length */
#define PSIZE(a) (((a) + 7) / (8))