On Thu, Oct 25, 2018 at 08:51:30AM +0200, Claudio Jeker wrote:
> Next step on my quest to make the RIB code better.
> This changes the following things:
> - network_flush is now using rib_dump_new to walk the Adj-RIB-In and
> remove all dynamically added announcements
> - peer_flush got generalized and is now used also in peer_down.
> It also uses a rib_dump_new call to walk the Adj-RIB-In and remove
> all prefixes from a peer but this is done synchronous for now.
> - peer_flush is now working correctly for AID_UNSPEC.
> - Change the rib_valid check to continue instead of break out of the loop.
> The rib table can have holes so the loop needs to continue.
>
> This removes the last three places that use the peer -> path -> prefix
> tailqs so the next step is to remove these and make rde_aspath just an
> object that is referenced by prefixes. After that adding a proper
> Adj-RIB-Out should be possible.
>
Question below.
Running on prod too :)
> Index: rde_rib.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
> retrieving revision 1.180
> diff -u -p -r1.180 rde_rib.c
> --- rde_rib.c 24 Oct 2018 08:26:37 -0000 1.180
> +++ rde_rib.c 24 Oct 2018 08:44:54 -0000
> @@ -339,8 +339,8 @@ rib_restart(struct rib_context *ctx)
> static void
> rib_dump_r(struct rib_context *ctx)
> {
> + struct rib_entry *re, *next;
> struct rib *rib;
> - struct rib_entry *re;
> unsigned int i;
>
> rib = rib_byid(ctx->ctx_rib_id);
> @@ -352,7 +352,8 @@ rib_dump_r(struct rib_context *ctx)
> else
> re = rib_restart(ctx);
>
> - for (i = 0; re != NULL; re = RB_NEXT(rib_tree, unused, re)) {
> + for (i = 0; re != NULL; re = next) {
> + next = RB_NEXT(rib_tree, unused, re);
Why not RB_FOREACH_SAFE ?
> if (re->rib_id != ctx->ctx_rib_id)
> fatalx("%s: Unexpected RIB %u != %u.", __func__,
> re->rib_id, ctx->ctx_rib_id);
> @@ -435,6 +436,10 @@ rib_dump_new(u_int16_t id, u_int8_t aid,
>
> LIST_INSERT_HEAD(&rib_dumps, ctx, entry);
>
> + /* requested a sync traversal */
> + if (count == 0)
> + rib_dump_r(ctx);
> +
> return 0;
> }
>
> @@ -664,65 +669,6 @@ path_lookup(struct rde_aspath *aspath, s
> return (NULL);
> }
>
> -void
> -path_remove(struct rde_aspath *asp)
> -{
> - struct prefix *p, *np;
> -
> - for (p = TAILQ_FIRST(&asp->prefixes); p != NULL; p = np) {
> - np = TAILQ_NEXT(p, path_l);
> - if (asp->pftableid) {
> - struct bgpd_addr addr;
> -
> - pt_getaddr(p->re->prefix, &addr);
> - /* Commit is done in peer_down() */
> - rde_send_pftable(prefix_aspath(p)->pftableid, &addr,
> - p->re->prefix->prefixlen, 1);
> - }
> - prefix_destroy(p);
> - }
> -}
> -
> -/* remove all stale routes or if staletime is 0 remove all routes for
> - a specified AID. */
> -u_int32_t
> -path_remove_stale(struct rde_aspath *asp, u_int8_t aid, time_t staletime)
> -{
> - struct prefix *p, *np;
> - u_int32_t rprefixes;
> -
> - rprefixes=0;
> - /*
> - * This is called when a session flapped and during that time
> - * the pending updates for that peer are getting reset.
> - */
> - for (p = TAILQ_FIRST(&asp->prefixes); p != NULL; p = np) {
> - np = TAILQ_NEXT(p, path_l);
> - if (p->re->prefix->aid != aid)
> - continue;
> -
> - if (staletime && p->lastchange > staletime)
> - continue;
> -
> - if (asp->pftableid) {
> - struct bgpd_addr addr;
> -
> - pt_getaddr(p->re->prefix, &addr);
> - /* Commit is done in peer_flush() */
> - rde_send_pftable(prefix_aspath(p)->pftableid, &addr,
> - p->re->prefix->prefixlen, 1);
> - }
> -
> - /* only count Adj-RIB-In */
> - if (re_rib(p->re) == &ribs[RIB_ADJ_IN].rib)
> - rprefixes++;
> -
> - prefix_destroy(p);
> - }
> - return (rprefixes);
> -}
> -
> -
> /*
> * This function can only called when all prefix have been removed first.
> * Normally this happens directly out of the prefix removal functions.
> @@ -1144,29 +1090,6 @@ prefix_destroy(struct prefix *p)
>
> if (path_empty(asp))
> path_destroy(asp);
> -}
> -
> -/*
> - * helper function to clean up the dynamically added networks
> - */
> -void
> -prefix_network_clean(struct rde_peer *peer)
> -{
> - struct rde_aspath *asp, *xasp;
> - struct prefix *p, *xp;
> -
> - for (asp = TAILQ_FIRST(&peer->path_h); asp != NULL; asp = xasp) {
> - xasp = TAILQ_NEXT(asp, peer_l);
> - if ((asp->flags & F_ANN_DYNAMIC) != F_ANN_DYNAMIC)
> - continue;
> - for (p = TAILQ_FIRST(&asp->prefixes); p != NULL; p = xp) {
> - xp = TAILQ_NEXT(p, path_l);
> - prefix_unlink(p);
> - prefix_free(p);
> - }
> - if (path_empty(asp))
> - path_destroy(asp);
> - }
> }
>
> /*
>
