When graceful restart is active on a multi-AFI/SAFI session then the first
EoR will clear the refresh timer and so stale routes can remain in the
other AFI/SAFI RIB (if the neighbor does not finish and sends an EoR
message for those too).

The timer is global for all AFI/SAFI. So instead of clearing the
CAPA_GR_RESTARTING flag introduce a CAPA_GR_FINISHED flag that
indicates that the restart handling was done.

Now for this to work some extra things had to be adjusted:
- session_graceful_restart() needs to ensure CAPA_GR_FINISHED does not
remain set. This is done by just setting CAPA_GR_RESTARTING or clearing
flags (capa_neg_calc only checks CAPA_GR_RESTARTING | CAPA_GR_FINISHED).
- Always check for (flags & (CAPA_GR_RESTARTING | CAPA_GR_FINISHED)) ==
CAPA_GR_RESTARTING to know that the retart is still going.
- never stop the Restart Timer and stop clearing CAPA_GR_RESTARTING
- In parse_capabilities() the code was not quite right. First always
start with a clean slate, since 'the receiver ... MUST ignore all but
the last instance'. Also handle the N flag when no AFI/SAFI pairs are
present (helper only role).
- In capa_neg_calc() only write the capa.neg data if grestart is actually
turned on on our side. Normally capa.neg.grestart.restart is first
checked and so data in capa.neg.grestart.flag wont matter but I still find
this a bit cleaner.

What is currently still missing is proper handling of the CAPA_GR_R_FLAG
when announcing the capability. The current code is OK but not quite
right.
-- 
:wq Claudio


Index: bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
diff -u -p -r1.548 bgpd.h
--- bgpd.h	21 Sep 2026 19:06:16 -0000	1.548
+++ bgpd.h	24 Sep 2026 09:34:50 -0000
@@ -471,6 +471,7 @@ enum capa_codes {
 #define	CAPA_GR_RESTART		0x02
 #define	CAPA_GR_FORWARD		0x04
 #define	CAPA_GR_RESTARTING	0x08
+#define	CAPA_GR_FINISHED	0x10
 #define	CAPA_GR_TIMEMASK	0x0fff
 #define	CAPA_GR_R_FLAG		0x8000
 #define	CAPA_GR_N_FLAG		0x4000
Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
diff -u -p -r1.541 session.c
--- session.c	14 Sep 2026 13:27:14 -0000	1.541
+++ session.c	24 Sep 2026 13:04:18 -0000
@@ -1065,7 +1065,7 @@ session_graceful_restart(struct peer *p)
 			log_peer_warnx(&p->conf,
 			    "graceful restart of %s, keeping routes",
 			    aid2str(i));
-			p->capa.neg.grestart.flags[i] |= CAPA_GR_RESTARTING;
+			p->capa.neg.grestart.flags[i] = CAPA_GR_RESTARTING;
 		} else if (p->capa.neg.mp[i]) {
 			imsg_rde(IMSG_SESSION_NOGRACE, p->conf.id,
 			    &i, sizeof(i));
@@ -1087,9 +1087,10 @@ session_graceful_stop(struct peer *p)
 		 * In all other cases the session was already flushed when the
 		 * session went down or when the new open message was parsed.
 		 */
-		if (p->capa.neg.grestart.flags[i] & CAPA_GR_RESTARTING)
+		if ((p->capa.neg.grestart.flags[i] & (CAPA_GR_RESTARTING |
+		    CAPA_GR_FINISHED)) == CAPA_GR_RESTARTING)
 			session_graceful_flush(p, i, "time-out");
-		p->capa.neg.grestart.flags[i] &= ~CAPA_GR_RESTARTING;
+		p->capa.neg.grestart.flags[i] |= CAPA_GR_FINISHED;
 	}
 }
 
@@ -1552,18 +1553,18 @@ session_dispatch_imsg(struct imsgbuf *im
 			}
 			if (aid < AID_MIN || aid >= AID_MAX)
 				fatalx("IMSG_SESSION_RESTARTED: bad AID");
