Hi Claudio, > On 10/12/2020, at 1:13 AM, Claudio Jeker <cje...@diehard.n-r-g.com> wrote: > > This diff makes the timer code independent from struct peer. This way > it can be used in different places without too much issues.
ok procter@ > OK? > -- > :wq Claudio > > Index: control.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/control.c,v > retrieving revision 1.101 > diff -u -p -r1.101 control.c > --- control.c 5 Nov 2020 11:28:11 -0000 1.101 > +++ control.c 9 Dec 2020 11:57:05 -0000 > @@ -333,7 +333,8 @@ control_dispatch_msg(struct pollfd *pfd, > IMSG_CTL_SHOW_NEIGHBOR, > 0, 0, -1, p, sizeof(*p)); > for (i = 1; i < Timer_Max; i++) { > - if (!timer_running(p, i, &d)) > + if (!timer_running(&p->timers, > + i, &d)) > continue; > ct.type = i; > ct.val = d; > @@ -403,7 +404,8 @@ control_dispatch_msg(struct pollfd *pfd, > if (!p->conf.down) { > session_stop(p, > ERR_CEASE_ADMIN_RESET); > - timer_set(p, Timer_IdleHold, > + timer_set(&p->timers, > + Timer_IdleHold, > SESSION_CLEAR_DELAY); > } else { > session_stop(p, > Index: session.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/session.c,v > retrieving revision 1.405 > diff -u -p -r1.405 session.c > --- session.c 5 Nov 2020 14:44:59 -0000 1.405 > +++ session.c 9 Dec 2020 11:56:19 -0000 > @@ -263,7 +263,8 @@ session_main(int debug, int verbose) > if (p->reconf_action == RECONF_REINIT) { > session_stop(p, ERR_CEASE_ADMIN_RESET); > if (!p->conf.down) > - timer_set(p, Timer_IdleHold, 0); > + timer_set(&p->timers, > + Timer_IdleHold, 0); > } > > /* deletion due? */ > @@ -272,7 +273,7 @@ session_main(int debug, int verbose) > session_demote(p, -1); > p->conf.demote_group[0] = 0; > session_stop(p, ERR_CEASE_PEER_UNCONF); > - timer_remove_all(p); > + timer_remove_all(&p->timers); > tcp_md5_del_listener(conf, p); > log_peer_warnx(&p->conf, "removed"); > RB_REMOVE(peer_head, &conf->peers, p); > @@ -366,10 +367,10 @@ session_main(int debug, int verbose) > now = getmonotime(); > RB_FOREACH(p, peer_head, &conf->peers) { > time_t nextaction; > - struct peer_timer *pt; > + struct timer *pt; > > /* check timers */ > - if ((pt = timer_nextisdue(p, now)) != NULL) { > + if ((pt = timer_nextisdue(&p->timers, now)) != NULL) { > switch (pt->type) { > case Timer_Hold: > bgp_fsm(p, EVNT_TIMER_HOLDTIME); > @@ -387,24 +388,27 @@ session_main(int debug, int verbose) > p->IdleHoldTime = > INTERVAL_IDLE_HOLD_INITIAL; > p->errcnt = 0; > - timer_stop(p, Timer_IdleHoldReset); > + timer_stop(&p->timers, > + Timer_IdleHoldReset); > break; > case Timer_CarpUndemote: > - timer_stop(p, Timer_CarpUndemote); > + timer_stop(&p->timers, > + Timer_CarpUndemote); > if (p->demoted && > p->state == STATE_ESTABLISHED) > session_demote(p, -1); > break; > case Timer_RestartTimeout: > - timer_stop(p, Timer_RestartTimeout); > + timer_stop(&p->timers, > + Timer_RestartTimeout); > session_graceful_stop(p); > break; > default: > fatalx("King Bula lost in time"); > } > } > - if ((nextaction = timer_nextduein(p, now)) != -1 && > - nextaction < timeout) > + if ((nextaction = timer_nextduein(&p->timers, > + now)) != -1 && nextaction < timeout) > timeout = nextaction; > > /* are we waiting for a write? */ > @@ -515,7 +519,7 @@ session_main(int debug, int verbose) > "bgpd shutting down", > sizeof(p->conf.reason)); > session_stop(p, ERR_CEASE_ADMIN_DOWN); > - timer_remove_all(p); > + timer_remove_all(&p->timers); > free(p); > } > > @@ -569,9 +573,9 @@ init_peer(struct peer *p) > > change_state(p, STATE_IDLE, EVNT_NONE); > if (p->conf.down) > - timer_stop(p, Timer_IdleHold); /* no autostart */ > + timer_stop(&p->timers, Timer_IdleHold); /* no autostart */ > else > - timer_set(p, Timer_IdleHold, 0); /* start ASAP */ > + timer_set(&p->timers, Timer_IdleHold, 0); /* start ASAP */ > > /* > * on startup, demote if requested. > @@ -592,9 +596,9 @@ bgp_fsm(struct peer *peer, enum session_ > case STATE_IDLE: > switch (event) { > case EVNT_START: > - timer_stop(peer, Timer_Hold); > - timer_stop(peer, Timer_Keepalive); > - timer_stop(peer, Timer_IdleHold); > + timer_stop(&peer->timers, Timer_Hold); > + timer_stop(&peer->timers, Timer_Keepalive); > + timer_stop(&peer->timers, Timer_IdleHold); > > /* allocate read buffer */ > peer->rbuf = calloc(1, sizeof(struct ibuf_read)); > @@ -610,14 +614,14 @@ bgp_fsm(struct peer *peer, enum session_ > peer->stats.last_rcvd_suberr = 0; > > if (!peer->depend_ok) > - timer_stop(peer, Timer_ConnectRetry); > + timer_stop(&peer->timers, Timer_ConnectRetry); > else if (peer->passive || peer->conf.passive || > peer->conf.template) { > change_state(peer, STATE_ACTIVE, event); > - timer_stop(peer, Timer_ConnectRetry); > + timer_stop(&peer->timers, Timer_ConnectRetry); > } else { > change_state(peer, STATE_CONNECT, event); > - timer_set(peer, Timer_ConnectRetry, > + timer_set(&peer->timers, Timer_ConnectRetry, > conf->connectretry); > session_connect(peer); > } > @@ -636,19 +640,19 @@ bgp_fsm(struct peer *peer, enum session_ > case EVNT_CON_OPEN: > session_tcp_established(peer); > session_open(peer); > - timer_stop(peer, Timer_ConnectRetry); > + timer_stop(&peer->timers, Timer_ConnectRetry); > peer->holdtime = INTERVAL_HOLD_INITIAL; > start_timer_holdtime(peer); > change_state(peer, STATE_OPENSENT, event); > break; > case EVNT_CON_OPENFAIL: > - timer_set(peer, Timer_ConnectRetry, > + timer_set(&peer->timers, Timer_ConnectRetry, > conf->connectretry); > session_close_connection(peer); > change_state(peer, STATE_ACTIVE, event); > break; > case EVNT_TIMER_CONNRETRY: > - timer_set(peer, Timer_ConnectRetry, > + timer_set(&peer->timers, Timer_ConnectRetry, > conf->connectretry); > session_connect(peer); > break; > @@ -665,19 +669,19 @@ bgp_fsm(struct peer *peer, enum session_ > case EVNT_CON_OPEN: > session_tcp_established(peer); > session_open(peer); > - timer_stop(peer, Timer_ConnectRetry); > + timer_stop(&peer->timers, Timer_ConnectRetry); > peer->holdtime = INTERVAL_HOLD_INITIAL; > start_timer_holdtime(peer); > change_state(peer, STATE_OPENSENT, event); > break; > case EVNT_CON_OPENFAIL: > - timer_set(peer, Timer_ConnectRetry, > + timer_set(&peer->timers, Timer_ConnectRetry, > conf->connectretry); > session_close_connection(peer); > change_state(peer, STATE_ACTIVE, event); > break; > case EVNT_TIMER_CONNRETRY: > - timer_set(peer, Timer_ConnectRetry, > + timer_set(&peer->timers, Timer_ConnectRetry, > peer->holdtime); > change_state(peer, STATE_CONNECT, event); > session_connect(peer); > @@ -697,7 +701,7 @@ bgp_fsm(struct peer *peer, enum session_ > break; > case EVNT_CON_CLOSED: > session_close_connection(peer); > - timer_set(peer, Timer_ConnectRetry, > + timer_set(&peer->timers, Timer_ConnectRetry, > conf->connectretry); > change_state(peer, STATE_ACTIVE, event); > break; > @@ -720,7 +724,7 @@ bgp_fsm(struct peer *peer, enum session_ > if (parse_notification(peer)) { > change_state(peer, STATE_IDLE, event); > /* don't punish, capa negotiation */ > - timer_set(peer, Timer_IdleHold, 0); > + timer_set(&peer->timers, Timer_IdleHold, 0); > peer->IdleHoldTime /= 2; > } else > change_state(peer, STATE_IDLE, event); > @@ -815,18 +819,18 @@ void > start_timer_holdtime(struct peer *peer) > { > if (peer->holdtime > 0) > - timer_set(peer, Timer_Hold, peer->holdtime); > + timer_set(&peer->timers, Timer_Hold, peer->holdtime); > else > - timer_stop(peer, Timer_Hold); > + timer_stop(&peer->timers, Timer_Hold); > } > > void > start_timer_keepalive(struct peer *peer) > { > if (peer->holdtime > 0) > - timer_set(peer, Timer_Keepalive, peer->holdtime / 3); > + timer_set(&peer->timers, Timer_Keepalive, peer->holdtime / 3); > else > - timer_stop(peer, Timer_Keepalive); > + timer_stop(&peer->timers, Timer_Keepalive); > } > > void > @@ -868,11 +872,11 @@ change_state(struct peer *peer, enum ses > if (peer->IdleHoldTime == 0) > peer->IdleHoldTime = INTERVAL_IDLE_HOLD_INITIAL; > peer->holdtime = INTERVAL_HOLD_INITIAL; > - timer_stop(peer, Timer_ConnectRetry); > - timer_stop(peer, Timer_Keepalive); > - timer_stop(peer, Timer_Hold); > - timer_stop(peer, Timer_IdleHold); > - timer_stop(peer, Timer_IdleHoldReset); > + timer_stop(&peer->timers, Timer_ConnectRetry); > + timer_stop(&peer->timers, Timer_Keepalive); > + timer_stop(&peer->timers, Timer_Hold); > + timer_stop(&peer->timers, Timer_IdleHold); > + timer_stop(&peer->timers, Timer_IdleHoldReset); > session_close_connection(peer); > msgbuf_clear(&peer->wbuf); > free(peer->rbuf); > @@ -884,7 +888,8 @@ change_state(struct peer *peer, enum ses > peer->conf.id, 0, -1, NULL, 0); > > if (event != EVNT_STOP) { > - timer_set(peer, Timer_IdleHold, peer->IdleHoldTime); > + timer_set(&peer->timers, Timer_IdleHold, > + peer->IdleHoldTime); > if (event != EVNT_NONE && > peer->IdleHoldTime < MAX_IDLE_HOLD/2) > peer->IdleHoldTime *= 2; > @@ -894,7 +899,7 @@ change_state(struct peer *peer, enum ses > (event == EVNT_CON_CLOSED || > event == EVNT_CON_FATAL)) { > /* don't punish graceful restart */ > - timer_set(peer, Timer_IdleHold, 0); > + timer_set(&peer->timers, Timer_IdleHold, 0); > peer->IdleHoldTime /= 2; > session_graceful_restart(peer); > } else > @@ -915,11 +920,11 @@ change_state(struct peer *peer, enum ses > /* do the graceful restart dance */ > session_graceful_restart(peer); > peer->holdtime = INTERVAL_HOLD_INITIAL; > - timer_stop(peer, Timer_ConnectRetry); > - timer_stop(peer, Timer_Keepalive); > - timer_stop(peer, Timer_Hold); > - timer_stop(peer, Timer_IdleHold); > - timer_stop(peer, Timer_IdleHoldReset); > + timer_stop(&peer->timers, Timer_ConnectRetry); > + timer_stop(&peer->timers, Timer_Keepalive); > + timer_stop(&peer->timers, Timer_Hold); > + timer_stop(&peer->timers, Timer_IdleHold); > + timer_stop(&peer->timers, Timer_IdleHoldReset); > session_close_connection(peer); > msgbuf_clear(&peer->wbuf); > bzero(&peer->capa.peer, sizeof(peer->capa.peer)); > @@ -935,9 +940,10 @@ change_state(struct peer *peer, enum ses > case STATE_OPENCONFIRM: > break; > case STATE_ESTABLISHED: > - timer_set(peer, Timer_IdleHoldReset, peer->IdleHoldTime); > + timer_set(&peer->timers, Timer_IdleHoldReset, > + peer->IdleHoldTime); > if (peer->demoted) > - timer_set(peer, Timer_CarpUndemote, > + timer_set(&peer->timers, Timer_CarpUndemote, > INTERVAL_HOLD_DEMOTED); > session_up(peer); > break; > @@ -981,7 +987,7 @@ session_accept(int listenfd) > p = getpeerbyip(conf, (struct sockaddr *)&cliaddr); > > if (p != NULL && p->state == STATE_IDLE && p->errcnt < 2) { > - if (timer_running(p, Timer_IdleHold, NULL)) { > + if (timer_running(&p->timers, Timer_IdleHold, NULL)) { > /* fast reconnect after clear */ > p->passive = 1; > bgp_fsm(p, EVNT_START); > @@ -1669,7 +1675,8 @@ session_graceful_restart(struct peer *p) > { > u_int8_t i; > > - timer_set(p, Timer_RestartTimeout, p->capa.neg.grestart.timeout); > + timer_set(&p->timers, Timer_RestartTimeout, > + p->capa.neg.grestart.timeout); > > for (i = 0; i < AID_MAX; i++) { > if (p->capa.neg.grestart.flags[i] & CAPA_GR_PRESENT) { > @@ -2131,7 +2138,8 @@ parse_open(struct peer *peer) > session_notification(peer, ERR_OPEN, ERR_OPEN_OPT, > NULL, 0); > change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN); > - timer_set(peer, Timer_IdleHold, 0); /* no punish */ > + /* no punish */ > + timer_set(&peer->timers, Timer_IdleHold, 0); > peer->IdleHoldTime /= 2; > return (-1); > } > @@ -2868,8 +2876,8 @@ session_dispatch_imsg(struct imsgbuf *ib > > bgp_fsm(p, EVNT_STOP); > if (t) > - timer_set(p, Timer_IdleHold, > - 60 * t); > + timer_set(&p->timers, > + Timer_IdleHold, 60 * t); > break; > default: > bgp_fsm(p, EVNT_CON_FATAL); > @@ -2903,7 +2911,7 @@ session_dispatch_imsg(struct imsgbuf *ib > aid2str(aid)); > p->capa.neg.grestart.flags[aid] &= > ~CAPA_GR_RESTARTING; > - timer_stop(p, Timer_RestartTimeout); > + timer_stop(&p->timers, Timer_RestartTimeout); > > /* signal back to RDE to cleanup stale routes */ > if (imsg_rde(IMSG_SESSION_RESTARTED, > Index: session.h > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/session.h,v > retrieving revision 1.147 > diff -u -p -r1.147 session.h > --- session.h 5 Nov 2020 11:28:11 -0000 1.147 > +++ session.h 9 Dec 2020 11:46:22 -0000 > @@ -187,13 +187,13 @@ enum Timer { > Timer_Max > }; > > -struct peer_timer { > - TAILQ_ENTRY(peer_timer) entry; > +struct timer { > + TAILQ_ENTRY(timer) entry; > enum Timer type; > time_t val; > }; > > -TAILQ_HEAD(peer_timer_head, peer_timer); > +TAILQ_HEAD(timer_head, timer); > > struct peer { > struct peer_config conf; > @@ -214,7 +214,7 @@ struct peer { > struct bgpd_addr local; > struct bgpd_addr local_alt; > struct bgpd_addr remote; > - struct peer_timer_head timers; > + struct timer_head timers; > struct msgbuf wbuf; > struct ibuf_read *rbuf; > struct peer *template; > @@ -314,11 +314,11 @@ int imsg_ctl_rde(int, pid_t, void *, u > void session_stop(struct peer *, u_int8_t); > > /* timer.c */ > -struct peer_timer *timer_get(struct peer *, enum Timer); > -struct peer_timer *timer_nextisdue(struct peer *, time_t); > -time_t timer_nextduein(struct peer *, time_t); > -int timer_running(struct peer *, enum Timer, time_t *); > -void timer_set(struct peer *, enum Timer, u_int); > -void timer_stop(struct peer *, enum Timer); > -void timer_remove(struct peer *, enum Timer); > -void timer_remove_all(struct peer *); > +struct timer *timer_get(struct timer_head *, enum Timer); > +struct timer *timer_nextisdue(struct timer_head *, time_t); > +time_t timer_nextduein(struct timer_head *, time_t); > +int timer_running(struct timer_head *, enum Timer, time_t *); > +void timer_set(struct timer_head *, enum Timer, u_int); > +void timer_stop(struct timer_head *, enum Timer); > +void timer_remove(struct timer_head *, enum Timer); > +void timer_remove_all(struct timer_head *); > Index: timer.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/timer.c,v > retrieving revision 1.18 > diff -u -p -r1.18 timer.c > --- timer.c 24 May 2019 11:37:52 -0000 1.18 > +++ timer.c 9 Dec 2020 12:01:39 -0000 > @@ -36,108 +36,109 @@ getmonotime(void) > return (ts.tv_sec); > } > > -struct peer_timer * > -timer_get(struct peer *p, enum Timer timer) > +struct timer * > +timer_get(struct timer_head *th, enum Timer timer) > { > - struct peer_timer *pt; > + struct timer *t; > > - TAILQ_FOREACH(pt, &p->timers, entry) > - if (pt->type == timer) > + TAILQ_FOREACH(t, th, entry) > + if (t->type == timer) > break; > > - return (pt); > + return (t); > } > > -struct peer_timer * > -timer_nextisdue(struct peer *p, time_t now) > +struct timer * > +timer_nextisdue(struct timer_head *th, time_t now) > { > - struct peer_timer *pt; > + struct timer *t; > > - pt = TAILQ_FIRST(&p->timers); > - if (pt != NULL && pt->val > 0 && pt->val <= now) > - return (pt); > + t = TAILQ_FIRST(th); > + if (t != NULL && t->val > 0 && t->val <= now) > + return (t); > return (NULL); > } > > time_t > -timer_nextduein(struct peer *p, time_t now) > +timer_nextduein(struct timer_head *th, time_t now) > { > - struct peer_timer *pt; > + struct timer *t; > > - if ((pt = TAILQ_FIRST(&p->timers)) != NULL && pt->val > 0) > - return (MAXIMUM(pt->val - now, 0)); > + if ((t = TAILQ_FIRST(th)) != NULL && t->val > 0) > + return (MAXIMUM(t->val - now, 0)); > return (-1); > } > > int > -timer_running(struct peer *p, enum Timer timer, time_t *left) > +timer_running(struct timer_head *th, enum Timer timer, time_t *left) > { > - struct peer_timer *pt = timer_get(p, timer); > + struct timer *t = timer_get(th, timer); > > - if (pt != NULL && pt->val > 0) { > + if (t != NULL && t->val > 0) { > if (left != NULL) > - *left = pt->val - getmonotime(); > + *left = t->val - getmonotime(); > return (1); > } > return (0); > } > > void > -timer_set(struct peer *p, enum Timer timer, u_int offset) > +timer_set(struct timer_head *th, enum Timer timer, u_int offset) > { > - struct peer_timer *t, *pt = timer_get(p, timer); > + struct timer *t = timer_get(th, timer); > + struct timer *next; > > - if (pt == NULL) { /* have to create */ > - if ((pt = malloc(sizeof(*pt))) == NULL) > + if (t == NULL) { /* have to create */ > + if ((t = malloc(sizeof(*t))) == NULL) > fatal("timer_set"); > - pt->type = timer; > + t->type = timer; > } else { > - if (pt->val == getmonotime() + (time_t)offset) > + if (t->val == getmonotime() + (time_t)offset) > return; > - TAILQ_REMOVE(&p->timers, pt, entry); > + TAILQ_REMOVE(th, t, entry); > } > > - pt->val = getmonotime() + offset; > + t->val = getmonotime() + offset; > > - TAILQ_FOREACH(t, &p->timers, entry) > - if (t->val == 0 || t->val > pt->val) > + TAILQ_FOREACH(next, th, entry) > + if (next->val == 0 || next->val > t->val) > break; > - if (t != NULL) > - TAILQ_INSERT_BEFORE(t, pt, entry); > + if (next != NULL) > + TAILQ_INSERT_BEFORE(next, t, entry); > else > - TAILQ_INSERT_TAIL(&p->timers, pt, entry); > + TAILQ_INSERT_TAIL(th, t, entry); > } > > void > -timer_stop(struct peer *p, enum Timer timer) > +timer_stop(struct timer_head *th, enum Timer timer) > { > - struct peer_timer *pt = timer_get(p, timer); > + struct timer *t = timer_get(th, timer); > > - if (pt != NULL) { > - pt->val = 0; > - TAILQ_REMOVE(&p->timers, pt, entry); > - TAILQ_INSERT_TAIL(&p->timers, pt, entry); > + if (t != NULL) { > + t->val = 0; > + TAILQ_REMOVE(th, t, entry); > + TAILQ_INSERT_TAIL(th, t, entry); > } > } > > void > -timer_remove(struct peer *p, enum Timer timer) > +timer_remove(struct timer_head *th, enum Timer timer) > { > - struct peer_timer *pt = timer_get(p, timer); > + struct timer *t = timer_get(th, timer); > > - if (pt != NULL) { > - TAILQ_REMOVE(&p->timers, pt, entry); > - free(pt); > + if (t != NULL) { > + TAILQ_REMOVE(th, t, entry); > + free(t); > } > } > > void > -timer_remove_all(struct peer *p) > +timer_remove_all(struct timer_head *th) > { > - struct peer_timer *pt; > + struct timer *t; > > - while ((pt = TAILQ_FIRST(&p->timers)) != NULL) { > - TAILQ_REMOVE(&p->timers, pt, entry); > - free(pt); > + while ((t = TAILQ_FIRST(th)) != NULL) { > + TAILQ_REMOVE(th, t, entry); > + free(t); > } > } >