From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: bgpd: rtr fix startup behaviour with empty cache
To: tech@openbsd.org
Date: Mon, 15 Jan 2024 12:15:15 +0100

During startup (and therefor version negotiation) the cache may respond
with a NO_DATA_AVAILABLE error. So we need to set the version based on
this error PDU's version (the code then triggers a RTR_EVNT_NO_DATA
which moves the state to RTR_STATE_ESTABLISHED). I moved the version
handling back into rtr_parse_header() so all of that is in one place.

When the no data event happens the session_id is still unset. So
rtr_parse_notify() needs to set the session_id when not yet set to avoid
an unnecessary error condition right after.

Last but not least add the needed call to rtr_send_reset_query() in the
RTR_EVNT_TIMER_RETRY case. When the retry timer fires because of the no
data condition the router should probe the cache again to see if there is
data available.

-- 
:wq Claudio

Index: rtr_proto.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rtr_proto.c,v
diff -u -p -r1.31 rtr_proto.c
--- rtr_proto.c	11 Jan 2024 15:38:05 -0000	1.31
+++ rtr_proto.c	15 Jan 2024 11:08:28 -0000
@@ -419,6 +419,7 @@ rtr_parse_header(struct rtr_session *rs,
 {
 	struct rtr_header rh;
 	size_t len;
+	uint16_t errcode;
 
 	if (ibuf_get(hdr, &rh, sizeof(rh)) == -1)
 		fatal("%s: ibuf_get", __func__);
@@ -443,7 +444,14 @@ rtr_parse_header(struct rtr_session *rs,
 			rtr_fsm(rs, RTR_EVNT_NEGOTIATION_DONE);
 			break;
 		case ERROR_REPORT:
-			/* version handled in rtr_parse_error() */
+			errcode = ntohs(rh.session_id);
+			if (errcode == UNSUPP_PROTOCOL_VERS ||
+			    errcode == NO_DATA_AVAILABLE) {
+				if (rh.version < rs->version) {
+					rs->prev_version = rs->version;
+					rs->version = rh.version;
+				}
+			}
 			break;
 		case SERIAL_NOTIFY:
 			/* ignore SERIAL_NOTIFY */
@@ -537,6 +545,10 @@ rtr_parse_notify(struct rtr_session *rs,
 	if (ibuf_get(pdu, &notify, sizeof(notify)) == -1)
 		goto badlen;
 
+	/* set session_id if not yet happened */
+	if (rs->session_id == -1)
+		rs->session_id = ntohs(notify.hdr.session_id);
+
 	if (rtr_check_session_id(rs, rs->session_id, &notify.hdr, pdu) == -1)
 		return -1;
 
@@ -960,10 +972,6 @@ rtr_parse_error(struct rtr_session *rs, 
 		rtr_fsm(rs, RTR_EVNT_NO_DATA);
 		rv = 0;
 	} else if (errcode == UNSUPP_PROTOCOL_VERS) {
-		if (rh.version < rs->version) {
-			rs->prev_version = rs->version;
-			rs->version = rh.version;
-		}
 		rtr_fsm(rs, RTR_EVNT_UNSUPP_PROTO_VERSION);
 		rv = 0;
 	} else
@@ -1126,6 +1134,11 @@ rtr_fsm(struct rtr_session *rs, enum rtr
 			timer_set(&rs->timers, Timer_Rtr_Retry, rs->retry);
 			rtr_imsg_compose(IMSG_SOCKET_CONN, rs->id, 0, NULL, 0);
 			break;
+		case RTR_STATE_ESTABLISHED:
+			if (rs->session_id == -1)
+				rtr_send_reset_query(rs);
+			else
+				rtr_send_serial_query(rs);
 		default:
 			break;
 		}