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);
> }
> }
>
