Skip to content

Commit

Permalink
bgpd: Withdraw routes without waiting for the coalescing timer to expire
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
ton31337 committed Dec 18, 2024
1 parent 6c3e1e4 commit 92c92ae
Showing 1 changed file with 49 additions and 46 deletions.
95 changes: 49 additions & 46 deletions bgpd/bgp_updgrp_adv.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit 92c92ae

Please sign in to comment.