The BGP protocol has a keepalive packet which resets the hold timer when a
packet is received. The problem is this covers only one side of the
transmission. It seems that some BGP implementations fail to process
messages in some cases but still send out KEEPALIVE packets. So bgpd
thinks everything is fine even though no updates where processed by the
other side (including our KEEPALIVE packets). The session is stuck in
limbo and with it some prefixes and routes.
Because of this I think it makes sense to add a send hold timer that is
reset whenever a write call to the socket is made. If a socket does not
become writable for holdtime seconds (90s by default) then the session is
reset similar to the hold timer expiring because no data was received.
This send holdtimer is not part of the BGP spec right now but looking at
discussions on the IDR mailing list I assume something like this may be
added at one point.
I would like to know what other people think and would especially like to
know if this diff causes session resets that should not happen.
Cheers
--
:wq Claudio
Index: bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.405
diff -u -p -r1.405 bgpd.h
--- bgpd.h 5 Nov 2020 11:52:59 -0000 1.405
+++ bgpd.h 13 Dec 2020 14:56:47 -0000
@@ -1467,6 +1467,7 @@ static const char * const timernames[] =
"ConnectRetryTimer",
"KeepaliveTimer",
"HoldTimer",
+ "SendHoldTimer",
"IdleHoldTimer",
"IdleHoldResetTimer",
"CarpUndemoteTimer",
Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
retrieving revision 1.406
diff -u -p -r1.406 session.c
--- session.c 11 Dec 2020 12:00:01 -0000 1.406
+++ session.c 13 Dec 2020 14:52:42 -0000
@@ -373,6 +373,7 @@ session_main(int debug, int verbose)
if ((pt = timer_nextisdue(&p->timers, now)) != NULL) {
switch (pt->type) {
case Timer_Hold:
+ case Timer_HoldSend:
bgp_fsm(p, EVNT_TIMER_HOLDTIME);
break;
case Timer_ConnectRetry:
@@ -597,6 +598,7 @@ bgp_fsm(struct peer *peer, enum session_
switch (event) {
case EVNT_START:
timer_stop(&peer->timers, Timer_Hold);
+ timer_stop(&peer->timers, Timer_HoldSend);
timer_stop(&peer->timers, Timer_Keepalive);
timer_stop(&peer->timers, Timer_IdleHold);
@@ -875,6 +877,7 @@ change_state(struct peer *peer, enum ses
timer_stop(&peer->timers, Timer_ConnectRetry);
timer_stop(&peer->timers, Timer_Keepalive);
timer_stop(&peer->timers, Timer_Hold);
+ timer_stop(&peer->timers, Timer_HoldSend);
timer_stop(&peer->timers, Timer_IdleHold);
timer_stop(&peer->timers, Timer_IdleHoldReset);
session_close_connection(peer);
@@ -923,6 +926,7 @@ change_state(struct peer *peer, enum ses
timer_stop(&peer->timers, Timer_ConnectRetry);
timer_stop(&peer->timers, Timer_Keepalive);
timer_stop(&peer->timers, Timer_Hold);
+ timer_stop(&peer->timers, Timer_HoldSend);
timer_stop(&peer->timers, Timer_IdleHold);
timer_stop(&peer->timers, Timer_IdleHoldReset);
session_close_connection(peer);
@@ -1780,6 +1784,8 @@ session_dispatch_msg(struct pollfd *pfd,
return (1);
}
p->stats.last_write = getmonotime();
+ if (p->holdtime > 0)
+ timer_set(&p->timers, Timer_HoldSend, p->holdtime);
if (p->throttled && p->wbuf.queued < SESS_MSG_LOW_MARK) {
if (imsg_rde(IMSG_XON, p->conf.id, NULL, 0) == -1)
log_peer_warn(&p->conf, "imsg_compose XON");
Index: session.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.h,v
retrieving revision 1.148
diff -u -p -r1.148 session.h
--- session.h 11 Dec 2020 12:00:01 -0000 1.148
+++ session.h 13 Dec 2020 14:52:17 -0000
@@ -180,6 +180,7 @@ enum Timer {
Timer_ConnectRetry,
Timer_Keepalive,
Timer_Hold,
+ Timer_HoldSend,
Timer_IdleHold,
Timer_IdleHoldReset,
Timer_CarpUndemote,
