Skip to content

Commit

Permalink
bgpd: Fix wrong pthread event cancelling
Browse files Browse the repository at this point in the history
0  __pthread_kill_implementation (no_tid=0, signo=6, threadid=130719886083648) at ./nptl/pthread_kill.c:44
1  __pthread_kill_internal (signo=6, threadid=130719886083648) at ./nptl/pthread_kill.c:78
2  __GI___pthread_kill (threadid=130719886083648, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
3  0x000076e399e42476 in __GI_raise (sig=6) at ../sysdeps/posix/raise.c:26
4  0x000076e39a34f950 in core_handler (signo=6, siginfo=0x76e3985fca30, context=0x76e3985fc900) at lib/sigevent.c:258
5  <signal handler called>
6  __pthread_kill_implementation (no_tid=0, signo=6, threadid=130719886083648) at ./nptl/pthread_kill.c:44
7  __pthread_kill_internal (signo=6, threadid=130719886083648) at ./nptl/pthread_kill.c:78
8  __GI___pthread_kill (threadid=130719886083648, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
9  0x000076e399e42476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
10 0x000076e399e287f3 in __GI_abort () at ./stdlib/abort.c:79
11 0x000076e39a39874b in _zlog_assert_failed (xref=0x76e39a46cca0 <_xref.27>, extra=0x0) at lib/zlog.c:789
12 0x000076e39a369dde in cancel_event_helper (m=0x5eda32df5e40, arg=0x5eda33afeed0, flags=1) at lib/event.c:1428
13 0x000076e39a369ef6 in event_cancel_event_ready (m=0x5eda32df5e40, arg=0x5eda33afeed0) at lib/event.c:1470
14 0x00005eda0a94a5b3 in bgp_stop (connection=0x5eda33afeed0) at bgpd/bgp_fsm.c:1355
15 0x00005eda0a94b4ae in bgp_stop_with_notify (connection=0x5eda33afeed0, code=8 '\b', sub_code=0 '\000') at bgpd/bgp_fsm.c:1610
16 0x00005eda0a979498 in bgp_packet_add (connection=0x5eda33afeed0, peer=0x5eda33b11800, s=0x76e3880daf90) at bgpd/bgp_packet.c:152
17 0x00005eda0a97a80f in bgp_keepalive_send (peer=0x5eda33b11800) at bgpd/bgp_packet.c:639
18 0x00005eda0a9511fd in peer_process (hb=0x5eda33c9ab80, arg=0x76e3985ffaf0) at bgpd/bgp_keepalives.c:111
19 0x000076e39a2cd8e6 in hash_iterate (hash=0x76e388000be0, func=0x5eda0a95105e <peer_process>, arg=0x76e3985ffaf0) at lib/hash.c:252
20 0x00005eda0a951679 in bgp_keepalives_start (arg=0x5eda3306af80) at bgpd/bgp_keepalives.c:214
21 0x000076e39a2c9932 in frr_pthread_inner (arg=0x5eda3306af80) at lib/frr_pthread.c:180
22 0x000076e399e94ac3 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:442
23 0x000076e399f26850 in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
(gdb) f 12
12 0x000076e39a369dde in cancel_event_helper (m=0x5eda32df5e40, arg=0x5eda33afeed0, flags=1) at lib/event.c:1428
1428		assert(m->owner == pthread_self());

In this decode the attempt to cancel the connection's events from
the wrong thread is causing the crash.  Modify the code to create an
event on the bm->master to cancel the events for the connection.

Signed-off-by: Donald Sharp <[email protected]>
  • Loading branch information
donaldsharp committed Dec 13, 2024
1 parent e256917 commit fbf69f9
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 2 deletions.
10 changes: 10 additions & 0 deletions bgpd/bgp_fsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ static struct peer *peer_xfer_conn(struct peer *from_peer)
EVENT_OFF(going_away->t_delayopen);
EVENT_OFF(going_away->t_connect_check_r);
EVENT_OFF(going_away->t_connect_check_w);
EVENT_OFF(going_away->t_stop_with_notify);
EVENT_OFF(keeper->t_routeadv);
EVENT_OFF(keeper->t_connect);
EVENT_OFF(keeper->t_delayopen);
Expand Down Expand Up @@ -1472,6 +1473,8 @@ enum bgp_fsm_state_progress bgp_stop(struct peer_connection *connection)
EVENT_OFF(connection->t_connect_check_r);
EVENT_OFF(connection->t_connect_check_w);

EVENT_OFF(connection->t_stop_with_notify);

/* Stop all timers. */
EVENT_OFF(connection->t_start);
EVENT_OFF(connection->t_connect);
Expand Down Expand Up @@ -3143,3 +3146,10 @@ void bgp_peer_gr_flags_update(struct peer *peer)
}
}
}

void bgp_event_stop_with_notify(struct event *event)
{
struct peer_connection *connection = EVENT_ARG(event);

bgp_stop_with_notify(connection, BGP_NOTIFY_HOLD_ERR, 0);
}
1 change: 1 addition & 0 deletions bgpd/bgp_fsm.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ enum bgp_fsm_state_progress {
extern void bgp_fsm_nht_update(struct peer_connection *connection,
struct peer *peer, bool has_valid_nexthops);
extern void bgp_event(struct event *event);
extern void bgp_event_stop_with_notify(struct event *event);
extern int bgp_event_update(struct peer_connection *connection,
enum bgp_fsm_events event);
extern enum bgp_fsm_state_progress bgp_stop(struct peer_connection *connection);
Expand Down
3 changes: 2 additions & 1 deletion bgpd/bgp_packet.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ static void bgp_packet_add(struct peer_connection *connection,
flog_err(EC_BGP_SENDQ_STUCK_PROPER,
"%pBP has not made any SendQ progress for 2 holdtimes (%jds), terminating session",
peer, sendholdtime);
BGP_EVENT_ADD(connection, TCP_fatal_error);
event_add_event(bm->master, bgp_event_stop_with_notify,
connection, 0, &connection->t_stop_with_notify);
} else if (delta > (intmax_t)holdtime &&
monotime(NULL) - peer->last_sendq_warn > 5) {
flog_warn(EC_BGP_SENDQ_STUCK_WARN,
Expand Down
2 changes: 2 additions & 0 deletions bgpd/bgpd.h
Original file line number Diff line number Diff line change
Expand Up @@ -1189,6 +1189,8 @@ struct peer_connection {
struct event *t_process_packet;
struct event *t_process_packet_error;

struct event *t_stop_with_notify;

union sockunion su;
#define BGP_CONNECTION_SU_UNSPEC(connection) \
(connection->su.sa.sa_family == AF_UNSPEC)
Expand Down
2 changes: 1 addition & 1 deletion zebra/kernel_netlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -931,7 +931,7 @@ static int netlink_recv_msg(struct nlsock *nl, struct msghdr *msg)
} while (status == -1 && errno == EINTR);

if (status == -1) {
if (errno == EWOULDBLOCK || errno == EAGAIN)
if (errno == EWOULDBLOCK || errno == EAGAIN || errno == EMSGSIZE)
return 0;
flog_err(EC_ZEBRA_RECVMSG_OVERRUN, "%s recvmsg overrun: %s",
nl->name, safe_strerror(errno));
Expand Down

0 comments on commit fbf69f9

Please sign in to comment.