proxmox-mirrors/mirror_frr
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_frr synced 2025-08-03 13:01:59 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

BGP: crash in update_subgroup_merge()

Browse Source 
Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by:   Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by:   Vivek Venkatraman <vivek@cumulusnetworks.com>

Ticket: CM-8191

On my hard node I have a route to 10.0.0.0/22 via eth0, I then learn
10.0.0.8/32 from peer 10.0.0.8 with a nexthop of 10.0.0.8:

superm-redxp-05# show ip bgp 10.0.0.8/32
BGP routing table entry for 10.0.0.8/32
Paths: (1 available, no best path)
  Not advertised to any peer
  80
    10.0.0.8 from r8(swp6) (10.0.0.8)
      Origin IGP, metric 0, localpref 100, valid, external
      AddPath ID: RX 0, TX 9
      Last update: Thu Nov 12 14:00:00 2015

superm-redxp-05#

I do a lookup for the nexthop and see that 10.0.0.8 is reachable via my
eth0 10.0.0.22 so I select a bestpath and install the route.  At this
point my route to 10.0.0.8 is a /32 that resolves via itself, NHT sees
that this is illegal and flags the nexthop as inaccessible.

superm-redxp-05# show ip bgp 10.0.0.8/32
BGP routing table entry for 10.0.0.8/32
Paths: (1 available, best #1, table Default-IP-Routing-Table)
  Advertised to non peer-group peers:
  r6(swp4) r7(swp5) r8(swp6) r2(10.1.2.2) r3(10.1.3.2) r4(10.1.4.2)
  80
    10.0.0.8 (inaccessible) from r8(swp6) (10.0.0.8)
      Origin IGP, metric 0, localpref 100, invalid, external, bestpath-from-AS 80, best
      AddPath ID: RX 0, TX 9
      Last update: Thu Nov 12 14:00:00 2015

superm-redxp-05#

at which point we withdraw the route, things churn, we relearn it and go
through the whole process over and over again.  We end up advertising and
withdrawing this route about 9 times a second!!

This exposed a crash in the update-group code where we try to merge two sub-groups
but the assert on adj_count fails because the timing worked out where one had
advertised 10.0.0.8/32 but the other had not.

NOTE: the race condition described above will be resolved via a separate patch.
This commit is contained in:
Daniel Walton 2015-11-17 02:09:57 +00:00
parent d44ca835fc
commit 7717b1837c
1 changed files with 4 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
bgpd/bgp_updgrp.c
View File

@ -998,7 +998,7 @@ update_subgroup_find (struct update_group *updgrp, struct peer_af *paf)
* Returns TRUE if this subgroup is in a state that allows it to be
* merged into another subgroup.
*/
static inline int
static int
update_subgroup_ready_for_merge (struct update_subgroup *subgrp)
{
@ -1032,7 +1032,7 @@ update_subgroup_ready_for_merge (struct update_subgroup *subgrp)
* Returns TRUE if the first subgroup can merge into the second
* subgroup.
*/
static inline int
static int
update_subgroup_can_merge_into (struct update_subgroup *subgrp,
struct update_subgroup *target)
{
@ -1051,22 +1051,10 @@ update_subgroup_can_merge_into (struct update_subgroup *subgrp,
CHECK_FLAG(target->sflags, SUBGRP_STATUS_DEFAULT_ORIGINATE))
return 0;
/*
* If there are any adv entries on the target, then its adj-out (the
* set of advertised routes) does not match that of the other
* subgrp, and we cannot merge the two.
*
* The adj-out is used when generating a route refresh to a peer in
* a subgroup. If it is not accurate, say it is missing an entry, we
* may miss sending a withdraw for an entry as part of a refresh.
*/
if (!advertise_list_is_empty (target))
if (subgrp->adj_count != target->adj_count)
return 0;
if (update_subgroup_needs_refresh (target))
return 0;
return 1;
return update_subgroup_ready_for_merge (target);
}
/*