Move setting of the shutdown reason to session_stop()

Also make sure that something is logged when a session is stopped.
Part of a bigger diff which was OK tb@

3 files changed, 31 insertions, 27 deletions

diff --git a/usr.sbin/bgpd/control.c b/usr.sbin/bgpd/control.c
index 43a9816cbdc..49fd55ad110 100644
--- a/usr.sbin/bgpd/control.c
+++ b/usr.sbin/bgpd/control.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: control.c,v 1.116 2024/01/11 15:46:25 claudio Exp $ */
+/*	$OpenBSD: control.c,v 1.117 2024/04/22 09:36:04 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -393,31 +393,28 @@ control_dispatch_msg(struct pollfd *pfd, struct peer_head *peers)
 				case IMSG_CTL_NEIGHBOR_DOWN:
 					neighbor.reason[
 					    sizeof(neighbor.reason) - 1] = '\0';
-					strlcpy(p->conf.reason,
-					    neighbor.reason,
-					    sizeof(p->conf.reason));
 					p->conf.down = 1;
-					session_stop(p, ERR_CEASE_ADMIN_DOWN);
+					session_stop(p, ERR_CEASE_ADMIN_DOWN,
+					    neighbor.reason);
 					control_result(c, CTL_RES_OK);
 					break;
 				case IMSG_CTL_NEIGHBOR_CLEAR:
 					neighbor.reason[
 					    sizeof(neighbor.reason) - 1] = '\0';
-					strlcpy(p->conf.reason,
-					    neighbor.reason,
-					    sizeof(p->conf.reason));
 					p->IdleHoldTime =
 					    INTERVAL_IDLE_HOLD_INITIAL;
 					p->errcnt = 0;
 					if (!p->conf.down) {
 						session_stop(p,
-						    ERR_CEASE_ADMIN_RESET);
+						    ERR_CEASE_ADMIN_RESET,
+						    neighbor.reason);
 						timer_set(&p->timers,
 						    Timer_IdleHold,
 						    SESSION_CLEAR_DELAY);
 					} else {
 						session_stop(p,
-						    ERR_CEASE_ADMIN_DOWN);
+						    ERR_CEASE_ADMIN_DOWN,
+						    neighbor.reason);
 					}
 					control_result(c, CTL_RES_OK);
 					break;
diff --git a/usr.sbin/bgpd/session.c b/usr.sbin/bgpd/session.c
index ef1b14cbbb3..751d9627db7 100644
--- a/usr.sbin/bgpd/session.c
+++ b/usr.sbin/bgpd/session.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: session.c,v 1.471 2024/04/22 08:53:59 claudio Exp $ */
+/*	$OpenBSD: session.c,v 1.472 2024/04/22 09:36:04 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004, 2005 Henning Brauer <henning@openbsd.org>
@@ -266,11 +266,12 @@ session_main(int debug, int verbose)
 					if (p->demoted)
 						session_demote(p, -1);
 					p->conf.demote_group[0] = 0;
-					session_stop(p, ERR_CEASE_PEER_UNCONF);
+					session_stop(p, ERR_CEASE_PEER_UNCONF,
+					    NULL);
 					timer_remove_all(&p->timers);
 					tcp_md5_del_listener(conf, p);
-					log_peer_warnx(&p->conf, "removed");
 					RB_REMOVE(peer_head, &conf->peers, p);
+					log_peer_warnx(&p->conf, "removed");
 					free(p);
 					peer_cnt--;
 					continue;
@@ -513,12 +514,10 @@ session_main(int debug, int verbose)
 	}
 
 	RB_FOREACH_SAFE(p, peer_head, &conf->peers, next) {
-		RB_REMOVE(peer_head, &conf->peers, p);
-		strlcpy(p->conf.reason,
-		    "bgpd shutting down",
-		    sizeof(p->conf.reason));
-		session_stop(p, ERR_CEASE_ADMIN_DOWN);
+		session_stop(p, ERR_CEASE_ADMIN_DOWN, "bgpd shutting down");
 		timer_remove_all(&p->timers);
+		tcp_md5_del_listener(conf, p);
+		RB_REMOVE(peer_head, &conf->peers, p);
 		free(p);
 	}
 
@@ -3150,7 +3149,8 @@ session_dispatch_imsg(struct imsgbuf *imsgbuf, int idx, u_int *listener_cnt)
 					} else if (!depend_ok && p->depend_ok) {
 						p->depend_ok = depend_ok;
 						session_stop(p,
-						    ERR_CEASE_OTHER_CHANGE);
+						    ERR_CEASE_OTHER_CHANGE,
+						    NULL);
 					}
 				}
 			break;
@@ -3616,21 +3616,21 @@ session_demote(struct peer *p, int level)
 }
 
 void
-session_stop(struct peer *peer, uint8_t subcode)
+session_stop(struct peer *peer, uint8_t subcode, const char *reason)
 {
 	struct ibuf *ibuf;
-	char *communication;
 
-	communication = peer->conf.reason;
+	if (reason != NULL)
+		strlcpy(peer->conf.reason, reason, sizeof(peer->conf.reason));
 
 	ibuf = ibuf_dynamic(0, REASON_LEN);
 
 	if ((subcode == ERR_CEASE_ADMIN_DOWN ||
 	    subcode == ERR_CEASE_ADMIN_RESET) &&
-	    communication != NULL && *communication != '\0' &&
+	    reason != NULL && *reason != '\0' &&
 	    ibuf != NULL) {
-		if (ibuf_add_n8(ibuf, strlen(communication)) == -1 ||
-		    ibuf_add(ibuf, communication, strlen(communication))) {
+		if (ibuf_add_n8(ibuf, strlen(reason)) == -1 ||
+		    ibuf_add(ibuf, reason, strlen(reason))) {
 			log_peer_warnx(&peer->conf,
 			    "trying to send overly long shutdown reason");
 			ibuf_free(ibuf);
@@ -3645,6 +3645,13 @@ session_stop(struct peer *peer, uint8_t subcode)
 		break;
 	default:
 		/* session not open, no need to send notification */
+		if (subcode >= sizeof(suberr_cease_names) / sizeof(char *) ||
+		    suberr_cease_names[subcode] == NULL)
+			log_peer_warnx(&peer->conf, "session stop: %s, "
+			    "unknown subcode %u", errnames[ERR_CEASE], subcode);
+		else
+			log_peer_warnx(&peer->conf, "session stop: %s, %s",
+			    errnames[ERR_CEASE], suberr_cease_names[subcode]);
 		break;
 	}
 	ibuf_free(ibuf);
diff --git a/usr.sbin/bgpd/session.h b/usr.sbin/bgpd/session.h
index 1f459b79dcf..598ce70a109 100644
--- a/usr.sbin/bgpd/session.h
+++ b/usr.sbin/bgpd/session.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: session.h,v 1.168 2024/03/22 07:19:28 claudio Exp $ */
+/*	$OpenBSD: session.h,v 1.169 2024/04/22 09:36:04 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -331,7 +331,7 @@ int		 peer_matched(struct peer *, struct ctl_neighbor *);
 int		 imsg_ctl_parent(struct imsg *);
 int		 imsg_ctl_rde(struct imsg *);
 int		 imsg_ctl_rde_msg(int, uint32_t, pid_t);
-void		 session_stop(struct peer *, uint8_t);
+void		 session_stop(struct peer *, uint8_t, const char *);
 
 /* timer.c */
 struct timer	*timer_get(struct timer_head *, enum Timer);