-			if (p->capa.neg.grestart.flags[aid] &
+			if ((p->capa.neg.grestart.flags[aid] &
+			    (CAPA_GR_RESTARTING | CAPA_GR_FINISHED)) ==
 			    CAPA_GR_RESTARTING) {
 				log_peer_warnx(&p->conf,
 				    "graceful restart of %s finished",
 				    aid2str(aid));
-				p->capa.neg.grestart.flags[aid] &=
-				    ~CAPA_GR_RESTARTING;
-				timer_stop(&p->timers, Timer_RestartTimeout);
-
 				/* signal back to RDE to cleanup stale routes */
 				imsg_rde(IMSG_SESSION_RESTARTED,
 				    peerid, &aid, sizeof(aid));
+
+				p->capa.neg.grestart.flags[aid] |=
+				    CAPA_GR_FINISHED;
 			}
 			break;
 		default:
Index: session_bgp.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session_bgp.c,v
diff -u -p -r1.13 session_bgp.c
--- session_bgp.c	30 Aug 2026 23:43:23 -0000	1.13
+++ session_bgp.c	24 Sep 2026 13:02:38 -0000
@@ -769,16 +769,12 @@ parse_capabilities(struct peer *peer, st
 			peer->remote_role = capa2role(role);
 			break;
 		case CAPA_RESTART:
-			if (capa_len == 2) {
-				/* peer only supports EoR marker */
-				peer->capa.peer.grestart.restart = 1;
-				peer->capa.peer.grestart.timeout = 0;
-				break;
-			} else if (capa_len % 4 != 2) {
+			memset(&peer->capa.peer.grestart, 0,
+			    sizeof(peer->capa.peer.grestart));
+
+			if (capa_len % 4 != 2) {
 				log_peer_warnx(&peer->conf,
 				    "Bad graceful restart capability");
-				peer->capa.peer.grestart.restart = 0;
-				peer->capa.peer.grestart.timeout = 0;
 				break;
 			}
 
@@ -786,8 +782,17 @@ parse_capabilities(struct peer *peer, st
  bad_gr_restart:
 				log_peer_warnx(&peer->conf,
 				    "Bad graceful restart capability");
-				peer->capa.peer.grestart.restart = 0;
-				peer->capa.peer.grestart.timeout = 0;
+				memset(&peer->capa.peer.grestart, 0,
+				    sizeof(peer->capa.peer.grestart));
+				break;
+			}
+
+			if (gr_header & CAPA_GR_N_FLAG)
+				peer->capa.peer.grestart.grnotification = 1;
+
+			if (capa_len == 2) {
+				/* peer only supports EoR marker */
+				peer->capa.peer.grestart.restart = 1;
 				break;
 			}
 
@@ -796,7 +801,8 @@ parse_capabilities(struct peer *peer, st
 			if (peer->capa.peer.grestart.timeout == 0) {
 				log_peer_warnx(&peer->conf, "Received "
 				    "graceful restart with zero timeout");
-				peer->capa.peer.grestart.restart = 0;
+				memset(&peer->capa.peer.grestart, 0,
+				    sizeof(peer->capa.peer.grestart));
 				break;
 			}
 
@@ -812,7 +818,7 @@ parse_capabilities(struct peer *peer, st
 					    afi, safi);
 					continue;
 				}
-				peer->capa.peer.grestart.flags[aid] |=
+				peer->capa.peer.grestart.flags[aid] =
 				    CAPA_GR_PRESENT;
 				if (flags & CAPA_GR_F_FLAG)
 					peer->capa.peer.grestart.flags[aid] |=
@@ -822,8 +828,6 @@ parse_capabilities(struct peer *peer, st
 					    CAPA_GR_RESTART;
 				peer->capa.peer.grestart.restart = 2;
 			}
-			if (gr_header & CAPA_GR_N_FLAG)
-				peer->capa.peer.grestart.grnotification = 1;
 			break;
 		case CAPA_AS4BYTE:
 			if (capa_len != 4 ||
@@ -1318,24 +1322,26 @@ capa_neg_calc(struct peer *p)
 	 */
 
 	for (i = AID_MIN; i < AID_MAX; i++) {
-		int8_t	negflags;
+		int8_t	negflags, peerflags;
 
 		/* disable GR if the AFI/SAFI is not present */
-		if ((p->capa.peer.grestart.flags[i] & CAPA_GR_PRESENT &&
-		    p->capa.neg.mp[i] == 0))
+		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) {
+		peerflags = p->capa.peer.grestart.flags[i];
+		if ((negflags & (CAPA_GR_RESTARTING | CAPA_GR_FINISHED)) ==
+		    CAPA_GR_RESTARTING) {
 			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;
+			    peerflags & CAPA_GR_FORWARD) {
+				peerflags |= CAPA_GR_RESTARTING;
 			} else {
 				session_graceful_flush(p, i, "not restarted");
 			}
 		}
+		if (p->capa.ann.grestart.restart != 0)
+			p->capa.neg.grestart.flags[i] = peerflags;
 	}
 	p->capa.neg.grestart.timeout = p->capa.peer.grestart.timeout;
 	p->capa.neg.grestart.restart = p->capa.peer.grestart.restart;