Merge pull request #11926 from mjstapp/fix_bgp_io_race

bgpd: avoid notify race between io and main pthreads
This commit is contained in:
Donatas Abraitis 2022-09-12 18:24:18 +02:00 committed by GitHub
commit c3c5f18c2b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 38 additions and 13 deletions

View File

@ -37,7 +37,7 @@
#include "bgpd/bgp_debug.h" // for bgp_debug_neighbor_events, bgp_type_str #include "bgpd/bgp_debug.h" // for bgp_debug_neighbor_events, bgp_type_str
#include "bgpd/bgp_errors.h" // for expanded error reference information #include "bgpd/bgp_errors.h" // for expanded error reference information
#include "bgpd/bgp_fsm.h" // for BGP_EVENT_ADD, bgp_event #include "bgpd/bgp_fsm.h" // for BGP_EVENT_ADD, bgp_event
#include "bgpd/bgp_packet.h" // for bgp_notify_send_with_data, bgp_notify... #include "bgpd/bgp_packet.h" // for bgp_notify_io_invalid...
#include "bgpd/bgp_trace.h" // for frrtraces #include "bgpd/bgp_trace.h" // for frrtraces
#include "bgpd/bgpd.h" // for peer, BGP_MARKER_SIZE, bgp_master, bm #include "bgpd/bgpd.h" // for peer, BGP_MARKER_SIZE, bgp_master, bm
/* clang-format on */ /* clang-format on */
@ -526,8 +526,8 @@ static bool validate_header(struct peer *peer)
return false; return false;
if (memcmp(m_correct, m_rx, BGP_MARKER_SIZE) != 0) { if (memcmp(m_correct, m_rx, BGP_MARKER_SIZE) != 0) {
bgp_notify_send(peer, BGP_NOTIFY_HEADER_ERR, bgp_notify_io_invalid(peer, BGP_NOTIFY_HEADER_ERR,
BGP_NOTIFY_HEADER_NOT_SYNC); BGP_NOTIFY_HEADER_NOT_SYNC, NULL, 0);
return false; return false;
} }
@ -547,9 +547,8 @@ static bool validate_header(struct peer *peer)
zlog_debug("%s unknown message type 0x%02x", peer->host, zlog_debug("%s unknown message type 0x%02x", peer->host,
type); type);
bgp_notify_send_with_data(peer, BGP_NOTIFY_HEADER_ERR, bgp_notify_io_invalid(peer, BGP_NOTIFY_HEADER_ERR,
BGP_NOTIFY_HEADER_BAD_MESTYPE, &type, BGP_NOTIFY_HEADER_BAD_MESTYPE, &type, 1);
1);
return false; return false;
} }
@ -574,9 +573,9 @@ static bool validate_header(struct peer *peer)
uint16_t nsize = htons(size); uint16_t nsize = htons(size);
bgp_notify_send_with_data(peer, BGP_NOTIFY_HEADER_ERR, bgp_notify_io_invalid(peer, BGP_NOTIFY_HEADER_ERR,
BGP_NOTIFY_HEADER_BAD_MESLEN, BGP_NOTIFY_HEADER_BAD_MESLEN,
(unsigned char *)&nsize, 2); (unsigned char *)&nsize, 2);
return false; return false;
} }

View File

@ -871,8 +871,9 @@ bool bgp_notify_received_hard_reset(struct peer *peer, uint8_t code,
* @param data Data portion * @param data Data portion
* @param datalen length of data portion * @param datalen length of data portion
*/ */
void bgp_notify_send_with_data(struct peer *peer, uint8_t code, static void bgp_notify_send_internal(struct peer *peer, uint8_t code,
uint8_t sub_code, uint8_t *data, size_t datalen) uint8_t sub_code, uint8_t *data,
size_t datalen, bool use_curr)
{ {
struct stream *s; struct stream *s;
bool hard_reset = bgp_notify_send_hard_reset(peer, code, sub_code); bool hard_reset = bgp_notify_send_hard_reset(peer, code, sub_code);
@ -917,8 +918,11 @@ void bgp_notify_send_with_data(struct peer *peer, uint8_t code,
* If possible, store last packet for debugging purposes. This check is * If possible, store last packet for debugging purposes. This check is
* in place because we are sometimes called with a doppelganger peer, * in place because we are sometimes called with a doppelganger peer,
* who tends to have a plethora of fields nulled out. * who tends to have a plethora of fields nulled out.
*
* Some callers should not attempt this - the io pthread for example
* should not touch internals of the peer struct.
*/ */
if (peer->curr) { if (use_curr && peer->curr) {
size_t packetsize = stream_get_endp(peer->curr); size_t packetsize = stream_get_endp(peer->curr);
assert(packetsize <= peer->max_packet_size); assert(packetsize <= peer->max_packet_size);
memcpy(peer->last_reset_cause, peer->curr->data, packetsize); memcpy(peer->last_reset_cause, peer->curr->data, packetsize);
@ -1001,7 +1005,27 @@ void bgp_notify_send_with_data(struct peer *peer, uint8_t code,
*/ */
void bgp_notify_send(struct peer *peer, uint8_t code, uint8_t sub_code) void bgp_notify_send(struct peer *peer, uint8_t code, uint8_t sub_code)
{ {
bgp_notify_send_with_data(peer, code, sub_code, NULL, 0); bgp_notify_send_internal(peer, code, sub_code, NULL, 0, true);
}
/*
* Enqueue notification; called from the main pthread, peer object access is ok.
*/
void bgp_notify_send_with_data(struct peer *peer, uint8_t code,
uint8_t sub_code, uint8_t *data, size_t datalen)
{
bgp_notify_send_internal(peer, code, sub_code, data, datalen, true);
}
/*
* For use by the io pthread, queueing a notification but avoiding access to
* the peer object.
*/
void bgp_notify_io_invalid(struct peer *peer, uint8_t code, uint8_t sub_code,
uint8_t *data, size_t datalen)
{
/* Avoid touching the peer object */
bgp_notify_send_internal(peer, code, sub_code, data, datalen, false);
} }
/* /*

View File

@ -62,6 +62,8 @@ extern void bgp_open_send(struct peer *);
extern void bgp_notify_send(struct peer *, uint8_t, uint8_t); extern void bgp_notify_send(struct peer *, uint8_t, uint8_t);
extern void bgp_notify_send_with_data(struct peer *, uint8_t, uint8_t, extern void bgp_notify_send_with_data(struct peer *, uint8_t, uint8_t,
uint8_t *, size_t); uint8_t *, size_t);
void bgp_notify_io_invalid(struct peer *peer, uint8_t code, uint8_t sub_code,
uint8_t *data, size_t datalen);
extern void bgp_route_refresh_send(struct peer *peer, afi_t afi, safi_t safi, extern void bgp_route_refresh_send(struct peer *peer, afi_t afi, safi_t safi,
uint8_t orf_type, uint8_t when_to_refresh, uint8_t orf_type, uint8_t when_to_refresh,
int remove, uint8_t subtype); int remove, uint8_t subtype);