]> git.puffer.fish Git - mirror/frr.git/commitdiff
bgpd: Do not keep stale paths in Adj-RIB-Out if not addpath aware
authorDonatas Abraitis <donatas@opensourcerouting.org>
Thu, 27 Feb 2025 14:08:21 +0000 (16:08 +0200)
committerDonatas Abraitis <donatas@opensourcerouting.org>
Mon, 17 Mar 2025 14:02:16 +0000 (16:02 +0200)
```
munet> r1 shi vtysh -c 'show ip bgp update advertised-routes'
update group 1, subgroup 1
BGP table version is 5, local router ID is 192.168.137.1
Status codes:  s suppressed, d damped, h history, u unsorted, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Origin codes:  i - IGP, e - EGP, ? - incomplete
     Network          Next Hop            Metric LocPrf Weight Path
 *> 1.0.0.0/24       192.168.137.201                10      0 65200 65444 i
 *> 10.0.0.0/24      192.168.137.100                10      0 65100 65444 65444 i
 *> 10.65.10.0/24    192.168.137.100          0     10      0 65100 i
 *> 10.200.2.0/24    192.168.137.202          0     10      0 65200 i
```

Announce one more 10.0.0.0/24 via 65200 and we have TWO paths 10.0.0.0/24 in adj-rib-out:

```
munet> r1 shi vtysh -c 'show ip bgp update advertised-routes'
update group 1, subgroup 1
BGP table version is 6, local router ID is 192.168.137.1
Status codes:  s suppressed, d damped, h history, u unsorted, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Origin codes:  i - IGP, e - EGP, ? - incomplete
     Network          Next Hop            Metric LocPrf Weight Path
 *> 1.0.0.0/24       192.168.137.201                10      0 65200 65444 i
 *> 10.0.0.0/24      192.168.137.100                10      0 65100 65444 65444 i
 *> 10.0.0.0/24      192.168.137.201                10      0 65200 65444 i
 *> 10.65.10.0/24    192.168.137.100          0     10      0 65100 i
 *> 10.200.2.0/24    192.168.137.202          0     10      0 65200 i
```

Stop announcing 10.0.0.0/24 via 65200 and we still have TWO paths for 10.0.0.0/24...

```
munet> r1 shi vtysh -c 'show ip bgp update advertised-routes'
update group 1, subgroup 1
BGP table version is 7, local router ID is 192.168.137.1
Status codes:  s suppressed, d damped, h history, u unsorted, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Origin codes:  i - IGP, e - EGP, ? - incomplete
     Network          Next Hop            Metric LocPrf Weight Path
 *> 1.0.0.0/24       192.168.137.201                10      0 65200 65444 i
 *> 10.0.0.0/24      192.168.137.100                10      0 65100 65444 65444 i
 *> 10.0.0.0/24      192.168.137.201                10      0 65200 65444 i
 *> 10.65.10.0/24    192.168.137.100          0     10      0 65100 i
 *> 10.200.2.0/24    192.168.137.202          0     10      0 65200 i
```

Why do we need to keep old paths in adj-rib-out if we don't have e.g. AddPaths enabled?

Shouldn't it be like here? (only one 10.0.0.0/24 in adj-rib-out for this update-group instead of multiple (stale from previous announcements))

```
munet> r1 shi vtysh -c 'show ip bgp update advertised-routes'
update group 1, subgroup 1
BGP table version is 6, local router ID is 192.168.137.1
Status codes:  s suppressed, d damped, h history, u unsorted, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Origin codes:  i - IGP, e - EGP, ? - incomplete
     Network          Next Hop            Metric LocPrf Weight Path
 *> 1.0.0.0/24       192.168.137.201                10      0 65200 65444 i
 *> 10.0.0.0/24      192.168.137.201                10      0 65200 65444 i
 *> 10.65.10.0/24    192.168.137.100          0     10      0 65100 i
 *> 10.200.2.0/24    192.168.137.202          0     10      0 65200 i
```

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
bgpd/bgp_updgrp_adv.c

index 8f816eb30decf00cc1989349628abcbeb94a9324..f821a1453a3ba26f297681aa7adc4785c9424049 100644 (file)
@@ -275,6 +275,21 @@ static int group_announce_route_walkcb(struct update_group *updgrp, void *arg)
                                        subgrp, ctx->pi, ctx->dest, afi, safi,
                                        bgp_addpath_id_for_peer(peer, afi, safi,
                                                                &ctx->pi->tx_addpath));
+
+                               /* Remove paths from Adj-RIB-Out if it's not a best path.
+                                * Why should we keep Adj-RIB-Out with stale paths?
+                                */
+                               RB_FOREACH_SAFE (adj, bgp_adj_out_rb, &ctx->dest->adj_out,
+                                                adj_next) {
+                                       if (adj->subgroup != subgrp)
+                                               continue;
+
+                                       if (!adj->adv)
+                                               subgroup_process_announce_selected(subgrp, NULL,
+                                                                                  ctx->dest, afi,
+                                                                                  safi,
+                                                                                  adj->addpath_tx_id);
+                               }
                        } else {
                                RB_FOREACH_SAFE (adj, bgp_adj_out_rb, &ctx->dest->adj_out,
                                                 adj_next) {