On Fri, Sep 09, 2022 at 03:20:00PM +0200, Theo Buehler wrote:
> On Fri, Sep 09, 2022 at 02:49:12PM +0200, Claudio Jeker wrote:
> > So bgpctl has the or-shorter flag to indicate that not only the best
> > matching prefix should be shown, instead all matching prefixes are shown.
> > Currently this is done by a full table walk which is super expensive.
> > There is no real reason to do that. One can just start the lookup with
> > prefixlen = 0 and retry until the prefixlen of the request is reached.
>
> ok tb.
>
> One small comment inline.
>
> >
> > After this I want to also run or-longer without a full table walk (it
> > should only walk a subtree). Then I think rde_dump_ctx_new() should be
> > refactored.
> > --
> > :wq Claudio
> >
> > Index: rde.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> > retrieving revision 1.574
> > diff -u -p -r1.574 rde.c
> > --- rde.c 1 Sep 2022 13:23:24 -0000 1.574
> > +++ rde.c 9 Sep 2022 12:40:18 -0000
> > @@ -2836,12 +2838,27 @@ rde_dump_ctx_new(struct ctl_show_rib_req
> > }
> >
> > do {
> > - if (req->prefixlen == hostplen)
> > + if (req->flags & F_SHORTER) {
> > + for (plen = 0; plen <= req->prefixlen;
> > + plen++) {
> > + p = prefix_adjout_lookup(peer,
> > + &req->prefix, plen);
> > + /* dump all matching paths */
> > + while (p != NULL) {
> > + rde_dump_adjout_upcall(
> > + p, ctx);
> > + p = prefix_adjout_next(
> > + peer, p);
> > + }
> > + }
> > + p = NULL;
>
> It probably helps readability to keep this, but I think p is always NULL
> here since the for loop runs at least once.
I think you are right and p has to be NULL. We enter the loop at least
once and then p is NULL at the end of the loop. I still prefer the
explicit p = NULL there (maybe the compiler will notice and optimize it
out).
--
:wq Claudio
