Ede_Vau
/
frr
mirror of https://github.com/FRRouting/frr.git
1
1
Fork 0
Code Issues Releases Wiki Activity

bgpd: Don't try to recursively hold peer io mutex 

BGP was modified in a0b937de42
to grab the peer->io_mtx before validating the header to ensure
that the input Queue was not being modified by anyone else at that
moment in time.  Unfortunately validate_header can detect a problem
and attempt to relock the mutex, which deadlocks.  This deadlock in
the bgp_io pthread is the lone deadlock at first, eventually though
bgp attempts to write another packet to the peer( say when the
it's time to send the next packet ) and the main pthread of bgpd
becomes deadlocked and then the whole bgpd process is stuck at that
point in time leaving us dead in the water.

The point of locking the mutex earlier was to ensure that the input
Queue wasn't being modified by anyone else, (Say reading off it )
as that we wanted to ensure that we don't hold more packets then necessary.

Let's grab the mutex long enough to look at the input Q size, this
ensure that we have room and then we can validate_header and do the right
thing from there.  We'll need to lock the mutex when we actually move it
into the input Q as well.

Fixes: #12725
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
This commit is contained in:
Donald Sharp 2023-02-02 14:13:12 -05:00
parent 73fb874e0a
commit f1b1efdefc
1 changed files with 7 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
bgpd/bgp_io.c
View File

@ -173,12 +173,11 @@ static int read_ibuf_work(struct peer *peer)
uint16_t pktsize = 0;
struct stream *pkt;
/* Hold the I/O lock, we might not have space on the InQ */
frr_mutex_lock_autounlock(&peer->io_mtx);
/* ============================================== */
if (peer->ibuf->count >= bm->inq_limit)
return -ENOMEM;
frr_with_mutex (&peer->io_mtx) {
if (peer->ibuf->count >= bm->inq_limit)
return -ENOMEM;
}
/* check that we have enough data for a header */
if (ringbuf_remain(ibw) < BGP_HEADER_SIZE)
@ -211,7 +210,9 @@ static int read_ibuf_work(struct peer *peer)
stream_set_endp(pkt, pktsize);
frrtrace(2, frr_bgp, packet_read, peer, pkt);
stream_fifo_push(peer->ibuf, pkt);
frr_with_mutex (&peer->io_mtx) {
stream_fifo_push(peer->ibuf, pkt);
}
return pktsize;
}