rework timers.

stop changing tienmr values directly, always use new
timer_(get/set/stop/running) functions. preparation for more to come :)
ok claudio

7 files changed, 169 insertions, 80 deletions

diff --git a/usr.sbin/bgpctl/Makefile b/usr.sbin/bgpctl/Makefile
index 71ace1ba13f..575a423e570 100644
--- a/usr.sbin/bgpctl/Makefile
+++ b/usr.sbin/bgpctl/Makefile
@@ -1,9 +1,9 @@
-#	$OpenBSD: Makefile,v 1.9 2007/03/03 11:45:30 henning Exp $
+#	$OpenBSD: Makefile,v 1.10 2007/12/20 17:08:48 henning Exp $
 
 .PATH:		${.CURDIR}/../bgpd
 
 PROG=	bgpctl
-SRCS=	bgpctl.c parser.c buffer.c imsg.c util.c
+SRCS=	bgpctl.c parser.c buffer.c imsg.c util.c timer.c
 SRCS+=	irrfilter.c whois.c irr_asset.c irr_prefix.c irr_output.c
 SRCS+=	irr_parser.c
 CFLAGS+= -Wall
diff --git a/usr.sbin/bgpctl/bgpctl.c b/usr.sbin/bgpctl/bgpctl.c
index c19df54bf06..83cb3e101e9 100644
--- a/usr.sbin/bgpctl/bgpctl.c
+++ b/usr.sbin/bgpctl/bgpctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: bgpctl.c,v 1.128 2007/10/15 02:16:35 deraadt Exp $ */
+/*	$OpenBSD: bgpctl.c,v 1.129 2007/12/20 17:08:48 henning Exp $ */
 
 /*
  * Copyright (c) 2003 Henning Brauer <henning@openbsd.org>
@@ -54,7 +54,7 @@ int		 show_neighbor_msg(struct imsg *, enum neighbor_views);
 void		 print_neighbor_capa_mp_safi(u_int8_t);
 void		 print_neighbor_msgstats(struct peer *);
 void		 print_neighbor_timers(struct peer *);
-void		 print_timer(const char *, time_t, u_int);
+void		 print_timer(const char *, struct peer *, enum Timer, u_int);
 static char	*fmt_timeframe(time_t t);
 static char	*fmt_timeframe_core(time_t t);
 void		 show_fib_head(void);
@@ -681,22 +681,22 @@ print_neighbor_msgstats(struct peer *p)
 void
 print_neighbor_timers(struct peer *p)
 {
-	print_timer("IdleHoldTimer:", p->IdleHoldTimer, p->IdleHoldTime);
-	print_timer("ConnectRetryTimer:", p->ConnectRetryTimer,
+	print_timer("IdleHoldTimer:", p, Timer_IdleHold, p->IdleHoldTime);
+	print_timer("ConnectRetryTimer:", p, Timer_ConnectRetry,
 	    INTERVAL_CONNECTRETRY);
-	print_timer("HoldTimer:", p->HoldTimer, (u_int)p->holdtime);
-	print_timer("KeepaliveTimer:", p->KeepaliveTimer, (u_int)p->holdtime/3);
+	print_timer("HoldTimer:", p, Timer_Hold, (u_int)p->holdtime);
+	print_timer("KeepaliveTimer:", p, Timer_Keepalive, (u_int)p->holdtime/3);
 }
 
 void
-print_timer(const char *name, time_t val, u_int interval)
+print_timer(const char *name, struct peer *p, enum Timer timer, u_int interval)
 {
-	int	d;
+	time_t	d;
+	int	running = timer_running(p, timer, &d);
 
-	d = val - time(NULL);
 	printf("  %-20s ", name);
 
-	if (val == 0)
+	if (running == 0)
 		printf("%-20s", "not running");
 	else if (d <= 0)
 		printf("%-20s", "due");
diff --git a/usr.sbin/bgpd/Makefile b/usr.sbin/bgpd/Makefile
index c51a6cce387..4abf61a6d5b 100644
--- a/usr.sbin/bgpd/Makefile
+++ b/usr.sbin/bgpd/Makefile
@@ -1,4 +1,4 @@
-#	$OpenBSD: Makefile,v 1.26 2006/12/08 22:31:15 itojun Exp $
+#	$OpenBSD: Makefile,v 1.27 2007/12/20 17:08:47 henning Exp $
 
 .PATH:		${.CURDIR}/..
 
@@ -6,7 +6,7 @@ PROG=	bgpd
 SRCS=	bgpd.c buffer.c session.c log.c parse.y config.c imsg.c \
 	rde.c rde_rib.c rde_decide.c rde_prefix.c mrt.c kroute.c \
 	control.c pfkey.c rde_update.c rde_attr.c printconf.c \
-	rde_filter.c pftable.c name2id.c util.c carp.c
+	rde_filter.c pftable.c name2id.c util.c carp.c timer.c
 CFLAGS+= -Wall -I${.CURDIR}
 CFLAGS+= -Wstrict-prototypes -Wmissing-prototypes
 CFLAGS+= -Wmissing-declarations
diff --git a/usr.sbin/bgpd/control.c b/usr.sbin/bgpd/control.c
index e8a515f2dfc..cbf55ae23f8 100644
--- a/usr.sbin/bgpd/control.c
+++ b/usr.sbin/bgpd/control.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: control.c,v 1.55 2007/03/28 12:33:32 henning Exp $ */
+/*	$OpenBSD: control.c,v 1.56 2007/12/20 17:08:48 henning Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -303,8 +303,8 @@ control_dispatch_msg(struct pollfd *pfd, u_int *ctl_cnt)
 					break;
 				case IMSG_CTL_NEIGHBOR_CLEAR:
 					bgp_fsm(p, EVNT_STOP);
-					p->IdleHoldTimer = time(NULL) +
-					    SESSION_CLEAR_DELAY;
+					timer_set(p, Timer_IdleHold,
+					    SESSION_CLEAR_DELAY);
 					control_result(c, CTL_RES_OK);
 					break;
 				case IMSG_CTL_NEIGHBOR_RREFRESH:
diff --git a/usr.sbin/bgpd/session.c b/usr.sbin/bgpd/session.c
index 49b8d3f3289..c80a1b0f4a5 100644
--- a/usr.sbin/bgpd/session.c
+++ b/usr.sbin/bgpd/session.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: session.c,v 1.276 2007/06/19 09:44:55 pyr Exp $ */
+/*	$OpenBSD: session.c,v 1.277 2007/12/20 17:08:48 henning Exp $ */
 
 /*
  * Copyright (c) 2003, 2004, 2005 Henning Brauer <henning@openbsd.org>
@@ -56,7 +56,6 @@ void	session_sighdlr(int);
 int	setup_listeners(u_int *);
 void	init_conf(struct bgpd_config *);
 void	init_peer(struct peer *);
-int	timer_due(time_t);
 void	start_timer_holdtime(struct peer *);
 void	start_timer_keepalive(struct peer *);
 void	session_close_connection(struct peer *);
@@ -305,7 +304,7 @@ session_main(struct bgpd_config *config, struct peer *cpeers,
 				/* reinit due? */
 				if (p->conf.reconf_action == RECONF_REINIT) {
 					bgp_fsm(p, EVNT_STOP);
-					p->IdleHoldTimer = time(NULL);
+					timer_set(p, Timer_IdleHold, 0);
 				}
 
 				/* deletion due? */
