From d10dbd9f44783fefb500012a1165c8ab2f19ee60 Mon Sep 17 00:00:00 2001
From: claudio <claudio@openbsd.org>
Date: Thu, 27 May 2021 07:52:54 +0000
Subject: [PATCH] Improve graceful restart capability handling.

Announce only the graceful restart capability header but do not include any
AFI / SAFI pairs. bgpd does not preserve its forwarding state over restarts
and only implements the "Procedures for the Receiving Speaker".

When calculating the negotiated capabilities do not override the peer
capabilities (AFI / SAFI bits), just make sure the negotiated bits are
cleared. With this the peer capabilities are properly shown in bgpctl.
---
 usr.sbin/bgpd/session.c | 49 ++++++++++++-----------------------------
 1 file changed, 14 insertions(+), 35 deletions(-)

diff --git a/usr.sbin/bgpd/session.c b/usr.sbin/bgpd/session.c
index c8acfad57e3..455efa7617a 100644
--- a/usr.sbin/bgpd/session.c
+++ b/usr.sbin/bgpd/session.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: session.c,v 1.415 2021/05/16 09:09:11 claudio Exp $ */
+/*	$OpenBSD: session.c,v 1.416 2021/05/27 07:52:54 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004, 2005 Henning Brauer <henning@openbsd.org>
@@ -1443,41 +1443,20 @@ session_open(struct peer *p)
 	/* graceful restart and End-of-RIB marker, RFC 4724 */
 	if (p->capa.ann.grestart.restart) {
 		int		rst = 0;
-		u_int16_t	hdr;
-		u_int8_t	grlen;
-
-		if (mpcapa) {
-			grlen = 2 + 4 * mpcapa;
-			for (i = 0; i < AID_MAX; i++) {
-				if (p->capa.neg.grestart.flags[i] &
-				    CAPA_GR_RESTARTING)
-					rst++;
-			}
-		} else {	/* AID_INET */
-			grlen = 2 + 4;
-			if (p->capa.neg.grestart.flags[AID_INET] &
-			    CAPA_GR_RESTARTING)
+		u_int16_t	hdr = 0;
+
+		for (i = 0; i < AID_MAX; i++) {
+			if (p->capa.neg.grestart.flags[i] & CAPA_GR_RESTARTING)
 				rst++;
 		}
 
-		hdr = conf->holdtime;		/* default timeout */
-		/* if client does graceful restart don't set R flag */
+		/* Only set the R-flag if no graceful restart is ongoing */
 		if (!rst)
 			hdr |= CAPA_GR_R_FLAG;
 		hdr = htons(hdr);
 
-		errs += session_capa_add(opb, CAPA_RESTART, grlen);
+		errs += session_capa_add(opb, CAPA_RESTART, sizeof(hdr));
 		errs += ibuf_add(opb, &hdr, sizeof(hdr));
-
-		if (mpcapa) {
-			for (i = 0; i < AID_MAX; i++) {
-				if (p->capa.ann.mp[i]) {
-					errs += session_capa_add_gr(p, opb, i);
-				}
-			}
-		} else {	/* AID_INET */
-			errs += session_capa_add_gr(p, opb, AID_INET);
-		}
 	}
 
 	/* 4-bytes AS numbers, draft-ietf-idr-as4bytes-13 */
@@ -2585,24 +2564,24 @@ capa_neg_calc(struct peer *p)
 		int8_t	negflags;
 
 		/* disable GR if the AFI/SAFI is not present */
-		if (p->capa.ann.grestart.restart == 0 ||
-		    (p->capa.peer.grestart.flags[i] & CAPA_GR_PRESENT &&
+		if ((p->capa.peer.grestart.flags[i] & CAPA_GR_PRESENT &&
 		    p->capa.neg.mp[i] == 0))
 			p->capa.peer.grestart.flags[i] = 0;	/* disable */
 		/* look at current GR state and decide what to do */
 		negflags = p->capa.neg.grestart.flags[i];
 		p->capa.neg.grestart.flags[i] = p->capa.peer.grestart.flags[i];
 		if (negflags & CAPA_GR_RESTARTING) {
-			if (!(p->capa.peer.grestart.flags[i] &
-			    CAPA_GR_FORWARD)) {
+			if (p->capa.ann.grestart.restart != 0 ||
+			    p->capa.peer.grestart.flags[i] & CAPA_GR_FORWARD) {
+				p->capa.neg.grestart.flags[i] |=
+				    CAPA_GR_RESTARTING;
+			} else {
 				if (imsg_rde(IMSG_SESSION_FLUSH, p->conf.id,
 				    &i, sizeof(i)) == -1)
 					return (-1);
 				log_peer_warnx(&p->conf, "graceful restart of "
 				    "%s, not restarted, flushing", aid2str(i));
-			} else
-				p->capa.neg.grestart.flags[i] |=
-				    CAPA_GR_RESTARTING;
+			}
 		}
 	}
 	p->capa.neg.grestart.timeout = p->capa.peer.grestart.timeout;
-- 
2.20.1