Download raw body.
bgpd: fix min_holdtime handling and simplify holdtime
Tripped over this today. min_holdtime and holdtime are both in the global
bgpd_config and in the peer_config. If the value in peer_config is 0 then
the global value should be used.
The problem is this is only done for holdtime but not for min_holdtime.
I realy dislike this either or handling inside message processing code
and decided to instead set the peer_config holdtime and min_holdtime value
in init_peer() so the rest of the code does not need to care. Only during
config reloads we need to reapply the values again since the config is
copied over and so it is set back to 0.
The below diff does that.
--
:wq Claudio
Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
diff -u -p -r1.515 session.c
--- session.c 12 Feb 2025 16:49:56 -0000 1.515
+++ session.c 17 Feb 2025 14:57:55 -0000
@@ -56,7 +56,7 @@
void session_sighdlr(int);
int setup_listeners(u_int *);
-void init_peer(struct peer *);
+void init_peer(struct peer *, struct bgpd_config *);
void start_timer_holdtime(struct peer *);
void start_timer_sendholdtime(struct peer *);
void start_timer_keepalive(struct peer *);
@@ -257,7 +257,7 @@ session_main(int debug, int verbose)
RB_FOREACH_SAFE(p, peer_head, &conf->peers, next) {
/* new peer that needs init? */
if (p->state == STATE_NONE)
- init_peer(p);
+ init_peer(p, conf);
/* deletion due? */
if (p->reconf_action == RECONF_DELETE) {
@@ -571,7 +571,7 @@ session_main(int debug, int verbose)
}
void
-init_peer(struct peer *p)
+init_peer(struct peer *p, struct bgpd_config *c)
{
TAILQ_INIT(&p->timers);
p->fd = -1;
@@ -587,6 +587,12 @@ init_peer(struct peer *p)
else
p->depend_ok = 1;
+ /* apply holdtime and min_holdtime settings */
+ if (p->conf.holdtime == 0)
+ p->conf.holdtime = c->holdtime;
+ if (p->conf.min_holdtime == 0)
+ p->conf.min_holdtime = c->min_holdtime;
+
peer_cnt++;
change_state(p, STATE_IDLE, EVNT_NONE);
@@ -1509,7 +1515,6 @@ session_open(struct peer *p)
{
struct ibuf *buf, *opb;
size_t len, optparamlen;
- uint16_t holdtime;
uint8_t i;
int errs = 0, extlen = 0;
int mpcapa = 0;
@@ -1641,14 +1646,9 @@ session_open(struct peer *p)
return;
}
- if (p->conf.holdtime)
- holdtime = p->conf.holdtime;
- else
- holdtime = conf->holdtime;
-
errs += ibuf_add_n8(buf, 4);
errs += ibuf_add_n16(buf, p->conf.local_short_as);
- errs += ibuf_add_n16(buf, holdtime);
+ errs += ibuf_add_n16(buf, p->conf.holdtime);
/* is already in network byte order */
errs += ibuf_add_n32(buf, conf->bgpid);
errs += ibuf_add_n8(buf, optparamlen);
@@ -2231,7 +2231,7 @@ parse_open(struct peer *peer, struct ibu
{
uint8_t version, rversion;
uint16_t short_as;
- uint16_t holdtime, myholdtime;
+ uint16_t holdtime;
uint32_t as, bgpid;
uint8_t optparamlen;
@@ -2264,7 +2264,7 @@ parse_open(struct peer *peer, struct ibu
return (-1);
}
- if (holdtime && holdtime < peer->conf.min_holdtime) {
+ if (holdtime != 0 && holdtime < peer->conf.min_holdtime) {
log_peer_warnx(&peer->conf,
"peer requests unacceptable holdtime %u", holdtime);
session_notification(peer, ERR_OPEN, ERR_OPEN_HOLDTIME, NULL);
@@ -2272,13 +2272,10 @@ parse_open(struct peer *peer, struct ibu
return (-1);
}
- myholdtime = peer->conf.holdtime;
- if (!myholdtime)
- myholdtime = conf->holdtime;
- if (holdtime < myholdtime)
+ if (holdtime < peer->conf.holdtime)
peer->holdtime = holdtime;
else
- peer->holdtime = myholdtime;
+ peer->holdtime = peer->conf.holdtime;
/* check bgpid for validity - just disallow 0 */
if (bgpid == 0) {
@@ -3545,7 +3542,7 @@ getpeerbyip(struct bgpd_config *c, struc
newpeer->reconf_action = RECONF_KEEP;
newpeer->rpending = 0;
newpeer->wbuf = NULL;
- init_peer(newpeer);
+ init_peer(newpeer, c);
/* start delete timer, it is stopped when session goes up. */
timer_set(&newpeer->timers, Timer_SessionDown,
INTERVAL_SESSION_DOWN);
@@ -3829,6 +3826,12 @@ merge_peers(struct bgpd_config *c, struc
free(np);
p->reconf_action = RECONF_KEEP;
+
+ /* reapply holdtime and min_holdtime settings */
+ if (p->conf.holdtime == 0)
+ p->conf.holdtime = conf->holdtime;
+ if (p->conf.min_holdtime == 0)
+ p->conf.min_holdtime = conf->min_holdtime;
/* had demotion, is demoted, demote removed? */
if (p->demoted && !p->conf.demote_group[0])
bgpd: fix min_holdtime handling and simplify holdtime