From e1a32ec1c5a4468a8ab7f7d92febfdbebe475d79 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 1 Oct 2021 10:32:57 -0400 Subject: [PATCH 1/2] bgpd: bgp_announce_route should know if we should force the update or not When calling bgp_announce_route allow it to properly set the flag to force an update to go out or not. Signed-off-by: Donald Sharp --- bgpd/bgp_packet.c | 8 +++++--- bgpd/bgp_route.c | 14 ++++++++++---- bgpd/bgp_route.h | 3 ++- bgpd/bgpd.c | 17 +++++++++-------- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index bb2dbc9427..de9a89523b 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -1961,6 +1961,7 @@ static int bgp_route_refresh_receive(struct peer *peer, bgp_size_t size) struct update_group *updgrp; struct peer *updgrp_peer; uint8_t subtype; + bool force_update = false; bgp_size_t msg_length = size - (BGP_MSG_ROUTE_REFRESH_MIN_SIZE - BGP_HEADER_SIZE); @@ -2222,7 +2223,7 @@ static int bgp_route_refresh_receive(struct peer *peer, bgp_size_t size) /* Avoid supressing duplicate routes later * when processing in subgroup_announce_table(). */ - SET_FLAG(paf->subgroup->sflags, SUBGRP_STATUS_FORCE_UPDATES); + force_update = true; /* If the peer is configured for default-originate clear the * SUBGRP_STATUS_DEFAULT_ORIGINATE flag so that we will @@ -2354,7 +2355,7 @@ static int bgp_route_refresh_receive(struct peer *peer, bgp_size_t size) } /* Perform route refreshment to the peer */ - bgp_announce_route(peer, afi, safi); + bgp_announce_route(peer, afi, safi, force_update); /* No FSM action necessary */ return BGP_PACKET_NOOP; @@ -2457,7 +2458,8 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt, peer->afc_recv[afi][safi] = 1; if (peer->afc[afi][safi]) { peer->afc_nego[afi][safi] = 1; - bgp_announce_route(peer, afi, safi); + bgp_announce_route(peer, afi, safi, + false); } } else { peer->afc_recv[afi][safi] = 0; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index fc97178450..5bc1b5a40a 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -4588,8 +4588,11 @@ static int bgp_announce_route_timer_expired(struct thread *t) * bgp_announce_route * * *Triggers* announcement of routes of a given AFI/SAFI to a peer. + * + * if force is true we will force an update even if the update + * limiting code is attempted to kick in. */ -void bgp_announce_route(struct peer *peer, afi_t afi, safi_t safi) +void bgp_announce_route(struct peer *peer, afi_t afi, safi_t safi, bool force) { struct peer_af *paf; struct update_subgroup *subgrp; @@ -4606,6 +4609,9 @@ void bgp_announce_route(struct peer *peer, afi_t afi, safi_t safi) if (!subgrp || paf->t_announce_route) return; + if (force) + SET_FLAG(subgrp->sflags, SUBGRP_STATUS_FORCE_UPDATES); + /* * Start a timer to stagger/delay the announce. This serves * two purposes - announcement can potentially be combined for @@ -4634,7 +4640,7 @@ void bgp_announce_route_all(struct peer *peer) safi_t safi; FOREACH_AFI_SAFI (afi, safi) - bgp_announce_route(peer, afi, safi); + bgp_announce_route(peer, afi, safi, false); } /* Flag or unflag bgp_dest to determine whether it should be treated by @@ -4772,7 +4778,7 @@ static int bgp_soft_reconfig_table_task(struct thread *thread) table->soft_reconfig_peers, peer); bgp_announce_route(peer, table->afi, - table->safi); + table->safi, false); if (list_isempty( table->soft_reconfig_peers)) { list_delete( @@ -4800,7 +4806,7 @@ static int bgp_soft_reconfig_table_task(struct thread *thread) */ for (ALL_LIST_ELEMENTS(table->soft_reconfig_peers, node, nnode, peer)) { listnode_delete(table->soft_reconfig_peers, peer); - bgp_announce_route(peer, table->afi, table->safi); + bgp_announce_route(peer, table->afi, table->safi, false); } list_delete(&table->soft_reconfig_peers); diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index 7609f7196d..37bf675b67 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -608,7 +608,8 @@ extern void bgp_process_queue_init(struct bgp *bgp); extern void bgp_route_init(void); extern void bgp_route_finish(void); extern void bgp_cleanup_routes(struct bgp *); -extern void bgp_announce_route(struct peer *, afi_t, safi_t); +extern void bgp_announce_route(struct peer *peer, afi_t afi, safi_t safi, + bool force); extern void bgp_stop_announce_route_timer(struct peer_af *paf); extern void bgp_announce_route_all(struct peer *); extern void bgp_default_originate(struct peer *, afi_t, safi_t, int); diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 925af80cb7..bdf646f603 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -2156,7 +2156,8 @@ static int peer_activate_af(struct peer *peer, afi_t afi, safi_t safi) CAPABILITY_ACTION_SET); if (peer->afc_recv[afi][safi]) { peer->afc_nego[afi][safi] = 1; - bgp_announce_route(peer, afi, safi); + bgp_announce_route(peer, afi, safi, + false); } } else { peer->last_reset = PEER_DOWN_AF_ACTIVATE; @@ -4142,7 +4143,7 @@ void peer_change_action(struct peer *peer, afi_t afi, safi_t safi, SUBGRP_STATUS_FORCE_UPDATES); update_group_adjust_peer(paf); - bgp_announce_route(peer, afi, safi); + bgp_announce_route(peer, afi, safi, false); } } @@ -5058,7 +5059,7 @@ int peer_default_originate_set(struct peer *peer, afi_t afi, safi_t safi, if (peer_established(peer) && peer->afc_nego[afi][safi]) { update_group_adjust_peer(peer_af_find(peer, afi, safi)); bgp_default_originate(peer, afi, safi, 0); - bgp_announce_route(peer, afi, safi); + bgp_announce_route(peer, afi, safi, false); } /* Skip peer-group mechanics for regular peers. */ @@ -5095,7 +5096,7 @@ int peer_default_originate_set(struct peer *peer, afi_t afi, safi_t safi, update_group_adjust_peer( peer_af_find(member, afi, safi)); bgp_default_originate(member, afi, safi, 0); - bgp_announce_route(member, afi, safi); + bgp_announce_route(member, afi, safi, false); } } @@ -5134,7 +5135,7 @@ int peer_default_originate_unset(struct peer *peer, afi_t afi, safi_t safi) if (peer_established(peer) && peer->afc_nego[afi][safi]) { update_group_adjust_peer(peer_af_find(peer, afi, safi)); bgp_default_originate(peer, afi, safi, 1); - bgp_announce_route(peer, afi, safi); + bgp_announce_route(peer, afi, safi, false); } /* Skip peer-group mechanics for regular peers. */ @@ -5165,7 +5166,7 @@ int peer_default_originate_unset(struct peer *peer, afi_t afi, safi_t safi) if (peer_established(member) && member->afc_nego[afi][safi]) { update_group_adjust_peer(peer_af_find(member, afi, safi)); bgp_default_originate(member, afi, safi, 1); - bgp_announce_route(member, afi, safi); + bgp_announce_route(member, afi, safi, false); } } @@ -5213,7 +5214,7 @@ static void peer_on_policy_change(struct peer *peer, afi_t afi, safi_t safi, if (outbound) { update_group_adjust_peer(peer_af_find(peer, afi, safi)); if (peer_established(peer)) - bgp_announce_route(peer, afi, safi); + bgp_announce_route(peer, afi, safi, false); } else { if (!peer_established(peer)) return; @@ -7480,7 +7481,7 @@ int peer_clear_soft(struct peer *peer, afi_t afi, safi_t safi, UNSET_FLAG(paf->subgroup->sflags, SUBGRP_STATUS_DEFAULT_ORIGINATE); - bgp_announce_route(peer, afi, safi); + bgp_announce_route(peer, afi, safi, false); } if (stype == BGP_CLEAR_SOFT_IN_ORF_PREFIX) { From f3d20a2aa5746ad4b00a53b04df4fd863d8c49b4 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 29 Sep 2021 11:42:19 -0400 Subject: [PATCH 2/2] bgpd: When removing v6 address being used as a nexthop ensure peer is reset With v6 interface based peering, we send the global as well as the LL address as nexthops to the peer. When either of these were removed on the interface we were not necessarily resetting the connection. Leaving bgp in a state where the peer had reachability for addresses that are no longer in use. Modify the code that when we receive an interface address deletion event. Check to see that we are using the v6 address as nexthops for that peer and if so, tell it to reset. I initially struggled with a hard reset of the peer or a clear but choose to follow other places in the code that we noticed address changes that resulted in hard resets. Ticket: #2799568 Signed-off-by: Donald Sharp --- bgpd/bgp_zebra.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 2a67bb2f8c..09fe399c29 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -338,8 +338,11 @@ static int bgp_interface_address_add(ZAPI_CALLBACK_ARGS) static int bgp_interface_address_delete(ZAPI_CALLBACK_ARGS) { + struct listnode *node, *nnode; struct connected *ifc; + struct peer *peer; struct bgp *bgp; + struct prefix *addr; bgp = bgp_lookup_by_vrf_id(vrf_id); @@ -356,6 +359,35 @@ static int bgp_interface_address_delete(ZAPI_CALLBACK_ARGS) bgp_connected_delete(bgp, ifc); } + addr = ifc->address; + + if (bgp) { + /* + * When we are using the v6 global as part of the peering + * nexthops and we are removing it, then we need to + * clear the peer data saved for that nexthop and + * cause a re-announcement of the route. Since + * we do not want the peering to bounce. + */ + for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { + afi_t afi; + safi_t safi; + + if (addr->family == AF_INET) + continue; + + if (!IN6_IS_ADDR_LINKLOCAL(&addr->u.prefix6) + && memcmp(&peer->nexthop.v6_global, + &addr->u.prefix6, 16) + == 0) { + memset(&peer->nexthop.v6_global, 0, 16); + FOREACH_AFI_SAFI (afi, safi) + bgp_announce_route(peer, afi, safi, + true); + } + } + } + connected_free(&ifc); return 0;