From dfd27b08d9d9ea2afef633e78123d48fa84f95c5 Mon Sep 17 00:00:00 2001 From: claudio Date: Tue, 28 Mar 2023 12:15:23 +0000 Subject: [PATCH] Introduce a semaphore to protect intermediate state from different RTR sessions to leak into the RDE via rtr_recalc. Only run rtr_recalc when the last or only RTR session is done with the update. Run a new timer along to ensure that the semaphore is not hold forever. The timeout is currently a very generous 60sec, no RTR cache should be that slow. OK tb@ --- usr.sbin/bgpd/bgpd.h | 3 ++- usr.sbin/bgpd/rtr.c | 20 +++++++++++++++++++- usr.sbin/bgpd/rtr_proto.c | 25 +++++++++++++++++++++++-- usr.sbin/bgpd/session.h | 5 ++++- 4 files changed, 48 insertions(+), 5 deletions(-) diff --git a/usr.sbin/bgpd/bgpd.h b/usr.sbin/bgpd/bgpd.h index 4cec43bcbe1..ecee4ef776a 100644 --- a/usr.sbin/bgpd/bgpd.h +++ b/usr.sbin/bgpd/bgpd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: bgpd.h,v 1.466 2023/03/28 12:06:15 claudio Exp $ */ +/* $OpenBSD: bgpd.h,v 1.467 2023/03/28 12:15:23 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -1639,6 +1639,7 @@ static const char * const timernames[] = { "RTR RefreshTimer", "RTR RetryTimer", "RTR ExpireTimer", + "RTR ActiveTimer", "" }; diff --git a/usr.sbin/bgpd/rtr.c b/usr.sbin/bgpd/rtr.c index 178679ef918..2c3b18cc9c7 100644 --- a/usr.sbin/bgpd/rtr.c +++ b/usr.sbin/bgpd/rtr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rtr.c,v 1.12 2023/03/09 17:21:21 claudio Exp $ */ +/* $OpenBSD: rtr.c,v 1.13 2023/03/28 12:15:23 claudio Exp $ */ /* * Copyright (c) 2020 Claudio Jeker @@ -40,6 +40,7 @@ static struct imsgbuf *ibuf_main; static struct imsgbuf *ibuf_rde; static struct bgpd_config *conf, *nconf; static struct timer_head expire_timer; +static int rtr_recalc_semaphore; static void rtr_sighdlr(int sig) @@ -58,6 +59,20 @@ rtr_sighdlr(int sig) #define EXPIRE_TIMEOUT 300 +void +rtr_sem_acquire(int cnt) +{ + rtr_recalc_semaphore += cnt; +} + +void +rtr_sem_release(int cnt) +{ + rtr_recalc_semaphore -= cnt; + if (rtr_recalc_semaphore < 0) + fatalx("rtr recalc semaphore underflow"); +} + /* * Every EXPIRE_TIMEOUT seconds traverse the static roa-set table and expire * all elements where the expires timestamp is smaller or equal to now. @@ -542,6 +557,9 @@ rtr_recalc(void) struct aspa_set *aspa; struct aspa_prep ap = { 0 }; + if (rtr_recalc_semaphore > 0) + return; + RB_INIT(&rt); RB_INIT(&at); diff --git a/usr.sbin/bgpd/rtr_proto.c b/usr.sbin/bgpd/rtr_proto.c index c13d0edff8d..bfa74c47b30 100644 --- a/usr.sbin/bgpd/rtr_proto.c +++ b/usr.sbin/bgpd/rtr_proto.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rtr_proto.c,v 1.15 2023/03/17 11:14:10 claudio Exp $ */ +/* $OpenBSD: rtr_proto.c,v 1.16 2023/03/28 12:15:23 claudio Exp $ */ /* * Copyright (c) 2020 Claudio Jeker @@ -40,6 +40,7 @@ struct rtr_header { #define RTR_DEFAULT_REFRESH 3600 #define RTR_DEFAULT_RETRY 600 #define RTR_DEFAULT_EXPIRE 7200 +#define RTR_DEFAULT_ACTIVE 60 enum rtr_pdu_type { SERIAL_NOTIFY = 0, @@ -99,6 +100,7 @@ enum rtr_event { RTR_EVNT_TIMER_REFRESH, RTR_EVNT_TIMER_RETRY, RTR_EVNT_TIMER_EXPIRE, + RTR_EVNT_TIMER_ACTIVE, RTR_EVNT_SEND_ERROR, RTR_EVNT_SERIAL_NOTIFY, RTR_EVNT_CACHE_RESPONSE, @@ -116,6 +118,7 @@ static const char *rtr_eventnames[] = { "refresh timer expired", "retry timer expired", "expire timer expired", + "activity timer expired", "sent error", "serial notify received", "cache response received", @@ -157,8 +160,10 @@ struct rtr_session { uint32_t refresh; uint32_t retry; uint32_t expire; + uint32_t active; int session_id; int fd; + int active_lock; enum rtr_state state; enum reconf_action reconf_action; enum rtr_error last_sent_error; @@ -1033,18 +1038,30 @@ rtr_fsm(struct rtr_session *rs, enum rtr_event event) rtr_reset_cache(rs); rtr_recalc(); break; + case RTR_EVNT_TIMER_ACTIVE: + log_warnx("rtr %s: activity timer fired", log_rtr(rs)); + rtr_sem_release(rs->active_lock); + rtr_recalc(); + rs->active_lock = 0; + break; case RTR_EVNT_CACHE_RESPONSE: rs->state = RTR_STATE_ACTIVE; timer_stop(&rs->timers, Timer_Rtr_Refresh); timer_stop(&rs->timers, Timer_Rtr_Retry); - /* XXX start timer to limit active time */ + timer_set(&rs->timers, Timer_Rtr_Active, rs->active); + /* prevent rtr_recalc from running while active */ + rs->active_lock = 1; + rtr_sem_acquire(rs->active_lock); break; case RTR_EVNT_END_OF_DATA: /* start refresh and expire timers */ timer_set(&rs->timers, Timer_Rtr_Refresh, rs->refresh); timer_set(&rs->timers, Timer_Rtr_Expire, rs->expire); + timer_stop(&rs->timers, Timer_Rtr_Active); rs->state = RTR_STATE_IDLE; + rtr_sem_release(rs->active_lock); rtr_recalc(); + rs->active_lock = 0; break; case RTR_EVNT_CACHE_RESET: rtr_reset_cache(rs); @@ -1164,6 +1181,9 @@ rtr_check_events(struct pollfd *pfds, size_t npfds) case Timer_Rtr_Expire: rtr_fsm(rs, RTR_EVNT_TIMER_EXPIRE); break; + case Timer_Rtr_Active: + rtr_fsm(rs, RTR_EVNT_TIMER_ACTIVE); + break; default: fatalx("King Bula lost in time"); } @@ -1237,6 +1257,7 @@ rtr_new(uint32_t id, char *descr) rs->refresh = RTR_DEFAULT_REFRESH; rs->retry = RTR_DEFAULT_RETRY; rs->expire = RTR_DEFAULT_EXPIRE; + rs->active = RTR_DEFAULT_ACTIVE; rs->state = RTR_STATE_CLOSED; rs->reconf_action = RECONF_REINIT; rs->last_recv_error = NO_ERROR; diff --git a/usr.sbin/bgpd/session.h b/usr.sbin/bgpd/session.h index c9e6b2164ba..65e96d6c87f 100644 --- a/usr.sbin/bgpd/session.h +++ b/usr.sbin/bgpd/session.h @@ -1,4 +1,4 @@ -/* $OpenBSD: session.h,v 1.161 2023/03/09 17:21:21 claudio Exp $ */ +/* $OpenBSD: session.h,v 1.162 2023/03/28 12:15:23 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -201,6 +201,7 @@ enum Timer { Timer_Rtr_Refresh, Timer_Rtr_Retry, Timer_Rtr_Expire, + Timer_Rtr_Active, Timer_Max }; @@ -334,6 +335,8 @@ void rtr_shutdown(void); void rtr_show(struct rtr_session *, pid_t); /* rtr.c */ +void rtr_sem_acquire(int); +void rtr_sem_release(int); void rtr_roa_insert(struct roa_tree *, struct roa *); void rtr_aspa_insert(struct aspa_tree *, struct aspa_set *); void rtr_main(int, int); -- 2.20.1