@@ -410,28 +409,28 @@ session_main(struct bgpd_config *config, struct peer *cpeers,
 
 		for (p = peers; p != NULL; p = p->next) {
 			/* check timers */
-			if (timer_due(p->HoldTimer))
+			if (timer_due(p, Timer_Hold))
 				bgp_fsm(p, EVNT_TIMER_HOLDTIME);
-			if (timer_due(p->ConnectRetryTimer))
+			if (timer_due(p, Timer_ConnectRetry))
 				bgp_fsm(p, EVNT_TIMER_CONNRETRY);
-			if (timer_due(p->KeepaliveTimer))
+			if (timer_due(p, Timer_Keepalive))
 				bgp_fsm(p, EVNT_TIMER_KEEPALIVE);
-			if (timer_due(p->IdleHoldTimer))
+			if (timer_due(p, Timer_IdleHold))
 				bgp_fsm(p, EVNT_START);
-			if (timer_due(p->IdleHoldResetTimer)) {
+			if (timer_due(p, Timer_IdleHoldReset)) {
 				p->IdleHoldTime /= 2;
 				if (p->IdleHoldTime <=
 				    INTERVAL_IDLE_HOLD_INITIAL) {
 					p->IdleHoldTime =
 					    INTERVAL_IDLE_HOLD_INITIAL;
-					p->IdleHoldResetTimer = 0;
+					timer_stop(p, Timer_IdleHoldReset);
 					p->errcnt = 0;
 				} else
-					p->IdleHoldResetTimer =
-					    time(NULL) + p->IdleHoldTime;
+					timer_set(p, Timer_IdleHoldReset,
+					    p->IdleHoldTime);
 			}
 
-			/* set nextaction to the first expiring timer */
+			/* XXX set nextaction to the first expiring timer */
 			if (p->ConnectRetryTimer &&
 			    p->ConnectRetryTimer < nextaction)
 				nextaction = p->ConnectRetryTimer;
@@ -445,7 +444,7 @@ session_main(struct bgpd_config *config, struct peer *cpeers,
 			    p->IdleHoldResetTimer < nextaction)
 				nextaction = p->IdleHoldResetTimer;
 
-			/* carp demotion */
+			/* XXX carp demotion */
 			if (p->demoted && p->state == STATE_ESTABLISHED) {
 				if (time(NULL) - p->stats.last_updown >=
 				    INTERVAL_HOLD_DEMOTED)
@@ -611,9 +610,9 @@ init_peer(struct peer *p)
 
 	change_state(p, STATE_IDLE, EVNT_NONE);
 	if (p->conf.down)
-		p->IdleHoldTimer = 0;		/* no autostart */
+		timer_stop(p, Timer_IdleHold);		/* no autostart */
 	else
-		p->IdleHoldTimer = time(NULL);	/* start ASAP */
+		timer_set(p, Timer_IdleHold, 0);	/* start ASAP */
 
 	/*
 	 * on startup, demote if requested.
@@ -634,9 +633,9 @@ bgp_fsm(struct peer *peer, enum session_events event)
 	case STATE_IDLE:
 		switch (event) {
 		case EVNT_START:
-			peer->HoldTimer = 0;
-			peer->KeepaliveTimer = 0;
-			peer->IdleHoldTimer = 0;
+			timer_stop(peer, Timer_Hold);
+			timer_stop(peer, Timer_Keepalive);
+			timer_stop(peer, Timer_IdleHold);
 
 			/* allocate read buffer */
 			peer->rbuf = calloc(1, sizeof(struct buf_read));
@@ -659,15 +658,15 @@ bgp_fsm(struct peer *peer, enum session_events event)
 			peer->stats.last_sent_suberr = 0;
 
 			if (!peer->depend_ok)
-				peer->ConnectRetryTimer = 0;
+				timer_stop(peer, Timer_ConnectRetry);
 			else if (peer->passive || peer->conf.passive ||
 			    peer->conf.template) {
 				change_state(peer, STATE_ACTIVE, event);
-				peer->ConnectRetryTimer = 0;
+				timer_stop(peer, Timer_ConnectRetry);
 			} else {
 				change_state(peer, STATE_CONNECT, event);
-				peer->ConnectRetryTimer =
-				    time(NULL) + INTERVAL_CONNECTRETRY;
+				timer_set(peer, Timer_ConnectRetry,
+				    INTERVAL_CONNECTRETRY);
 				session_connect(peer);
 			}
 			peer->passive = 0;
@@ -685,20 +684,20 @@ bgp_fsm(struct peer *peer, enum session_events event)
 		case EVNT_CON_OPEN:
 			session_tcp_established(peer);
 			session_open(peer);
-			peer->ConnectRetryTimer = 0;
+			timer_stop(peer, Timer_ConnectRetry);
 			peer->holdtime = INTERVAL_HOLD_INITIAL;
 			start_timer_holdtime(peer);
 			change_state(peer, STATE_OPENSENT, event);
 			break;
 		case EVNT_CON_OPENFAIL:
-			peer->ConnectRetryTimer =
-			    time(NULL) + INTERVAL_CONNECTRETRY;
+			timer_set(peer, Timer_ConnectRetry,
+			    INTERVAL_CONNECTRETRY);
 			session_close_connection(peer);
 			change_state(peer, STATE_ACTIVE, event);
 			break;
 		case EVNT_TIMER_CONNRETRY:
-			peer->ConnectRetryTimer =
-			    time(NULL) + INTERVAL_CONNECTRETRY;
+			timer_set(peer, Timer_ConnectRetry,
+			    INTERVAL_CONNECTRETRY);
 			session_connect(peer);
 			break;
 		default:
@@ -714,20 +713,20 @@ bgp_fsm(struct peer *peer, enum session_events event)
 		case EVNT_CON_OPEN:
 			session_tcp_established(peer);
 			session_open(peer);
-			peer->ConnectRetryTimer = 0;
+			timer_stop(peer, Timer_ConnectRetry);
 			peer->holdtime = INTERVAL_HOLD_INITIAL;
 			start_timer_holdtime(peer);
 			change_state(peer, STATE_OPENSENT, event);
 			break;
 		case EVNT_CON_OPENFAIL:
-			peer->ConnectRetryTimer =
-			    time(NULL) + INTERVAL_CONNECTRETRY;
+			timer_set(peer, Timer_ConnectRetry,
+			    INTERVAL_CONNECTRETRY);
 			session_close_connection(peer);
 			change_state(peer, STATE_ACTIVE, event);
 			break;
 		case EVNT_TIMER_CONNRETRY:
-			peer->ConnectRetryTimer =
-			    time(NULL) + peer->holdtime;
+			timer_set(peer, Timer_ConnectRetry,
+			    peer->holdtime);
 			change_state(peer, STATE_CONNECT, event);
 			session_connect(peer);
 			break;
@@ -747,8 +746,8 @@ bgp_fsm(struct peer *peer, enum session_events event)
 			break;
 		case EVNT_CON_CLOSED:
 			session_close_connection(peer);
-			peer->ConnectRetryTimer =
-			    time(NULL) + INTERVAL_CONNECTRETRY;
+			timer_set(peer, Timer_ConnectRetry,
+			    INTERVAL_CONNECTRETRY);
 			change_state(peer, STATE_ACTIVE, event);
 			break;
 		case EVNT_CON_FATAL:
@@ -770,7 +769,7 @@ bgp_fsm(struct peer *peer, enum session_events event)
 			if (parse_notification(peer)) {
 				change_state(peer, STATE_IDLE, event);
 				/* don't punish, capa negotiation */
-				peer->IdleHoldTimer = time(NULL);
+				timer_set(peer, Timer_IdleHold, 0);
 				peer->IdleHoldTime /= 2;
 			} else
 				change_state(peer, STATE_IDLE, event);
@@ -860,30 +859,22 @@ bgp_fsm(struct peer *peer, enum session_events event)
 	}
 }
 
-int
-timer_due(time_t timer)
-{
-	if (timer > 0 && timer <= time(NULL))
-		return (1);
-	return (0);
-}
-
 void
 start_timer_holdtime(struct peer *peer)
 {
 	if (peer->holdtime > 0)
-		peer->HoldTimer = time(NULL) + peer->holdtime;
+		timer_set(peer, Timer_Hold, peer->holdtime);
 	else
-		peer->HoldTimer = 0;
+		timer_stop(peer, Timer_Hold);
 }
 
 void
 start_timer_keepalive(struct peer *peer)
 {
 	if (peer->holdtime > 0)
-		peer->KeepaliveTimer = time(NULL) + peer->holdtime / 3;
+		timer_set(peer, Timer_Keepalive, peer->holdtime / 3);
 	else
-		peer->KeepaliveTimer = 0;
+		timer_stop(peer, Timer_Keepalive);
 }
 
 void
@@ -924,10 +915,10 @@ change_state(struct peer *peer, enum session_state state,
 		if (peer->IdleHoldTime == 0)
 			peer->IdleHoldTime = INTERVAL_IDLE_HOLD_INITIAL;
 		peer->holdtime = INTERVAL_HOLD_INITIAL;
-		peer->ConnectRetryTimer = 0;
-		peer->KeepaliveTimer = 0;
-		peer->HoldTimer = 0;
-		peer->IdleHoldResetTimer = 0;
+		timer_stop(peer, Timer_ConnectRetry);
+		timer_stop(peer, Timer_Keepalive);
+		timer_stop(peer, Timer_Hold);
+		timer_stop(peer, Timer_IdleHoldReset);
 		session_close_connection(peer);
 		msgbuf_clear(&peer->wbuf);
 		free(peer->rbuf);
@@ -936,7 +927,7 @@ change_state(struct peer *peer, enum session_state state,
 		if (peer->state == STATE_ESTABLISHED)
 			session_down(peer);
 		if (event != EVNT_STOP) {
-			peer->IdleHoldTimer = time(NULL) + peer->IdleHoldTime;
+			timer_set(peer, Timer_IdleHold, peer->IdleHoldTime);
 			if (event != EVNT_NONE &&
 			    peer->IdleHoldTime < MAX_IDLE_HOLD/2)
 				peer->IdleHoldTime *= 2;
@@ -959,7 +950,7 @@ change_state(struct peer *peer, enum session_state state,
 	case STATE_OPENCONFIRM:
 		break;
 	case STATE_ESTABLISHED:
-		peer->IdleHoldResetTimer = time(NULL) + peer->IdleHoldTime;
+		timer_set(peer, Timer_IdleHoldReset, peer->IdleHoldTime);
 		session_up(peer);
 		break;
 	default:		/* something seriously fucked */
@@ -1000,11 +991,13 @@ session_accept(int listenfd)
 
 	p = getpeerbyip((struct sockaddr *)&cliaddr);
 
-	if (p != NULL && p->state == STATE_IDLE && p->errcnt < 2 &&
-	    p->IdleHoldTimer > 0) {
-		/* fast reconnect after clear */
-		p->passive = 1;
-		bgp_fsm(p, EVNT_START);
+	if (p != NULL && p->state == STATE_IDLE && p->errcnt < 2) {
+		time_t	*t = timer_get(p, Timer_IdleHold);
+		if (*t > 0) {
+			/* fast reconnect after clear */
+			p->passive = 1;
+			bgp_fsm(p, EVNT_START);
+		}
 	}
 
 	if (p != NULL &&
@@ -1972,7 +1965,7 @@ parse_open(struct peer *peer)
 			session_notification(peer, ERR_OPEN, ERR_OPEN_OPT,
 				NULL, 0);
 			change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN);
-			peer->IdleHoldTimer = time(NULL);	/* no punish */
+			timer_set(peer, Timer_IdleHold, 0);	/* no punish */
 			peer->IdleHoldTime /= 2;
 			return (-1);
 		}
@@ -2532,9 +2525,8 @@ session_dispatch_imsg(struct imsgbuf *ibuf, int idx, u_int *listener_cnt)
 				case ERR_CEASE_MAX_PREFIX:
 					bgp_fsm(p, EVNT_STOP);
 					if (p->conf.max_prefix_restart)
-						p->IdleHoldTimer =
-						    time(NULL) + 60 *
-						    p->conf.max_prefix_restart;
+						timer_set(p, Timer_IdleHold, 60 *
+						    p->conf.max_prefix_restart);
 					break;
 				default:
 					bgp_fsm(p, EVNT_CON_FATAL);
diff --git a/usr.sbin/bgpd/session.h b/usr.sbin/bgpd/session.h
index f6973ef2f60..7e823497d22 100644
--- a/usr.sbin/bgpd/session.h
+++ b/usr.sbin/bgpd/session.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: session.h,v 1.92 2007/04/23 13:04:24 claudio Exp $ */
+/*	$OpenBSD: session.h,v 1.93 2007/12/20 17:08:48 henning Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -204,6 +204,15 @@ struct peer {
 
 struct peer	*peers;
 
+enum Timer {
+	Timer_None,
+	Timer_ConnectRetry,
+	Timer_Keepalive,
+	Timer_Hold,
+	Timer_IdleHold,
+	Timer_IdleHoldReset
+};
+
 /* session.c */
 void		 session_socket_blockmode(int, enum blockmodes);
 pid_t		 session_main(struct bgpd_config *, struct peer *,
@@ -259,3 +268,10 @@ int	 carp_demote_init(char *, int);
 void	 carp_demote_shutdown(void);
 int	 carp_demote_get(char *);
 int	 carp_demote_set(char *, int);
+
+/* timer.c */
+time_t		*timer_get(struct peer *, enum Timer);
+int		 timer_due(struct peer *, enum Timer);
+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);
diff --git a/usr.sbin/bgpd/timer.c b/usr.sbin/bgpd/timer.c
new file mode 100644
index 00000000000..07c89bc29c1
--- /dev/null
+++ b/usr.sbin/bgpd/timer.c
@@ -0,0 +1,81 @@
+/*	$OpenBSD: timer.c,v 1.1 2007/12/20 17:08:48 henning Exp $ */
+
+/*
+ * Copyright (c) 2003-2007 Henning Brauer <henning@openbsd.org>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <sys/param.h>
+#include <sys/types.h>
+
+#include "bgpd.h"
+#include "session.h"
+
+time_t *
+timer_get(struct peer *p, enum Timer timer)
+{
+	switch (timer) {
+	case Timer_None:
+		fatal("timer_get called with Timer_None");
+	case Timer_ConnectRetry:
+		return (&p->ConnectRetryTimer);
+	case Timer_Keepalive:
+		return (&p->KeepaliveTimer);
+	case Timer_Hold:
+		return (&p->HoldTimer);
+	case Timer_IdleHold:
+		return (&p->IdleHoldTimer);
+	case Timer_IdleHoldReset:
+		return (&p->IdleHoldResetTimer);
+	}
+
+	fatal("King Bula lost in time");
+}
+
+int
+timer_due(struct peer *p, enum Timer timer)
+{
+	time_t	*t = timer_get(p, timer);
+
+	if (*t > 0 && *t <= time(NULL))
+		return (1);
+	return (0);
+}
+
+int
+timer_running(struct peer *p, enum Timer timer, time_t *left)
+{
+	time_t	*t = timer_get(p, timer);
+
+	*left = *t - time(NULL);
+	if (*t > 0)
+		return (1);
+	return (0);
+}
+
+void
+timer_set(struct peer *p, enum Timer timer, u_int offset)
+{
+	time_t	*t = timer_get(p, timer);
+
+	*t = time(NULL) + offset;
+}
+
+void
+timer_stop(struct peer *p, enum Timer timer)
+{
+	time_t	*t = timer_get(p, timer);
+
+	*t = 0;
+}