From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: bgpd: reduce use of global conf in the SE
To: tech@openbsd.org
Date: Wed, 26 Feb 2025 10:56:33 +0100

The session engine has a global conf symbol that contains the bgpd_config.
This is a easy trap and something I like to change in the long run.

Here is a diff that that reduces the use of conf by storing some values
inside the peer (the bgpid and the connectretry time).

This is from a larger diff I sent out but I decided to move the
connectretry time to the peer_config so if we want we can have per peer
connectretry times.
-- 
:wq Claudio

Index: bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
diff -u -p -r1.515 bgpd.h
--- bgpd.h	20 Feb 2025 19:47:31 -0000	1.515
+++ bgpd.h	26 Feb 2025 09:47:16 -0000
@@ -507,6 +507,7 @@ struct peer_config {
 	uint16_t		 max_out_prefix_restart;
 	uint16_t		 holdtime;
 	uint16_t		 min_holdtime;
+	uint16_t		 connectretry;
 	uint16_t		 staletime;
 	uint16_t		 local_short_as;
 	uint16_t		 remote_port;
Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
diff -u -p -r1.519 session.c
--- session.c	26 Feb 2025 09:33:37 -0000	1.519
+++ session.c	26 Feb 2025 09:47:16 -0000
@@ -599,6 +599,9 @@ init_peer(struct peer *p, struct bgpd_co
 		p->conf.holdtime = c->holdtime;
 	if (p->conf.min_holdtime == 0)
 		p->conf.min_holdtime = c->min_holdtime;
+	if (p->conf.connectretry == 0)
+		p->conf.connectretry = c->connectretry;
+	p->local_bgpid = c->bgpid;
 
 	peer_cnt++;
 
@@ -643,7 +646,7 @@ bgp_fsm(struct peer *peer, enum session_
 			} else {
 				change_state(peer, STATE_CONNECT, event);
 				timer_set(&peer->timers, Timer_ConnectRetry,
-				    conf->connectretry);
+				    peer->conf.connectretry);
 				session_connect(peer);
 			}
 			peer->passive = 0;
@@ -671,13 +674,13 @@ bgp_fsm(struct peer *peer, enum session_
 			break;
 		case EVNT_CON_OPENFAIL:
 			timer_set(&peer->timers, Timer_ConnectRetry,
-			    conf->connectretry);
+			    peer->conf.connectretry);
 			session_close_connection(peer);
 			change_state(peer, STATE_ACTIVE, event);
 			break;
 		case EVNT_TIMER_CONNRETRY:
 			timer_set(&peer->timers, Timer_ConnectRetry,
-			    conf->connectretry);
+			    peer->conf.connectretry);
 			session_connect(peer);
 			break;
 		default:
@@ -700,7 +703,7 @@ bgp_fsm(struct peer *peer, enum session_
 			break;
 		case EVNT_CON_OPENFAIL:
 			timer_set(&peer->timers, Timer_ConnectRetry,
-			    conf->connectretry);
+			    peer->conf.connectretry);
 			session_close_connection(peer);
 			change_state(peer, STATE_ACTIVE, event);
 			break;
@@ -726,7 +729,7 @@ bgp_fsm(struct peer *peer, enum session_
 		case EVNT_CON_CLOSED:
 			session_close_connection(peer);
 			timer_set(&peer->timers, Timer_ConnectRetry,
-			    conf->connectretry);
+			    peer->conf.connectretry);
 			change_state(peer, STATE_ACTIVE, event);
 			break;
 		case EVNT_CON_FATAL:
@@ -1640,7 +1643,7 @@ session_open(struct peer *p)
 	errs += ibuf_add_n16(buf, p->conf.local_short_as);
 	errs += ibuf_add_n16(buf, p->conf.holdtime);
 	/* is already in network byte order */
-	errs += ibuf_add_n32(buf, conf->bgpid);
+	errs += ibuf_add_n32(buf, p->local_bgpid);
 	errs += ibuf_add_n8(buf, optparamlen);
 
 	if (extlen) {
@@ -2369,9 +2372,9 @@ parse_open(struct peer *peer, struct ibu
 	}
 
 	/* on iBGP sessions check for bgpid collision */
-	if (!peer->conf.ebgp && peer->remote_bgpid == conf->bgpid) {
+	if (!peer->conf.ebgp && peer->remote_bgpid == peer->local_bgpid) {
 		struct in_addr ina;
-		ina.s_addr = htonl(bgpid);
+		ina.s_addr = htonl(peer->remote_bgpid);
 		log_peer_warnx(&peer->conf, "peer BGPID %s conflicts with ours",
 		    inet_ntoa(ina));
 		session_notification(peer, ERR_OPEN, ERR_OPEN_BGPID, NULL);
@@ -3849,9 +3852,13 @@ merge_peers(struct bgpd_config *c, struc
 
 		/* reapply holdtime and min_holdtime settings */
 		if (p->conf.holdtime == 0)
-			p->conf.holdtime = conf->holdtime;
+			p->conf.holdtime = nc->holdtime;
 		if (p->conf.min_holdtime == 0)
-			p->conf.min_holdtime = conf->min_holdtime;
+			p->conf.min_holdtime = nc->min_holdtime;
+		if (p->conf.connectretry == 0)
+			p->conf.connectretry = nc->connectretry;
+		p->local_bgpid = nc->bgpid;
+
 
 		/* had demotion, is demoted, demote removed? */
 		if (p->demoted && !p->conf.demote_group[0])
Index: session.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.h,v
diff -u -p -r1.187 session.h
--- session.h	20 Feb 2025 19:47:31 -0000	1.187
+++ session.h	26 Feb 2025 09:47:16 -0000
@@ -216,6 +216,7 @@ struct peer {
 	u_int			 errcnt;
 	u_int			 IdleHoldTime;
 	unsigned int		 if_scope;	/* interface scope for IPv6 */
+	uint32_t		 local_bgpid;
 	uint32_t		 remote_bgpid;
 	enum session_state	 state;
 	enum session_state	 prev_state;