From 2e5b4e32c4d402bd547ba06ac2b797236e7eaea6 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 5 Nov 2024 15:47:08 -0500 Subject: [PATCH 1/7] bgpd: peer_notify_unconfig should be connection based Convert this function to being connection based. Signed-off-by: Donald Sharp --- bgpd/bgp_vty.c | 10 +++++----- bgpd/bgpd.c | 17 ++++++----------- bgpd/bgpd.h | 2 +- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index d1238bc8de57..d1f20b3bbacb 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -5264,7 +5264,7 @@ DEFUN (no_neighbor, * interface. */ if (peer->ifp) bgp_zebra_terminate_radv(peer->bgp, peer); - peer_notify_unconfig(peer); + peer_notify_unconfig(peer->connection); peer_delete(peer); return CMD_SUCCESS; } @@ -5300,10 +5300,10 @@ DEFUN (no_neighbor, if (CHECK_FLAG(peer->flags, PEER_FLAG_CAPABILITY_ENHE)) bgp_zebra_terminate_radv(peer->bgp, peer); - peer_notify_unconfig(peer); + peer_notify_unconfig(peer->connection); peer_delete(peer); if (other && other->connection->status != Deleted) { - peer_notify_unconfig(other); + peer_notify_unconfig(other->connection); peer_delete(other); } } @@ -5338,7 +5338,7 @@ DEFUN (no_neighbor_interface_config, /* Request zebra to terminate IPv6 RAs on this interface. */ if (peer->ifp) bgp_zebra_terminate_radv(peer->bgp, peer); - peer_notify_unconfig(peer); + peer_notify_unconfig(peer->connection); peer_delete(peer); } else { vty_out(vty, "%% Create the bgp interface first\n"); @@ -5746,7 +5746,7 @@ DEFUN (no_neighbor_set_peer_group, if (CHECK_FLAG(peer->flags, PEER_FLAG_CAPABILITY_ENHE)) bgp_zebra_terminate_radv(peer->bgp, peer); - peer_notify_unconfig(peer); + peer_notify_unconfig(peer->connection); ret = peer_delete(peer); return bgp_vty_return(vty, ret); diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 258fc87f96db..947251e01492 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3076,11 +3076,10 @@ int peer_group_remote_as(struct bgp *bgp, const char *group_name, as_t *as, return 0; } -void peer_notify_unconfig(struct peer *peer) +void peer_notify_unconfig(struct peer_connection *connection) { - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_PEER_UNCONFIG); + if (BGP_IS_VALID_STATE_FOR_NOTIF(connection->status)) + bgp_notify_send(connection, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_PEER_UNCONFIG); } static void peer_notify_shutdown(struct peer *peer) @@ -3107,9 +3106,9 @@ void peer_group_notify_unconfig(struct peer_group *group) other = peer->doppelganger; if (other && other->connection->status != Deleted) { other->group = NULL; - peer_notify_unconfig(other); + peer_notify_unconfig(other->connection); } else - peer_notify_unconfig(peer); + peer_notify_unconfig(peer->connection); } } @@ -8841,11 +8840,7 @@ void bgp_terminate(void) peer); continue; } - if (BGP_IS_VALID_STATE_FOR_NOTIF( - peer->connection->status)) - bgp_notify_send(peer->connection, - BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_PEER_UNCONFIG); + peer_notify_unconfig(peer->connection); } } diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index f123188ae8c4..e5252b78b4f5 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -2385,7 +2385,7 @@ extern int peer_remote_as(struct bgp *bgp, union sockunion *su, extern int peer_group_remote_as(struct bgp *bgp, const char *peer_str, as_t *as, enum peer_asn_type as_type, const char *as_str); extern int peer_delete(struct peer *peer); -extern void peer_notify_unconfig(struct peer *peer); +extern void peer_notify_unconfig(struct peer_connection *connection); extern int peer_group_delete(struct peer_group *); extern int peer_group_remote_as_delete(struct peer_group *); extern int peer_group_listen_range_add(struct peer_group *, struct prefix *); From ba0edb9545038a8026813d5997a958cc6ed88765 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 6 Nov 2024 08:15:06 -0500 Subject: [PATCH 2/7] bgpd: Add `peer_notify_config_change()` function We have about a bajillion tests of if we can notify the peer and then we send a config change notification. Let's just make a function that does this. Signed-off-by: Donald Sharp --- bgpd/bgp_fsm.c | 12 ++--- bgpd/bgp_vty.c | 11 ++--- bgpd/bgpd.c | 131 +++++++++++++++++-------------------------------- bgpd/bgpd.h | 1 + 4 files changed, 52 insertions(+), 103 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 490451f193db..2cdbadc63c58 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -2746,8 +2746,7 @@ static void bgp_gr_update_mode_of_all_peers(struct bgp *bgp, peer->last_reset = PEER_DOWN_CAPABILITY_CHANGE; if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); else bgp_session_reset_safe(peer, &nnode); } else { @@ -2769,8 +2768,7 @@ static void bgp_gr_update_mode_of_all_peers(struct bgp *bgp, 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); + peer_notify_config_change(member->connection); else bgp_session_reset(member); } @@ -2974,8 +2972,7 @@ unsigned int bgp_peer_gr_action(struct peer *peer, enum peer_mode old_state, peer->last_reset = PEER_DOWN_CAPABILITY_CHANGE; if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); else bgp_session_reset(peer); } else { @@ -2985,8 +2982,7 @@ unsigned int bgp_peer_gr_action(struct peer *peer, enum peer_mode old_state, 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); + peer_notify_config_change(member->connection); else bgp_session_reset(member); } diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index d1f20b3bbacb..651ec71b1a90 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -2940,9 +2940,7 @@ DEFUN(bgp_reject_as_sets, bgp_reject_as_sets_cmd, */ for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { peer->last_reset = PEER_DOWN_AS_SETS_REJECT; - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } return CMD_SUCCESS; @@ -2965,9 +2963,7 @@ DEFUN(no_bgp_reject_as_sets, no_bgp_reject_as_sets_cmd, */ for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { peer->last_reset = PEER_DOWN_AS_SETS_REJECT; - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } return CMD_SUCCESS; @@ -5101,8 +5097,7 @@ static int peer_conf_interface_get(struct vty *vty, const char *conf_if, /* v6only flag changed. Reset bgp seesion */ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); else bgp_session_reset(peer); } diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 947251e01492..5940b20de8c8 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -309,9 +309,7 @@ static int bgp_router_id_set(struct bgp *bgp, const struct in_addr *id, peer->last_reset = PEER_DOWN_RID_CHANGE; - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } /* EVPN uses router id in RD, update them */ @@ -447,8 +445,7 @@ void bm_wait_for_fib_set(bool set) peer->connection->status)) continue; - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } } } @@ -507,8 +504,7 @@ void bgp_suppress_fib_pending_set(struct bgp *bgp, bool set) if (!BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) continue; - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } } @@ -532,9 +528,7 @@ void bgp_cluster_id_set(struct bgp *bgp, struct in_addr *cluster_id) peer->last_reset = PEER_DOWN_CLID_CHANGE; - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } } @@ -556,9 +550,7 @@ void bgp_cluster_id_unset(struct bgp *bgp) peer->last_reset = PEER_DOWN_CLID_CHANGE; - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } } @@ -641,9 +633,7 @@ void bgp_confederation_id_set(struct bgp *bgp, as_t as, const char *as_str) peer->connection->status)) { peer->last_reset = PEER_DOWN_CONFED_ID_CHANGE; - bgp_notify_send(peer->connection, - BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } else bgp_session_reset_safe(peer, &nnode); } @@ -659,9 +649,7 @@ void bgp_confederation_id_set(struct bgp *bgp, as_t as, const char *as_str) peer->connection->status)) { peer->last_reset = PEER_DOWN_CONFED_ID_CHANGE; - bgp_notify_send(peer->connection, - BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } else bgp_session_reset_safe(peer, &nnode); } @@ -686,9 +674,7 @@ void bgp_confederation_id_unset(struct bgp *bgp) peer->last_reset = PEER_DOWN_CONFED_ID_CHANGE; if (BGP_IS_VALID_STATE_FOR_NOTIF( peer->connection->status)) - bgp_notify_send(peer->connection, - BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); else bgp_session_reset_safe(peer, &nnode); } @@ -740,9 +726,7 @@ void bgp_confederation_peers_add(struct bgp *bgp, as_t as, const char *as_str) peer->connection->status)) { peer->last_reset = PEER_DOWN_CONFED_PEER_CHANGE; - bgp_notify_send(peer->connection, - BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } else bgp_session_reset_safe(peer, &nnode); } @@ -797,9 +781,7 @@ void bgp_confederation_peers_remove(struct bgp *bgp, as_t as) peer->connection->status)) { peer->last_reset = PEER_DOWN_CONFED_PEER_CHANGE; - bgp_notify_send(peer->connection, - BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } else bgp_session_reset_safe(peer, &nnode); } @@ -2100,8 +2082,7 @@ void peer_as_change(struct peer *peer, as_t as, enum peer_asn_type as_type, if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { peer->last_reset = PEER_DOWN_REMOTE_AS_CHANGE; if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); else bgp_session_reset(peer); } @@ -2467,15 +2448,11 @@ static int peer_activate_af(struct peer *peer, afi_t afi, safi_t safi) false); } } else { - bgp_notify_send(peer->connection, - BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } } - if (peer->connection->status == OpenSent || - peer->connection->status == OpenConfirm) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); + /* * If we are turning on a AFI/SAFI locally and we've * started bringing a peer up, we need to tell @@ -2488,8 +2465,7 @@ static int peer_activate_af(struct peer *peer, afi_t afi, safi_t safi) other = peer->doppelganger; if (other && (other->connection->status == OpenSent || other->connection->status == OpenConfirm)) - bgp_notify_send(other->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(other->connection); } return 0; @@ -2596,14 +2572,10 @@ static bool non_peergroup_deactivate_af(struct peer *peer, afi_t afi, bgp_clear_route(peer, afi, safi); peer->pcount[afi][safi] = 0; } else { - bgp_notify_send(peer->connection, - BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } - } else { - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); - } + } else + peer_notify_config_change(peer->connection); } return false; @@ -3076,6 +3048,12 @@ int peer_group_remote_as(struct bgp *bgp, const char *group_name, as_t *as, return 0; } +void peer_notify_config_change(struct peer_connection *connection) +{ + if (BGP_IS_VALID_STATE_FOR_NOTIF(connection->status)) + bgp_notify_send(connection, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); +} + void peer_notify_unconfig(struct peer_connection *connection) { if (BGP_IS_VALID_STATE_FOR_NOTIF(connection->status)) @@ -3356,8 +3334,7 @@ int peer_group_bind(struct bgp *bgp, union sockunion *su, struct peer *peer, peer->last_reset = PEER_DOWN_RMAP_BIND; if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); else bgp_session_reset(peer); } @@ -4725,8 +4702,7 @@ void peer_change_action(struct peer *peer, afi_t afi, safi_t safi, PEER_FLAG_CONFIG_NODE))) peer_delete(peer->doppelganger); - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } else if (type == peer_change_reset_in) { if (CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_RCV)) bgp_route_refresh_send(peer, afi, safi, 0, 0, 0, @@ -4738,8 +4714,7 @@ void peer_change_action(struct peer *peer, afi_t afi, safi_t safi, PEER_FLAG_CONFIG_NODE))) peer_delete(peer->doppelganger); - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } } else if (type == peer_change_reset_out) { paf = peer_af_find(peer, afi, safi); @@ -4939,8 +4914,7 @@ static void peer_flag_modify_action(struct peer *peer, uint64_t flag) BGP_EVENT_ADD(peer->connection, BGP_Stop); } } else if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) { - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); } else bgp_session_reset(peer); } @@ -5427,9 +5401,7 @@ int peer_ebgp_multihop_set(struct peer *peer, int ttl) if (peer->sort != BGP_PEER_IBGP) { if (BGP_IS_VALID_STATE_FOR_NOTIF( peer->connection->status)) - bgp_notify_send(peer->connection, - BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); else bgp_session_reset(peer); @@ -5446,8 +5418,7 @@ int peer_ebgp_multihop_set(struct peer *peer, int ttl) member->ttl = group->conf->ttl; if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - bgp_notify_send(member->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(member->connection); else bgp_session_reset(member); @@ -5484,8 +5455,7 @@ int peer_ebgp_multihop_unset(struct peer *peer) if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); else bgp_session_reset(peer); @@ -5502,8 +5472,7 @@ int peer_ebgp_multihop_unset(struct peer *peer) if (member->connection->fd >= 0) { if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - bgp_notify_send(member->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(member->connection); else bgp_session_reset(member); } @@ -5657,8 +5626,7 @@ int peer_update_source_if_set(struct peer *peer, const char *ifname) peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); else bgp_session_reset(peer); @@ -5695,8 +5663,7 @@ int peer_update_source_if_set(struct peer *peer, const char *ifname) /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - bgp_notify_send(member->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(member->connection); else bgp_session_reset(member); @@ -5728,8 +5695,7 @@ void peer_update_source_addr_set(struct peer *peer, const union sockunion *su) peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); else bgp_session_reset(peer); @@ -5765,8 +5731,7 @@ void peer_update_source_addr_set(struct peer *peer, const union sockunion *su) /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - bgp_notify_send(member->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); else bgp_session_reset(member); @@ -5816,8 +5781,7 @@ void peer_update_source_unset(struct peer *peer) peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); else bgp_session_reset(peer); @@ -5852,8 +5816,7 @@ void peer_update_source_unset(struct peer *peer) /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - bgp_notify_send(member->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(member->connection); else bgp_session_reset(member); @@ -6885,8 +6848,7 @@ int peer_local_as_unset(struct peer *peer) peer->last_reset = PEER_DOWN_LOCAL_AS_CHANGE; /* Send notification or stop peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); else BGP_EVENT_ADD(peer->connection, BGP_Stop); @@ -6914,8 +6876,7 @@ int peer_local_as_unset(struct peer *peer) /* Send notification or stop peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - bgp_notify_send(member->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(member->connection); else bgp_session_reset(member); } @@ -6946,8 +6907,7 @@ int peer_password_set(struct peer *peer, const char *password) peer->last_reset = PEER_DOWN_PASSWORD_CHANGE; /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); else bgp_session_reset(peer); @@ -6984,8 +6944,7 @@ int peer_password_set(struct peer *peer, const char *password) member->last_reset = PEER_DOWN_PASSWORD_CHANGE; /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - bgp_notify_send(member->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(member->connection); else bgp_session_reset(member); @@ -7030,8 +6989,7 @@ int peer_password_unset(struct peer *peer) if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(peer->connection); else bgp_session_reset(peer); @@ -7057,8 +7015,7 @@ int peer_password_unset(struct peer *peer) /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - bgp_notify_send(member->connection, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + peer_notify_config_change(member->connection); else bgp_session_reset(member); diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index e5252b78b4f5..5af2d387ae53 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -2386,6 +2386,7 @@ extern int peer_group_remote_as(struct bgp *bgp, const char *peer_str, as_t *as, enum peer_asn_type as_type, const char *as_str); extern int peer_delete(struct peer *peer); extern void peer_notify_unconfig(struct peer_connection *connection); +extern void peer_notify_config_change(struct peer_connection *connection); extern int peer_group_delete(struct peer_group *); extern int peer_group_remote_as_delete(struct peer_group *); extern int peer_group_listen_range_add(struct peer_group *, struct prefix *); From eacf923b00c019e9a877c9716e5d6506594d532e Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 6 Nov 2024 08:24:28 -0500 Subject: [PATCH 3/7] bgpd: Fix pattern of usage in bgp_notify_config_change if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) peer_notify_config_change(peer->connection); else bgp_session_reset_safe(peer, &nnode); Let's add a bool return to peer_notify_config_change of whether or not it should call the peer session reset. This simplifies the code a bunch. Signed-off-by: Donald Sharp --- bgpd/bgp_fsm.c | 16 ++----- bgpd/bgp_vty.c | 4 +- bgpd/bgpd.c | 117 +++++++++++++++---------------------------------- bgpd/bgpd.h | 2 +- 4 files changed, 41 insertions(+), 98 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 2cdbadc63c58..6734c5e8dca6 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -2745,9 +2745,7 @@ static void bgp_gr_update_mode_of_all_peers(struct bgp *bgp, peer->last_reset = PEER_DOWN_CAPABILITY_CHANGE; - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - peer_notify_config_change(peer->connection); - else + if (!peer_notify_config_change(peer->connection)) bgp_session_reset_safe(peer, &nnode); } else { group = peer->group; @@ -2767,9 +2765,7 @@ static void bgp_gr_update_mode_of_all_peers(struct bgp *bgp, member->last_reset = PEER_DOWN_CAPABILITY_CHANGE; - if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - peer_notify_config_change(member->connection); - else + if (!peer_notify_config_change(member->connection)) bgp_session_reset(member); } } @@ -2971,9 +2967,7 @@ unsigned int bgp_peer_gr_action(struct peer *peer, enum peer_mode old_state, if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { peer->last_reset = PEER_DOWN_CAPABILITY_CHANGE; - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - peer_notify_config_change(peer->connection); - else + if (!peer_notify_config_change(peer->connection)) bgp_session_reset(peer); } else { group = peer->group; @@ -2981,9 +2975,7 @@ unsigned int bgp_peer_gr_action(struct peer *peer, enum peer_mode old_state, 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)) - peer_notify_config_change(member->connection); - else + if (!peer_notify_config_change(member->connection)) bgp_session_reset(member); } } diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 651ec71b1a90..6ff94129dcf5 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -5096,9 +5096,7 @@ static int peer_conf_interface_get(struct vty *vty, const char *conf_if, peer->last_reset = PEER_DOWN_V6ONLY_CHANGE; /* v6only flag changed. Reset bgp seesion */ - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - peer_notify_config_change(peer->connection); - else + if (!peer_notify_config_change(peer->connection)) bgp_session_reset(peer); } diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 5940b20de8c8..9d0f579c2862 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -629,12 +629,10 @@ void bgp_confederation_id_set(struct bgp *bgp, as_t as, const char *as_str) if (already_confed) { if (ptype == BGP_PEER_EBGP) { peer->local_as = as; - if (BGP_IS_VALID_STATE_FOR_NOTIF( - peer->connection->status)) { + if (peer_notify_config_change(peer->connection)) peer->last_reset = PEER_DOWN_CONFED_ID_CHANGE; - peer_notify_config_change(peer->connection); - } else + else bgp_session_reset_safe(peer, &nnode); } } else { @@ -645,12 +643,10 @@ void bgp_confederation_id_set(struct bgp *bgp, as_t as, const char *as_str) /* Reset the local_as to be our EBGP one */ if (ptype == BGP_PEER_EBGP) peer->local_as = as; - if (BGP_IS_VALID_STATE_FOR_NOTIF( - peer->connection->status)) { + if (peer_notify_config_change(peer->connection)) peer->last_reset = PEER_DOWN_CONFED_ID_CHANGE; - peer_notify_config_change(peer->connection); - } else + else bgp_session_reset_safe(peer, &nnode); } } @@ -672,10 +668,7 @@ void bgp_confederation_id_unset(struct bgp *bgp) if (peer_sort(peer) != BGP_PEER_IBGP) { peer->local_as = bgp->as; peer->last_reset = PEER_DOWN_CONFED_ID_CHANGE; - if (BGP_IS_VALID_STATE_FOR_NOTIF( - peer->connection->status)) - peer_notify_config_change(peer->connection); - else + if (!peer_notify_config_change(peer->connection)) bgp_session_reset_safe(peer, &nnode); } } @@ -722,12 +715,10 @@ void bgp_confederation_peers_add(struct bgp *bgp, as_t as, const char *as_str) if (peer->as == as) { peer->local_as = bgp->as; (void)peer_sort(peer); - if (BGP_IS_VALID_STATE_FOR_NOTIF( - peer->connection->status)) { + if (peer_notify_config_change(peer->connection)) peer->last_reset = PEER_DOWN_CONFED_PEER_CHANGE; - peer_notify_config_change(peer->connection); - } else + else bgp_session_reset_safe(peer, &nnode); } } @@ -777,12 +768,10 @@ void bgp_confederation_peers_remove(struct bgp *bgp, as_t as) if (peer->as == as) { peer->local_as = bgp->confed_id; (void)peer_sort(peer); - if (BGP_IS_VALID_STATE_FOR_NOTIF( - peer->connection->status)) { + if (peer_notify_config_change(peer->connection)) peer->last_reset = PEER_DOWN_CONFED_PEER_CHANGE; - peer_notify_config_change(peer->connection); - } else + else bgp_session_reset_safe(peer, &nnode); } } @@ -2081,9 +2070,7 @@ void peer_as_change(struct peer *peer, as_t as, enum peer_asn_type as_type, /* Stop peer. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { peer->last_reset = PEER_DOWN_REMOTE_AS_CHANGE; - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - peer_notify_config_change(peer->connection); - else + if (!peer_notify_config_change(peer->connection)) bgp_session_reset(peer); } origtype = peer_sort_lookup(peer); @@ -2463,8 +2450,7 @@ static int peer_activate_af(struct peer *peer, afi_t afi, safi_t safi) * activation. */ other = peer->doppelganger; - if (other && (other->connection->status == OpenSent || - other->connection->status == OpenConfirm)) + if (other) peer_notify_config_change(other->connection); } @@ -3048,10 +3034,14 @@ int peer_group_remote_as(struct bgp *bgp, const char *group_name, as_t *as, return 0; } -void peer_notify_config_change(struct peer_connection *connection) +bool peer_notify_config_change(struct peer_connection *connection) { - if (BGP_IS_VALID_STATE_FOR_NOTIF(connection->status)) + if (BGP_IS_VALID_STATE_FOR_NOTIF(connection->status)) { bgp_notify_send(connection, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); + return true; + } + + return false; } void peer_notify_unconfig(struct peer_connection *connection) @@ -3333,9 +3323,7 @@ int peer_group_bind(struct bgp *bgp, union sockunion *su, struct peer *peer, peer->last_reset = PEER_DOWN_RMAP_BIND; - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - peer_notify_config_change(peer->connection); - else + if (!peer_notify_config_change(peer->connection)) bgp_session_reset(peer); } @@ -4913,9 +4901,7 @@ static void peer_flag_modify_action(struct peer *peer, uint64_t flag) peer->v_start = BGP_INIT_START_TIMER; BGP_EVENT_ADD(peer->connection, BGP_Stop); } - } else if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) { - peer_notify_config_change(peer->connection); - } else + } else if (!peer_notify_config_change(peer->connection)) bgp_session_reset(peer); } @@ -5399,10 +5385,7 @@ int peer_ebgp_multihop_set(struct peer *peer, int ttl) if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { if (peer->sort != BGP_PEER_IBGP) { - if (BGP_IS_VALID_STATE_FOR_NOTIF( - peer->connection->status)) - peer_notify_config_change(peer->connection); - else + if (!peer_notify_config_change(peer->connection)) bgp_session_reset(peer); /* Reconfigure BFD peer with new TTL. */ @@ -5417,9 +5400,7 @@ int peer_ebgp_multihop_set(struct peer *peer, int ttl) member->ttl = group->conf->ttl; - if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - peer_notify_config_change(member->connection); - else + if (!peer_notify_config_change(member->connection)) bgp_session_reset(member); /* Reconfigure BFD peer with new TTL. */ @@ -5454,9 +5435,7 @@ int peer_ebgp_multihop_unset(struct peer *peer) peer->ttl = ttl; if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - peer_notify_config_change(peer->connection); - else + if (!peer_notify_config_change(peer->connection)) bgp_session_reset(peer); /* Reconfigure BFD peer with new TTL. */ @@ -5471,9 +5450,7 @@ int peer_ebgp_multihop_unset(struct peer *peer) member->ttl = BGP_DEFAULT_TTL; if (member->connection->fd >= 0) { - if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - peer_notify_config_change(member->connection); - else + if (!peer_notify_config_change(member->connection)) bgp_session_reset(member); } @@ -5625,9 +5602,7 @@ int peer_update_source_if_set(struct peer *peer, const char *ifname) if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; /* Send notification or reset peer depending on state. */ - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - peer_notify_config_change(peer->connection); - else + if (!peer_notify_config_change(peer->connection)) bgp_session_reset(peer); /* Apply new source configuration to BFD session. */ @@ -5662,9 +5637,7 @@ int peer_update_source_if_set(struct peer *peer, const char *ifname) member->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; /* Send notification or reset peer depending on state. */ - if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - peer_notify_config_change(member->connection); - else + if (!peer_notify_config_change(member->connection)) bgp_session_reset(member); /* Apply new source configuration to BFD session. */ @@ -5694,9 +5667,7 @@ void peer_update_source_addr_set(struct peer *peer, const union sockunion *su) if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; /* Send notification or reset peer depending on state. */ - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - peer_notify_config_change(peer->connection); - else + if (!peer_notify_config_change(peer->connection)) bgp_session_reset(peer); /* Apply new source configuration to BFD session. */ @@ -5730,9 +5701,7 @@ void peer_update_source_addr_set(struct peer *peer, const union sockunion *su) member->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; /* Send notification or reset peer depending on state. */ - if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - peer_notify_config_change(peer->connection); - else + if (!peer_notify_config_change(peer->connection)) bgp_session_reset(member); /* Apply new source configuration to BFD session. */ @@ -5780,9 +5749,7 @@ void peer_update_source_unset(struct peer *peer) if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; /* Send notification or reset peer depending on state. */ - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - peer_notify_config_change(peer->connection); - else + if (!peer_notify_config_change(peer->connection)) bgp_session_reset(peer); /* Apply new source configuration to BFD session. */ @@ -5815,9 +5782,7 @@ void peer_update_source_unset(struct peer *peer) member->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; /* Send notification or reset peer depending on state. */ - if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - peer_notify_config_change(member->connection); - else + if (!peer_notify_config_change(member->connection)) bgp_session_reset(member); /* Apply new source configuration to BFD session. */ @@ -6847,9 +6812,7 @@ int peer_local_as_unset(struct peer *peer) if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { peer->last_reset = PEER_DOWN_LOCAL_AS_CHANGE; /* Send notification or stop peer depending on state. */ - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - peer_notify_config_change(peer->connection); - else + if (!peer_notify_config_change(peer->connection)) BGP_EVENT_ADD(peer->connection, BGP_Stop); /* Skip peer-group mechanics for regular peers. */ @@ -6875,9 +6838,7 @@ int peer_local_as_unset(struct peer *peer) member->last_reset = PEER_DOWN_LOCAL_AS_CHANGE; /* Send notification or stop peer depending on state. */ - if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - peer_notify_config_change(member->connection); - else + if (!peer_notify_config_change(member->connection)) bgp_session_reset(member); } @@ -6906,9 +6867,7 @@ int peer_password_set(struct peer *peer, const char *password) if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { peer->last_reset = PEER_DOWN_PASSWORD_CHANGE; /* Send notification or reset peer depending on state. */ - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - peer_notify_config_change(peer->connection); - else + if (!peer_notify_config_change(peer->connection)) bgp_session_reset(peer); /* @@ -6943,9 +6902,7 @@ int peer_password_set(struct peer *peer, const char *password) member->last_reset = PEER_DOWN_PASSWORD_CHANGE; /* Send notification or reset peer depending on state. */ - if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - peer_notify_config_change(member->connection); - else + if (!peer_notify_config_change(member->connection)) bgp_session_reset(member); /* Attempt to install password on socket. */ @@ -6988,9 +6945,7 @@ int peer_password_unset(struct peer *peer) /* Check if handling a regular peer. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { /* Send notification or reset peer depending on state. */ - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) - peer_notify_config_change(peer->connection); - else + if (!peer_notify_config_change(peer->connection)) bgp_session_reset(peer); /* Attempt to uninstall password on socket. */ @@ -7014,9 +6969,7 @@ int peer_password_unset(struct peer *peer) XFREE(MTYPE_PEER_PASSWORD, member->password); /* Send notification or reset peer depending on state. */ - if (BGP_IS_VALID_STATE_FOR_NOTIF(member->connection->status)) - peer_notify_config_change(member->connection); - else + if (!peer_notify_config_change(member->connection)) bgp_session_reset(member); /* Attempt to uninstall password on socket. */ diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 5af2d387ae53..2b6921b69503 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -2386,7 +2386,7 @@ extern int peer_group_remote_as(struct bgp *bgp, const char *peer_str, as_t *as, enum peer_asn_type as_type, const char *as_str); extern int peer_delete(struct peer *peer); extern void peer_notify_unconfig(struct peer_connection *connection); -extern void peer_notify_config_change(struct peer_connection *connection); +extern bool peer_notify_config_change(struct peer_connection *connection); extern int peer_group_delete(struct peer_group *); extern int peer_group_remote_as_delete(struct peer_group *); extern int peer_group_listen_range_add(struct peer_group *, struct prefix *); From 2771431938d3cdde4a210bbf4c600ef88c985642 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 6 Nov 2024 11:55:43 -0500 Subject: [PATCH 4/7] bgpd: Modify bgp_udpatesockname to pass in a connection Signed-off-by: Donald Sharp --- bgpd/bgp_fsm.c | 2 +- bgpd/bgp_network.c | 8 ++++---- bgpd/bgp_network.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 6734c5e8dca6..cd63480f20a2 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -1809,7 +1809,7 @@ bgp_connect_fail(struct peer_connection *connection) */ static void bgp_connect_in_progress_update_connection(struct peer *peer) { - bgp_updatesockname(peer); + bgp_updatesockname(peer, peer->connection); if (!peer->su_remote && !BGP_CONNECTION_SU_UNSPEC(peer->connection)) { /* if connect initiated, then dest port and dest addresses are well known */ peer->su_remote = sockunion_dup(&peer->connection->su); diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index 844f6b9af2f1..89c71060a22a 100644 --- a/bgpd/bgp_network.c +++ b/bgpd/bgp_network.c @@ -861,7 +861,7 @@ enum connect_result bgp_connect(struct peer_connection *connection) htons(peer->port), ifindex); } -void bgp_updatesockname(struct peer *peer) +void bgp_updatesockname(struct peer *peer, struct peer_connection *connection) { if (peer->su_local) { sockunion_free(peer->su_local); @@ -873,14 +873,14 @@ void bgp_updatesockname(struct peer *peer) peer->su_remote = NULL; } - peer->su_local = sockunion_getsockname(peer->connection->fd); - peer->su_remote = sockunion_getpeername(peer->connection->fd); + peer->su_local = sockunion_getsockname(connection->fd); + peer->su_remote = sockunion_getpeername(connection->fd); } /* After TCP connection is established. Get local address and port. */ int bgp_getsockname(struct peer *peer) { - bgp_updatesockname(peer); + bgp_updatesockname(peer, peer->connection); if (!bgp_zebra_nexthop_set(peer->su_local, peer->su_remote, &peer->nexthop, peer)) { diff --git a/bgpd/bgp_network.h b/bgpd/bgp_network.h index 61ca19a34da5..481661825d21 100644 --- a/bgpd/bgp_network.h +++ b/bgpd/bgp_network.h @@ -23,7 +23,7 @@ extern void bgp_close_vrf_socket(struct bgp *bgp); extern void bgp_close(void); extern enum connect_result bgp_connect(struct peer_connection *connection); extern int bgp_getsockname(struct peer *peer); -extern void bgp_updatesockname(struct peer *peer); +extern void bgp_updatesockname(struct peer *peer, struct peer_connection *connection); extern int bgp_md5_set_prefix(struct bgp *bgp, struct prefix *p, const char *password); From 72f716ef2880408a3a6e71691f5c8aae24b059cd Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 6 Nov 2024 14:25:20 -0500 Subject: [PATCH 5/7] bgpd: Modify bgp_connect_in_progress_update_connection to use connection Signed-off-by: Donald Sharp --- bgpd/bgp_fsm.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index cd63480f20a2..240ec59481bc 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -1807,12 +1807,14 @@ bgp_connect_fail(struct peer_connection *connection) /* after connect is called(), getpeername is able to return * port and address on non established streams */ -static void bgp_connect_in_progress_update_connection(struct peer *peer) +static void bgp_connect_in_progress_update_connection(struct peer_connection *connection) { - bgp_updatesockname(peer, peer->connection); + struct peer *peer = connection->peer; + + bgp_updatesockname(peer, connection); if (!peer->su_remote && !BGP_CONNECTION_SU_UNSPEC(peer->connection)) { /* if connect initiated, then dest port and dest addresses are well known */ - peer->su_remote = sockunion_dup(&peer->connection->su); + peer->su_remote = sockunion_dup(&connection->su); if (sockunion_family(peer->su_remote) == AF_INET) peer->su_remote->sin.sin_port = htons(peer->port); else if (sockunion_family(peer->su_remote) == AF_INET6) @@ -1916,7 +1918,7 @@ static enum bgp_fsm_state_progress bgp_start(struct peer_connection *connection) __func__, peer->connection->fd); return BGP_FSM_FAILURE; } - bgp_connect_in_progress_update_connection(peer); + bgp_connect_in_progress_update_connection(connection); /* * - when the socket becomes ready, poll() will signify POLLOUT From 1baeb81632d46c20f7f75e619cfea73784d66c01 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 6 Nov 2024 14:31:19 -0500 Subject: [PATCH 6/7] bgpd: bgp_getsockname should use connection Let's use the connection associated with the peer instead. Signed-off-by: Donald Sharp --- bgpd/bgp_fsm.c | 12 ++++++------ bgpd/bgp_network.c | 4 +++- bgpd/bgp_network.h | 2 +- bgpd/bgp_packet.c | 2 +- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 240ec59481bc..463296f0258a 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -265,7 +265,7 @@ static struct peer *peer_xfer_conn(struct peer *from_peer) from_peer->addpath_paths_limit[afi][safi]; } - if (bgp_getsockname(peer) < 0) { + if (bgp_getsockname(keeper) < 0) { flog_err(EC_LIB_SOCKET, "%%bgp_getsockname() failed for %s peer %s fd %d (from_peer fd %d)", (CHECK_FLAG(peer->sflags, PEER_STATUS_ACCEPT_PEER) @@ -277,7 +277,7 @@ static struct peer *peer_xfer_conn(struct peer *from_peer) return NULL; } if (going_away->status > Active) { - if (bgp_getsockname(from_peer) < 0) { + if (bgp_getsockname(going_away) < 0) { flog_err(EC_LIB_SOCKET, "%%bgp_getsockname() failed for %s from_peer %s fd %d (peer fd %d)", @@ -1694,11 +1694,11 @@ bgp_connect_success(struct peer_connection *connection) return bgp_stop(connection); } - if (bgp_getsockname(peer) < 0) { + if (bgp_getsockname(connection) < 0) { flog_err_sys(EC_LIB_SOCKET, "%s: bgp_getsockname(): failed for peer %s, fd %d", __func__, peer->host, connection->fd); - bgp_notify_send(peer->connection, BGP_NOTIFY_FSM_ERR, + bgp_notify_send(connection, BGP_NOTIFY_FSM_ERR, bgp_fsm_error_subcode(connection->status)); bgp_writes_on(connection); return BGP_FSM_FAILURE; @@ -1740,11 +1740,11 @@ bgp_connect_success_w_delayopen(struct peer_connection *connection) return bgp_stop(connection); } - if (bgp_getsockname(peer) < 0) { + if (bgp_getsockname(connection) < 0) { flog_err_sys(EC_LIB_SOCKET, "%s: bgp_getsockname(): failed for peer %s, fd %d", __func__, peer->host, connection->fd); - bgp_notify_send(peer->connection, BGP_NOTIFY_FSM_ERR, + bgp_notify_send(connection, BGP_NOTIFY_FSM_ERR, bgp_fsm_error_subcode(connection->status)); bgp_writes_on(connection); return BGP_FSM_FAILURE; diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index 89c71060a22a..e6117a5ce021 100644 --- a/bgpd/bgp_network.c +++ b/bgpd/bgp_network.c @@ -878,8 +878,10 @@ void bgp_updatesockname(struct peer *peer, struct peer_connection *connection) } /* After TCP connection is established. Get local address and port. */ -int bgp_getsockname(struct peer *peer) +int bgp_getsockname(struct peer_connection *connection) { + struct peer *peer = connection->peer; + bgp_updatesockname(peer, peer->connection); if (!bgp_zebra_nexthop_set(peer->su_local, peer->su_remote, diff --git a/bgpd/bgp_network.h b/bgpd/bgp_network.h index 481661825d21..ed1a72ec8900 100644 --- a/bgpd/bgp_network.h +++ b/bgpd/bgp_network.h @@ -22,7 +22,7 @@ extern int bgp_socket(struct bgp *bgp, unsigned short port, extern void bgp_close_vrf_socket(struct bgp *bgp); extern void bgp_close(void); extern enum connect_result bgp_connect(struct peer_connection *connection); -extern int bgp_getsockname(struct peer *peer); +extern int bgp_getsockname(struct peer_connection *connection); extern void bgp_updatesockname(struct peer *peer, struct peer_connection *connection); extern int bgp_md5_set_prefix(struct bgp *bgp, struct prefix *p, diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index a76a300c11bc..e9cc52449b42 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -2054,7 +2054,7 @@ static int bgp_open_receive(struct peer_connection *connection, return BGP_Stop; /* Get sockname. */ - if (bgp_getsockname(peer) < 0) { + if (bgp_getsockname(connection) < 0) { flog_err_sys(EC_LIB_SOCKET, "%s: bgp_getsockname() failed for peer: %s", __func__, peer->host); From 7bf3f53e44b8e54cdc92450ae6a68b7f17d36684 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 6 Nov 2024 15:30:32 -0500 Subject: [PATCH 7/7] bgpd: peer_active is connection oriented, make it so Signed-off-by: Donald Sharp --- bgpd/bgp_fsm.c | 4 ++-- bgpd/bgp_network.c | 6 +++--- bgpd/bgp_nexthop.c | 2 +- bgpd/bgp_zebra.c | 2 +- bgpd/bgpd.c | 27 ++++++++++++++------------- bgpd/bgpd.h | 2 +- 6 files changed, 22 insertions(+), 21 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 463296f0258a..cadef3997423 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -325,8 +325,8 @@ void bgp_timer_set(struct peer_connection *connection) /* First entry point of peer's finite state machine. In Idle status start timer is on unless peer is shutdown or peer is inactive. All other timer must be turned off */ - if (BGP_PEER_START_SUPPRESSED(peer) || !peer_active(peer) - || peer->bgp->vrf_id == VRF_UNKNOWN) { + if (BGP_PEER_START_SUPPRESSED(peer) || !peer_active(connection) || + peer->bgp->vrf_id == VRF_UNKNOWN) { EVENT_OFF(connection->t_start); } else { BGP_TIMER_ON(connection->t_start, bgp_start_timer, diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index e6117a5ce021..f1bea1c189ff 100644 --- a/bgpd/bgp_network.c +++ b/bgpd/bgp_network.c @@ -504,7 +504,7 @@ static void bgp_accept(struct event *thread) bgp_fsm_change_status(connection1, Active); EVENT_OFF(connection1->t_start); - if (peer_active(peer1)) { + if (peer_active(peer1->connection)) { if (CHECK_FLAG(peer1->flags, PEER_FLAG_TIMER_DELAYOPEN)) BGP_EVENT_ADD(connection1, @@ -557,7 +557,7 @@ static void bgp_accept(struct event *thread) } /* Check that at least one AF is activated for the peer. */ - if (!peer_active(peer1)) { + if (!peer_active(connection1)) { if (bgp_debug_neighbor_events(peer1)) zlog_debug( "%s - incoming conn rejected - no AF activated for peer", @@ -658,7 +658,7 @@ static void bgp_accept(struct event *thread) bgp_event_update(connection1, TCP_connection_closed); } - if (peer_active(peer)) { + if (peer_active(peer->connection)) { if (CHECK_FLAG(peer->flags, PEER_FLAG_TIMER_DELAYOPEN)) BGP_EVENT_ADD(connection, TCP_connection_open_w_delay); else diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c index bf0f3b15cfde..1ef90a8e382a 100644 --- a/bgpd/bgp_nexthop.c +++ b/bgpd/bgp_nexthop.c @@ -444,7 +444,7 @@ void bgp_connected_add(struct bgp *bgp, struct connected *ifc) !peer_established(peer->connection) && !CHECK_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY)) { connection = peer->connection; - if (peer_active(peer)) + if (peer_active(connection)) BGP_EVENT_ADD(connection, BGP_Stop); BGP_EVENT_ADD(connection, BGP_Start); } diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 16f4a0d2df19..688dfacaa0b6 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -137,7 +137,7 @@ static void bgp_start_interface_nbrs(struct bgp *bgp, struct interface *ifp) for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { if (peer->conf_if && (strcmp(peer->conf_if, ifp->name) == 0) && !peer_established(peer->connection)) { - if (peer_active(peer)) + if (peer_active(peer->connection)) BGP_EVENT_ADD(peer->connection, BGP_Stop); BGP_EVENT_ADD(peer->connection, BGP_Start); } diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 9d0f579c2862..f92ae969f8c4 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1986,7 +1986,7 @@ struct peer *peer_create(union sockunion *su, const char *conf_if, bgp->coalesce_time = MIN(BGP_MAX_SUBGROUP_COALESCE_TIME, ct); } - active = peer_active(peer); + active = peer_active(peer->connection); if (!active) { if (peer->connection->su.sa.sa_family == AF_UNSPEC) peer->last_reset = PEER_DOWN_NBR_ADDR; @@ -2019,7 +2019,7 @@ struct peer *peer_create(union sockunion *su, const char *conf_if, if (bgp->autoshutdown) peer_flag_set(peer, PEER_FLAG_SHUTDOWN); /* Set up peer's events and timers. */ - else if (!active && peer_active(peer)) + else if (!active && peer_active(peer->connection)) bgp_timer_set(peer->connection); bgp_peer_gr_flags_update(peer); @@ -2412,13 +2412,13 @@ static int peer_activate_af(struct peer *peer, afi_t afi, safi_t safi) if (peer_af_create(peer, afi, safi) == NULL) return 1; - active = peer_active(peer); + active = peer_active(peer->connection); peer->afc[afi][safi] = 1; if (peer->group) peer_group2peer_config_copy_af(peer->group, peer, afi, safi); - if (!active && peer_active(peer)) { + if (!active && peer_active(peer->connection)) { bgp_timer_set(peer->connection); } else { peer->last_reset = PEER_DOWN_AF_ACTIVATE; @@ -3358,7 +3358,7 @@ int peer_group_bind(struct bgp *bgp, union sockunion *su, struct peer *peer, } /* Set up peer's events and timers. */ - if (peer_active(peer)) + if (peer_active(peer->connection)) bgp_timer_set(peer->connection); } @@ -4599,9 +4599,11 @@ bool bgp_path_attribute_treat_as_withdraw(struct peer *peer, char *buf, } /* If peer is configured at least one address family return 1. */ -bool peer_active(struct peer *peer) +bool peer_active(struct peer_connection *connection) { - if (BGP_CONNECTION_SU_UNSPEC(peer->connection)) + struct peer *peer = connection->peer; + + if (BGP_CONNECTION_SU_UNSPEC(connection)) return false; if (peer->bfd_config) { @@ -6296,7 +6298,7 @@ int peer_timers_connect_set(struct peer *peer, uint32_t connect) /* Skip peer-group mechanics for regular peers. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { if (!peer_established(peer->connection)) { - if (peer_active(peer)) + if (peer_active(peer->connection)) BGP_EVENT_ADD(peer->connection, BGP_Stop); BGP_EVENT_ADD(peer->connection, BGP_Start); } @@ -6317,7 +6319,7 @@ int peer_timers_connect_set(struct peer *peer, uint32_t connect) member->v_connect = connect; if (!peer_established(member->connection)) { - if (peer_active(member)) + if (peer_active(member->connection)) BGP_EVENT_ADD(member->connection, BGP_Stop); BGP_EVENT_ADD(member->connection, BGP_Start); } @@ -6350,7 +6352,7 @@ int peer_timers_connect_unset(struct peer *peer) /* Skip peer-group mechanics for regular peers. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { if (!peer_established(peer->connection)) { - if (peer_active(peer)) + if (peer_active(peer->connection)) BGP_EVENT_ADD(peer->connection, BGP_Stop); BGP_EVENT_ADD(peer->connection, BGP_Start); } @@ -6371,7 +6373,7 @@ int peer_timers_connect_unset(struct peer *peer) member->v_connect = peer->bgp->default_connect_retry; if (!peer_established(member->connection)) { - if (peer_active(member)) + if (peer_active(member->connection)) BGP_EVENT_ADD(member->connection, BGP_Stop); BGP_EVENT_ADD(member->connection, BGP_Start); } @@ -8646,8 +8648,7 @@ static int peer_unshut_after_cfg(struct bgp *bgp) peer->host); peer->shut_during_cfg = false; - if (peer_active(peer) && - peer->connection->status != Established) { + if (peer_active(peer->connection) && peer->connection->status != Established) { if (peer->connection->status != Idle) BGP_EVENT_ADD(peer->connection, BGP_Stop); BGP_EVENT_ADD(peer->connection, BGP_Start); diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 2b6921b69503..df55d879e71d 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -2295,7 +2295,7 @@ extern struct peer *peer_unlock_with_caller(const char *, struct peer *); extern enum bgp_peer_sort peer_sort(struct peer *peer); extern enum bgp_peer_sort peer_sort_lookup(struct peer *peer); -extern bool peer_active(struct peer *); +extern bool peer_active(struct peer_connection *connection); extern bool peer_active_nego(struct peer *); extern bool peer_afc_received(struct peer *peer); extern bool peer_afc_advertised(struct peer *peer);