On Tue, Aug 02, 2022 at 12:58:13PM +0100, Stuart Henderson wrote:
> On 2022/08/02 12:34, Claudio Jeker wrote:
> > On startup we load the routing table in bgpd and at that moment a cleanup
> > of old bgpd routes should happen. I noticed this is not the case because
> > fib_sync is not set and so send_rtmsg() just returns.
> > I think we need to force fib_sync in fetchtable() to make sure the cleanup
> > happens correctly.
>
> How much of a problem is it that this clears any existing bgpd routes
> if "fib update no" is set?
>
> I guess in the common case there won't be any anyway, but is there
> any use-case for running two copies of bgpd, one with fib update, one
> without, on the same machine?
One case I see is if you run two bgpd in on two different rtables (not
rdomains). In that case the table used for nexthop lookups will be used by
both daemons. Not sure if this is an issue or not.
The problem with fib_sync being 0 comes from parse.y::parse_config()
if ((rr = add_rib("Adj-RIB-In")) == NULL)
fatal("add_rib failed");
rr->flags = F_RIB_NOFIB | F_RIB_NOEVALUATE;
if ((rr = add_rib("Loc-RIB")) == NULL)
fatal("add_rib failed");
rib_add_fib(rr, conf->default_tableid);
rr->flags = F_RIB_LOCAL;
First the Adj-RIB-In is added then the Loc-RIB. The Adj-RIB-In has
F_RIB_NOFIB | F_RIB_NOEVALUATE and so fib_sync is off when fetchtable is
called in ktable_new().
Afterwards the ktable is updated by Loc-RIB and fib_sync is enabled but
the cleanup did not happen.
So a quick fix is to switch the order in parse.y but heck that probably
breaks later on if used with other tables.
I guess fetchtable() needs to become smarter so that it can be called more
than once. That would also allow to handle RTM_DESYNC in bgpd.
> > OK?
> > --
> > :wq Claudio
> >
> > Index: kroute.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> > retrieving revision 1.285
> > diff -u -p -r1.285 kroute.c
> > --- kroute.c 28 Jul 2022 14:05:13 -0000 1.285
> > +++ kroute.c 2 Aug 2022 10:13:59 -0000
> > @@ -2726,6 +2726,7 @@ fetchtable(struct ktable *kt)
> > char *buf = NULL, *next, *lim;
> > struct rt_msghdr *rtm;
> > struct kroute_full kf;
> > + int fib_sync;
> >
> > mib[0] = CTL_NET;
> > mib[1] = PF_ROUTE;
> > @@ -2754,6 +2755,10 @@ fetchtable(struct ktable *kt)
> > }
> > }
> >
> > + /* force fib_sync on during fetch */
> > + fib_sync = kt->fib_sync;
> > + kt->fib_sync = 1;
> > +
> > lim = buf + len;
> > for (next = buf; next < lim; next += rtm->rtm_msglen) {
> > rtm = (struct rt_msghdr *)next;
> > @@ -2768,6 +2773,8 @@ fetchtable(struct ktable *kt)
> > else
> > kroute_insert(kt, &kf);
> > }
> > + kt->fib_sync = fib_sync;
> > +
> > free(buf);
> > return (0);
> > }
> >
>
--
:wq Claudio
