From: claudio <claudio@openbsd.org>
Date: Thu, 27 May 2021 07:52:54 +0000 (+0000)
Subject: Improve graceful restart capability handling.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=d10dbd9f44783fefb500012a1165c8ab2f19ee60;p=openbsd

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.
---

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;