Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bgpd: avoid notify race between io and main pthreads #11926

Merged
merged 1 commit into from
Sep 12, 2022
Merged

bgpd: avoid notify race between io and main pthreads #11926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions bgpd/bgp_io.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
#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_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/bgpd.h" // for peer, BGP_MARKER_SIZE, bgp_master, bm
/* clang-format on */
Expand Down Expand Up @@ -526,8 +526,8 @@ static bool validate_header(struct peer *peer)
return false;

if (memcmp(m_correct, m_rx, BGP_MARKER_SIZE) != 0) {
bgp_notify_send(peer, BGP_NOTIFY_HEADER_ERR,
BGP_NOTIFY_HEADER_NOT_SYNC);
bgp_notify_io_invalid(peer, BGP_NOTIFY_HEADER_ERR,
BGP_NOTIFY_HEADER_NOT_SYNC, NULL, 0);
return false;
}

Expand All @@ -547,9 +547,8 @@ static bool validate_header(struct peer *peer)
zlog_debug("%s unknown message type 0x%02x", peer->host,
type);

bgp_notify_send_with_data(peer, BGP_NOTIFY_HEADER_ERR,
BGP_NOTIFY_HEADER_BAD_MESTYPE, &type,
1);
bgp_notify_io_invalid(peer, BGP_NOTIFY_HEADER_ERR,
BGP_NOTIFY_HEADER_BAD_MESTYPE, &type, 1);
return false;
}

Expand All @@ -574,9 +573,9 @@ static bool validate_header(struct peer *peer)

uint16_t nsize = htons(size);

bgp_notify_send_with_data(peer, BGP_NOTIFY_HEADER_ERR,
BGP_NOTIFY_HEADER_BAD_MESLEN,
(unsigned char *)&nsize, 2);
bgp_notify_io_invalid(peer, BGP_NOTIFY_HEADER_ERR,
BGP_NOTIFY_HEADER_BAD_MESLEN,
(unsigned char *)&nsize, 2);
return false;
}

Expand Down
32 changes: 28 additions & 4 deletions bgpd/bgp_packet.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -871,8 +871,9 @@ bool bgp_notify_received_hard_reset(struct peer *peer, uint8_t code,
* @param data Data portion
* @param datalen length of data portion
*/
void bgp_notify_send_with_data(struct peer *peer, uint8_t code,
uint8_t sub_code, uint8_t *data, size_t datalen)
static void bgp_notify_send_internal(struct peer *peer, uint8_t code,
uint8_t sub_code, uint8_t *data,
size_t datalen, bool use_curr)
{
struct stream *s;
bool hard_reset = bgp_notify_send_hard_reset(peer, code, sub_code);
Expand Down Expand Up @@ -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
* in place because we are sometimes called with a doppelganger peer,
* 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);
assert(packetsize <= peer->max_packet_size);
memcpy(peer->last_reset_cause, peer->curr->data, packetsize);
Expand Down Expand Up @@ -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)
{
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);
}

/*
Expand Down
2 changes: 2 additions & 0 deletions bgpd/bgp_packet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_with_data(struct peer *, uint8_t, uint8_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,
uint8_t orf_type, uint8_t when_to_refresh,
int remove, uint8_t subtype);
Expand Down