mirror of
https://git.proxmox.com/git/mirror_frr
synced 2025-11-02 23:34:11 +00:00
bgpd: fix bgp_node locking issues
* bgpd: Connected table locks were being locked but not unlocked, such that
eventually a lock would exceed 2^31 and become negative, thus triggering
an assert later on.
* bgp_main.c: (bgp_exit) delete connected elements along with ifp's.
* bgp_nexthop.c: (bgp_nexthop_lookup{,_ipv6}) add missing unlocks
(bgp_multiaccess_check_v4) ditto
(bgp_connected_{add,delete}) Use a distinct memtype for bgp_connected_ref.
(bgp_scan_finish) reset the nexthop cache to clean it up when bgpd exits
* bgp_route.c: fix missing bgp_node unlocks
* lib/memtype.c: (memory_list_bgp) add MTYPE_BGP_CONN
* testing: has been tested for almost 2 months now.
This commit is contained in:
parent
cca85d27a5
commit
6c88b44dcb
@ -243,7 +243,15 @@ bgp_exit (int status)
|
||||
if (retain_mode)
|
||||
if_add_hook (IF_DELETE_HOOK, NULL);
|
||||
for (ALL_LIST_ELEMENTS (iflist, node, nnode, ifp))
|
||||
if_delete (ifp);
|
||||
{
|
||||
struct listnode *c_node, *c_nnode;
|
||||
struct connected *c;
|
||||
|
||||
for (ALL_LIST_ELEMENTS (ifp->connected, c_node, c_nnode, c))
|
||||
bgp_connected_delete (c);
|
||||
|
||||
if_delete (ifp);
|
||||
}
|
||||
list_free (iflist);
|
||||
|
||||
/* reverse bgp_attr_init */
|
||||
|
||||
@ -276,6 +276,8 @@ bgp_nexthop_lookup_ipv6 (struct peer *peer, struct bgp_info *ri, int *changed,
|
||||
|
||||
if (bnc->metric != oldbnc->metric)
|
||||
bnc->metricchanged = 1;
|
||||
|
||||
bgp_unlock_node (oldrn);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -365,6 +367,8 @@ bgp_nexthop_lookup (afi_t afi, struct peer *peer, struct bgp_info *ri,
|
||||
|
||||
if (bnc->metric != oldbnc->metric)
|
||||
bnc->metricchanged = 1;
|
||||
|
||||
bgp_unlock_node (oldrn);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -571,7 +575,7 @@ bgp_connected_add (struct connected *ifc)
|
||||
}
|
||||
else
|
||||
{
|
||||
bc = XCALLOC (0, sizeof (struct bgp_connected_ref));
|
||||
bc = XCALLOC (MTYPE_BGP_CONN, sizeof (struct bgp_connected_ref));
|
||||
bc->refcnt = 1;
|
||||
rn->info = bc;
|
||||
}
|
||||
@ -596,7 +600,7 @@ bgp_connected_add (struct connected *ifc)
|
||||
}
|
||||
else
|
||||
{
|
||||
bc = XCALLOC (0, sizeof (struct bgp_connected_ref));
|
||||
bc = XCALLOC (MTYPE_BGP_CONN, sizeof (struct bgp_connected_ref));
|
||||
bc->refcnt = 1;
|
||||
rn->info = bc;
|
||||
}
|
||||
@ -636,7 +640,7 @@ bgp_connected_delete (struct connected *ifc)
|
||||
bc->refcnt--;
|
||||
if (bc->refcnt == 0)
|
||||
{
|
||||
XFREE (0, bc);
|
||||
XFREE (MTYPE_BGP_CONN, bc);
|
||||
rn->info = NULL;
|
||||
}
|
||||
bgp_unlock_node (rn);
|
||||
@ -662,7 +666,7 @@ bgp_connected_delete (struct connected *ifc)
|
||||
bc->refcnt--;
|
||||
if (bc->refcnt == 0)
|
||||
{
|
||||
XFREE (0, bc);
|
||||
XFREE (MTYPE_BGP_CONN, bc);
|
||||
rn->info = NULL;
|
||||
}
|
||||
bgp_unlock_node (rn);
|
||||
@ -1136,11 +1140,15 @@ bgp_multiaccess_check_v4 (struct in_addr nexthop, char *peer)
|
||||
rn1 = bgp_node_match (bgp_connected_table[AFI_IP], &p1);
|
||||
if (! rn1)
|
||||
return 0;
|
||||
bgp_unlock_node (rn1);
|
||||
|
||||
rn2 = bgp_node_match (bgp_connected_table[AFI_IP], &p2);
|
||||
if (! rn2)
|
||||
return 0;
|
||||
bgp_unlock_node (rn2);
|
||||
|
||||
/* This is safe, even with above unlocks, since we are just
|
||||
comparing pointers to the objects, not the objects themselves. */
|
||||
if (rn1 == rn2)
|
||||
return 1;
|
||||
|
||||
@ -1316,6 +1324,9 @@ bgp_scan_init (void)
|
||||
void
|
||||
bgp_scan_finish (void)
|
||||
{
|
||||
/* Only the current one needs to be reset. */
|
||||
bgp_nexthop_cache_reset (bgp_nexthop_cache_table[AFI_IP]);
|
||||
|
||||
bgp_table_unlock (cache1_table[AFI_IP]);
|
||||
cache1_table[AFI_IP] = NULL;
|
||||
|
||||
@ -1326,6 +1337,9 @@ bgp_scan_finish (void)
|
||||
bgp_connected_table[AFI_IP] = NULL;
|
||||
|
||||
#ifdef HAVE_IPV6
|
||||
/* Only the current one needs to be reset. */
|
||||
bgp_nexthop_cache_reset (bgp_nexthop_cache_table[AFI_IP6]);
|
||||
|
||||
bgp_table_unlock (cache1_table[AFI_IP6]);
|
||||
cache1_table[AFI_IP6] = NULL;
|
||||
|
||||
|
||||
@ -6561,7 +6561,10 @@ bgp_show_route_in_table (struct vty *vty, struct bgp *bgp,
|
||||
if ((rm = bgp_node_match (table, &match)) != NULL)
|
||||
{
|
||||
if (prefix_check && rm->p.prefixlen != match.prefixlen)
|
||||
continue;
|
||||
{
|
||||
bgp_unlock_node (rm);
|
||||
continue;
|
||||
}
|
||||
|
||||
for (ri = rm->info; ri; ri = ri->next)
|
||||
{
|
||||
@ -6575,6 +6578,8 @@ bgp_show_route_in_table (struct vty *vty, struct bgp *bgp,
|
||||
display++;
|
||||
route_vty_out_detail (vty, bgp, &rm->p, ri, AFI_IP, SAFI_MPLS_VPN);
|
||||
}
|
||||
|
||||
bgp_unlock_node (rm);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -6598,6 +6603,8 @@ bgp_show_route_in_table (struct vty *vty, struct bgp *bgp,
|
||||
route_vty_out_detail (vty, bgp, &rn->p, ri, afi, safi);
|
||||
}
|
||||
}
|
||||
|
||||
bgp_unlock_node (rn);
|
||||
}
|
||||
}
|
||||
|
||||
@ -11355,41 +11362,49 @@ bgp_clear_damp_route (struct vty *vty, const char *view_name,
|
||||
|
||||
if ((table = rn->info) != NULL)
|
||||
if ((rm = bgp_node_match (table, &match)) != NULL)
|
||||
if (! prefix_check || rm->p.prefixlen == match.prefixlen)
|
||||
{
|
||||
ri = rm->info;
|
||||
while (ri)
|
||||
{
|
||||
if (ri->extra && ri->extra->damp_info)
|
||||
{
|
||||
ri_temp = ri->next;
|
||||
bgp_damp_info_free (ri->extra->damp_info, 1);
|
||||
ri = ri_temp;
|
||||
}
|
||||
else
|
||||
ri = ri->next;
|
||||
}
|
||||
}
|
||||
{
|
||||
if (! prefix_check || rm->p.prefixlen == match.prefixlen)
|
||||
{
|
||||
ri = rm->info;
|
||||
while (ri)
|
||||
{
|
||||
if (ri->extra && ri->extra->damp_info)
|
||||
{
|
||||
ri_temp = ri->next;
|
||||
bgp_damp_info_free (ri->extra->damp_info, 1);
|
||||
ri = ri_temp;
|
||||
}
|
||||
else
|
||||
ri = ri->next;
|
||||
}
|
||||
}
|
||||
|
||||
bgp_unlock_node (rm);
|
||||
}
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
if ((rn = bgp_node_match (bgp->rib[afi][safi], &match)) != NULL)
|
||||
if (! prefix_check || rn->p.prefixlen == match.prefixlen)
|
||||
{
|
||||
ri = rn->info;
|
||||
while (ri)
|
||||
{
|
||||
if (ri->extra && ri->extra->damp_info)
|
||||
{
|
||||
ri_temp = ri->next;
|
||||
bgp_damp_info_free (ri->extra->damp_info, 1);
|
||||
ri = ri_temp;
|
||||
}
|
||||
else
|
||||
ri = ri->next;
|
||||
}
|
||||
}
|
||||
{
|
||||
if (! prefix_check || rn->p.prefixlen == match.prefixlen)
|
||||
{
|
||||
ri = rn->info;
|
||||
while (ri)
|
||||
{
|
||||
if (ri->extra && ri->extra->damp_info)
|
||||
{
|
||||
ri_temp = ri->next;
|
||||
bgp_damp_info_free (ri->extra->damp_info, 1);
|
||||
ri = ri_temp;
|
||||
}
|
||||
else
|
||||
ri = ri->next;
|
||||
}
|
||||
}
|
||||
|
||||
bgp_unlock_node (rn);
|
||||
}
|
||||
}
|
||||
|
||||
return CMD_SUCCESS;
|
||||
|
||||
@ -108,6 +108,7 @@ struct memory_list memory_list_bgp[] =
|
||||
{ MTYPE_BGP_NODE, "BGP node" },
|
||||
{ MTYPE_BGP_ROUTE, "BGP route" },
|
||||
{ MTYPE_BGP_ROUTE_EXTRA, "BGP ancillary route info" },
|
||||
{ MTYPE_BGP_CONN, "BGP connected" },
|
||||
{ MTYPE_BGP_STATIC, "BGP static" },
|
||||
{ MTYPE_BGP_ADVERTISE_ATTR, "BGP adv attr" },
|
||||
{ MTYPE_BGP_ADVERTISE, "BGP adv" },
|
||||
|
||||
Loading…
Reference in New Issue
Block a user