mirror of
https://git.proxmox.com/git/mirror_frr
synced 2025-05-29 07:59:35 +00:00
[bgpd] reference count the BGP instance
When a BGP instance is deleted with lots of routes and neighbors it is possible for the peer rsclient queue to run after bgp_delete has been called. This would lead to bgpd crashing, see https://bugzilla.vyatta.com/show_bug.cgi?id=3436 The fix is to add reference counting to the BGP instance and defer actual freeing until all references are gone. This patch also fixes a memory leak where the self-reference peer instance was being created but never freed. The check in bgp_clear_route is no longer valid because it is possible for it to be called when peer is in Deleted state during cleanup.
This commit is contained in:
parent
dde7258666
commit
0088b5dc55
@ -1562,6 +1562,7 @@ bgp_processq_del (struct work_queue *wq, void *data)
|
||||
{
|
||||
struct bgp_process_queue *pq = data;
|
||||
|
||||
bgp_unlock(pq->bgp);
|
||||
bgp_unlock_node (pq->rn);
|
||||
XFREE (MTYPE_BGP_PROCESS_QUEUE, pq);
|
||||
}
|
||||
@ -1611,6 +1612,7 @@ bgp_process (struct bgp *bgp, struct bgp_node *rn, afi_t afi, safi_t safi)
|
||||
|
||||
pqnode->rn = bgp_lock_node (rn); /* unlocked by bgp_processq_del */
|
||||
pqnode->bgp = bgp;
|
||||
bgp_lock(bgp);
|
||||
pqnode->afi = afi;
|
||||
pqnode->safi = safi;
|
||||
|
||||
@ -2843,19 +2845,6 @@ bgp_clear_route (struct peer *peer, afi_t afi, safi_t safi)
|
||||
*/
|
||||
if (!peer->clear_node_queue->thread)
|
||||
bgp_clear_node_complete (peer->clear_node_queue);
|
||||
else
|
||||
{
|
||||
/* clearing queue scheduled. Normal if in Established state
|
||||
* (and about to transition out of it), but otherwise...
|
||||
*/
|
||||
if (peer->status != Established)
|
||||
{
|
||||
plog_err (peer->log, "%s [Error] State %s is not Established,"
|
||||
" but routes were cleared - bug!",
|
||||
peer->host, LOOKUP (bgp_status_msg, peer->status));
|
||||
assert (peer->status == Established);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void
|
||||
|
45
bgpd/bgpd.c
45
bgpd/bgpd.c
@ -688,7 +688,9 @@ static inline void
|
||||
peer_free (struct peer *peer)
|
||||
{
|
||||
assert (peer->status == Deleted);
|
||||
|
||||
|
||||
bgp_unlock(peer->bgp);
|
||||
|
||||
/* this /ought/ to have been done already through bgp_stop earlier,
|
||||
* but just to be sure..
|
||||
*/
|
||||
@ -791,6 +793,7 @@ peer_new (struct bgp *bgp)
|
||||
peer->password = NULL;
|
||||
peer->bgp = bgp;
|
||||
peer = peer_lock (peer); /* initial reference */
|
||||
bgp_lock (bgp);
|
||||
|
||||
/* Set default flags. */
|
||||
for (afi = AFI_IP; afi < AFI_MAX; afi++)
|
||||
@ -1909,6 +1912,7 @@ bgp_create (as_t *as, const char *name)
|
||||
if ( (bgp = XCALLOC (MTYPE_BGP, sizeof (struct bgp))) == NULL)
|
||||
return NULL;
|
||||
|
||||
bgp_lock (bgp);
|
||||
bgp->peer_self = peer_new (bgp);
|
||||
bgp->peer_self->host = strdup ("Static announcement");
|
||||
|
||||
@ -2045,7 +2049,6 @@ bgp_delete (struct bgp *bgp)
|
||||
struct listnode *node;
|
||||
struct listnode *next;
|
||||
afi_t afi;
|
||||
safi_t safi;
|
||||
int i;
|
||||
|
||||
/* Delete static route. */
|
||||
@ -2059,14 +2062,46 @@ bgp_delete (struct bgp *bgp)
|
||||
|
||||
for (ALL_LIST_ELEMENTS (bgp->group, node, next, group))
|
||||
peer_group_delete (group);
|
||||
list_delete (bgp->group);
|
||||
|
||||
for (ALL_LIST_ELEMENTS (bgp->peer, node, next, peer))
|
||||
peer_delete (peer);
|
||||
list_delete (bgp->peer);
|
||||
|
||||
for (ALL_LIST_ELEMENTS (bgp->rsclient, node, next, peer))
|
||||
peer_delete (peer);
|
||||
|
||||
if (bgp->peer_self) {
|
||||
peer_delete(bgp->peer_self);
|
||||
bgp->peer_self = NULL;
|
||||
}
|
||||
|
||||
bgp_unlock(bgp); /* initial reference */
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
static void bgp_free (struct bgp *);
|
||||
|
||||
void
|
||||
bgp_lock (struct bgp *bgp)
|
||||
{
|
||||
++bgp->lock;
|
||||
}
|
||||
|
||||
void
|
||||
bgp_unlock(struct bgp *bgp)
|
||||
{
|
||||
if (--bgp->lock == 0)
|
||||
bgp_free (bgp);
|
||||
}
|
||||
|
||||
static void
|
||||
bgp_free (struct bgp *bgp)
|
||||
{
|
||||
afi_t afi;
|
||||
safi_t safi;
|
||||
|
||||
list_delete (bgp->group);
|
||||
list_delete (bgp->peer);
|
||||
list_delete (bgp->rsclient);
|
||||
|
||||
listnode_delete (bm->bgp, bgp);
|
||||
@ -2085,8 +2120,6 @@ bgp_delete (struct bgp *bgp)
|
||||
XFREE (MTYPE_ROUTE_TABLE,bgp->rib[afi][safi]);
|
||||
}
|
||||
XFREE (MTYPE_BGP, bgp);
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
struct peer *
|
||||
|
@ -70,6 +70,9 @@ struct bgp
|
||||
/* Name of this BGP instance. */
|
||||
char *name;
|
||||
|
||||
/* Reference count to allow peer_delete to finish after bgp_delete */
|
||||
int lock;
|
||||
|
||||
/* Self peer. */
|
||||
struct peer *peer_self;
|
||||
|
||||
@ -843,6 +846,9 @@ extern int bgp_flag_set (struct bgp *, int);
|
||||
extern int bgp_flag_unset (struct bgp *, int);
|
||||
extern int bgp_flag_check (struct bgp *, int);
|
||||
|
||||
extern void bgp_lock (struct bgp *);
|
||||
extern void bgp_unlock (struct bgp *);
|
||||
|
||||
extern int bgp_router_id_set (struct bgp *, struct in_addr *);
|
||||
|
||||
extern int bgp_cluster_id_set (struct bgp *, struct in_addr *);
|
||||
|
Loading…
Reference in New Issue
Block a user