bgpd: Withdraw routes without waiting for the coalescing timer to expire

TL;DR; BGP keeps advertising prefixes which were already removed via `network`
command.

This was happening randomly, but with this patch, no issues happened.

Before:

```
frr# sh ip bgp neighbors 10.0.5.1 advertised-routes
BGP table version is 586, local router ID is 44.44.33.43, vrf id 0
Default local pref 100, local AS 2
Status codes: s suppressed, d damped, h history, * valid, > best, = multipath,
i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes: i - IGP, e - EGP, ? - incomplete
RPKI validation codes: V valid, I invalid, N Not found

Network Next Hop Metric LocPrf Weight Path
*> 15.15.15.0/24 0.0.0.0 0 32768 2 i ============> stale entry
*> 15.15.16.0/24 0.0.0.0 0 32768 2 i
*> 15.15.17.0/24 0.0.0.0 0 32768 2 i

frr# sh ip bgp 15.15.15.0/24
% Network not in table
```

Logs:

```
root@b03d1542-0215-5ecf-013d-af2a7150a4f5:~# cat /log/syslog/frr.log | grep 15.15.15.0
2024/12/08 07:56:58 BGP: [K423X-ETGCQ] group_announce_route_walkcb: afi=IPv4, safi=unicast, p=15.15.15.0/24
2024/12/08 07:56:58 BGP: [T5JFA-13199] subgroup_process_announce_selected: p=15.15.15.0/24, selected=0xde4060
2024/12/08 07:56:58 BGP: [K423X-ETGCQ] group_announce_route_walkcb: afi=IPv4, safi=unicast, p=15.15.15.0/24
2024/12/08 07:56:58 BGP: [T5JFA-13199] subgroup_process_announce_selected: p=15.15.15.0/24, selected=0xde4060
2024/12/08 07:56:58 BGP: [HVRWP-5R9NQ] u78:s77 send UPDATE 15.15.15.0/24 IPv4 unicast
```

Or imagine the situation where withdrawals are postponed for 20-30s., blackholing
happens.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

1 files changed, 49 insertions, 46 deletions

diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c
index a1bf9a4c61..8f816eb30d 100644
--- a/bgpd/bgp_updgrp_adv.c
+++ b/bgpd/bgp_updgrp_adv.c
@@ -228,64 +228,67 @@ static int group_announce_route_walkcb(struct update_group *updgrp, void *arg)
 			   afi2str(afi), safi2str(safi), ctx->dest);
 
 	UPDGRP_FOREACH_SUBGRP (updgrp, subgrp) {
-		/* withdraw stale addpath without waiting for the coalesce timer timeout.
-		 * Otherwise, since adj->addpath_tx_id is overwritten, the code never
-		 * notice anymore it has to do a withdrawal.
-		 */
-		if (addpath_capable)
-			subgrp_withdraw_stale_addpath(ctx, subgrp);
-		/*
-		 * Skip the subgroups that have coalesce timer running. We will
-		 * walk the entire prefix table for those subgroups when the
-		 * coalesce timer fires.
-		 */
-		if (!subgrp->t_coalesce) {
-
-			/* An update-group that uses addpath */
-			if (addpath_capable) {
-				subgrp_announce_addpath_best_selected(ctx->dest,
-								      subgrp);
+		/* An update-group that uses addpath */
+		if (addpath_capable) {
+			/* Send withdrawals without waiting for coalesting timer
+			 * to expire.
+			 */
+			if (subgrp->t_coalesce) {
+				subgrp_withdraw_stale_addpath(ctx, subgrp);
 
-				/* Process the bestpath last so the "show [ip]
-				 * bgp neighbor x.x.x.x advertised"
-				 * output shows the attributes from the bestpath
-				 */
-				if (ctx->pi)
-					subgroup_process_announce_selected(
-						subgrp, ctx->pi, ctx->dest, afi,
-						safi,
-						bgp_addpath_id_for_peer(
-							peer, afi, safi,
-							&ctx->pi->tx_addpath));
+				goto done;
 			}
-			/* An update-group that does not use addpath */
-			else {
-				if (ctx->pi) {
-					subgroup_process_announce_selected(
-						subgrp, ctx->pi, ctx->dest, afi,
-						safi,
-						bgp_addpath_id_for_peer(
-							peer, afi, safi,
-							&ctx->pi->tx_addpath));
-				} else {
-					/* Find the addpath_tx_id of the path we
-					 * had advertised and
-					 * send a withdraw */
-					RB_FOREACH_SAFE (adj, bgp_adj_out_rb,
-							 &ctx->dest->adj_out,
+
+			subgrp_withdraw_stale_addpath(ctx, subgrp);
+			subgrp_announce_addpath_best_selected(ctx->dest, subgrp);
+
+			/* Process the bestpath last so the
+			 * "show [ip] bgp neighbor x.x.x.x advertised" output shows
+			 * the attributes from the bestpath.
+			 */
+			if (ctx->pi)
+				subgroup_process_announce_selected(
+					subgrp, ctx->pi, ctx->dest, afi, safi,
+					bgp_addpath_id_for_peer(peer, afi, safi,
+								&ctx->pi->tx_addpath));
+		} else {
+			/* Send withdrawals without waiting for coalesting timer
+			 * to expire.
+			 */
+			if (subgrp->t_coalesce) {
+				if (!ctx->pi || CHECK_FLAG(ctx->pi->flags, BGP_PATH_UNUSEABLE)) {
+					RB_FOREACH_SAFE (adj, bgp_adj_out_rb, &ctx->dest->adj_out,
 							 adj_next) {
 						if (adj->subgroup == subgrp) {
 							subgroup_process_announce_selected(
-								subgrp, NULL,
-								ctx->dest, afi,
-								safi,
+								subgrp, NULL, ctx->dest, afi, safi,
 								adj->addpath_tx_id);
 						}
 					}
 				}
+
+				goto done;
+			}
+
+			if (ctx->pi) {
+				subgroup_process_announce_selected(
+					subgrp, ctx->pi, ctx->dest, afi, safi,
+					bgp_addpath_id_for_peer(peer, afi, safi,
+								&ctx->pi->tx_addpath));
+			} else {
+				RB_FOREACH_SAFE (adj, bgp_adj_out_rb, &ctx->dest->adj_out,
+						 adj_next) {
+					if (adj->subgroup == subgrp) {
+						subgroup_process_announce_selected(subgrp, NULL,
+										   ctx->dest, afi,
+										   safi,
+										   adj->addpath_tx_id);
+					}
+				}
 			}
 		}
 
+done:
 		/* Notify BGP Conditional advertisement */
 		bgp_notify_conditional_adv_scanner(subgrp);
 	}