Download raw body.
bgpd: cleanup listerner handling in session.c
On Thu, Nov 13, 2025 at 01:13:13PM +0100, Claudio Jeker wrote:
> On Thu, Nov 13, 2025 at 11:48:12AM +0100, Theo Buehler wrote:
> > On Thu, Nov 13, 2025 at 11:09:41AM +0100, Claudio Jeker wrote:
> > > I dislike how the listener handling in session.c RECONF_DONE is spread
> > > over multiple functions. Just move everything into one place
> > > (setup_listeners) and call it a day.
> > >
> > > I think the result is much simpler to read.
> >
> > Yes, it is much more pleasant. It is plausible but not entirely obvious
> > to me that none of the nconf->listen_addrs has ->reconf == RECONF_NONE.
> >
> > If you are sure about that,
>
> If you look at the IMSG_RECONF_LISTENER code you see:
>
> if (la == NULL) {
> if (nla.reconf != RECONF_REINIT)
> fatalx("king bula sez: "
> "expected REINIT");
> ...
> la->reconf = RECONF_REINIT;
> TAILQ_INSERT_TAIL(nconf->listen_addrs, la,
> entry);
> }
>
> It is the only place where listeners are added to nconf->listen_addrs.
> So none of them has ->reconf == RECONF_NONE.
Great, thanks. ok tb then.
>
> > ok
> >
> > > --
> > > :wq Claudio
> > >
> > > Index: session.c
> > > ===================================================================
> > > RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
> > > diff -u -p -r1.527 session.c
> > > --- session.c 30 Oct 2025 12:43:18 -0000 1.527
> > > +++ session.c 13 Nov 2025 09:40:03 -0000
> > > @@ -108,10 +108,24 @@ int
> > > setup_listeners(u_int *la_cnt)
> > > {
> > > int ttl = 255;
> > > - struct listen_addr *la;
> > > + struct listen_addr *la, *next;
> > > u_int cnt = 0;
> > >
> > > - TAILQ_FOREACH(la, conf->listen_addrs, entry) {
> > > + /* add new listeners */
> > > + TAILQ_CONCAT(conf->listen_addrs, nconf->listen_addrs, entry);
> > > +
> > > + TAILQ_FOREACH_SAFE(la, conf->listen_addrs, entry, next) {
> > > + /* check for and delete no longer used listeners */
> > > + if (la->reconf == RECONF_NONE) {
> > > + log_info("not listening on %s any more",
> > > + log_sockaddr((struct sockaddr *)
> > > + &la->sa, la->sa_len));
> > > + TAILQ_REMOVE(conf->listen_addrs, la, entry);
> > > + close(la->fd);
> > > + free(la);
> > > + continue;
> > > + }
> > > +
> > > la->reconf = RECONF_NONE;
> > > cnt++;
> > >
> > > @@ -1173,7 +1187,7 @@ session_dispatch_imsg(struct imsgbuf *im
> > > struct mrt *mrt;
> > > struct imsgbuf *i;
> > > struct peer *p;
> > > - struct listen_addr *la, *next, nla;
> > > + struct listen_addr *la, nla;
> > > struct session_dependon sdon;
> > > struct bgpd_config tconf;
> > > uint32_t peerid;
> > > @@ -1342,25 +1356,8 @@ session_dispatch_imsg(struct imsgbuf *im
> > > copy_config(conf, nconf);
> > > merge_peers(conf, nconf);
> > >
> > > - /* delete old listeners */
> > > - TAILQ_FOREACH_SAFE(la, conf->listen_addrs, entry,
> > > - next) {
> > > - if (la->reconf == RECONF_NONE) {
> > > - log_info("not listening on %s any more",
> > > - log_sockaddr((struct sockaddr *)
> > > - &la->sa, la->sa_len));
> > > - TAILQ_REMOVE(conf->listen_addrs, la,
> > > - entry);
> > > - close(la->fd);
> > > - free(la);
> > > - }
> > > - }
> > > -
> > > - /* add new listeners */
> > > - TAILQ_CONCAT(conf->listen_addrs, nconf->listen_addrs,
> > > - entry);
> > > -
> > > setup_listeners(listener_cnt);
> > > +
> > > free_config(nconf);
> > > nconf = NULL;
> > > pending_reconf = 0;
> > >
> >
>
> --
> :wq Claudio
bgpd: cleanup listerner handling in session.c