Claudio Jeker([email protected]) on 2019.08.14 09:14:34 +0200:
> There is no reason anymore for this double wrapping of structs.
> Remove it to make the code simpler.
nice change and reads ok
/B.
> OK?
> --
> :wq Claudio
>
> Index: rde.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.485
> diff -u -p -r1.485 rde.c
> --- rde.c 13 Aug 2019 12:16:20 -0000 1.485
> +++ rde.c 14 Aug 2019 07:05:47 -0000
> @@ -794,12 +794,12 @@ rde_dispatch_imsg_parent(struct imsgbuf
> } else if (rib->flags == rn.flags &&
> rib->rtableid == rn.rtableid) {
> /* no change to rib apart from filters */
> - rib_desc(rib)->state = RECONF_KEEP;
> + rib->state = RECONF_KEEP;
> } else {
> /* reload rib because somehing changed */
> rib->flags_tmp = rn.flags;
> rib->rtableid_tmp = rn.rtableid;
> - rib_desc(rib)->state = RECONF_RELOAD;
> + rib->state = RECONF_RELOAD;
> }
> break;
> case IMSG_RECONF_FILTER:
> @@ -848,14 +848,14 @@ rde_dispatch_imsg_parent(struct imsgbuf
> }
> r->peer.ribid = rib->id;
> if (r->dir == DIR_IN) {
> - nr = rib_desc(rib)->in_rules_tmp;
> + nr = rib->in_rules_tmp;
> if (nr == NULL) {
> nr = calloc(1,
> sizeof(struct filter_head));
> if (nr == NULL)
> fatal(NULL);
> TAILQ_INIT(nr);
> - rib_desc(rib)->in_rules_tmp = nr;
> + rib->in_rules_tmp = nr;
> }
> TAILQ_INSERT_TAIL(nr, r, entry);
> } else
> @@ -1412,7 +1412,7 @@ rde_update_update(struct rde_peer *peer,
> aspath_origin(in->aspath.aspath));
>
> /* add original path to the Adj-RIB-In */
> - if (prefix_update(&ribs[RIB_ADJ_IN].rib, peer, in, prefix, prefixlen,
> + if (prefix_update(&ribs[RIB_ADJ_IN], peer, in, prefix, prefixlen,
> vstate) == 1)
> peer->prefix_cnt++;
>
> @@ -1440,9 +1440,9 @@ rde_update_update(struct rde_peer *peer,
> rde_update_log("update", i, peer,
> &state.nexthop->exit_nexthop, prefix,
> prefixlen);
> - prefix_update(&ribs[i].rib, peer, &state, prefix,
> + prefix_update(&ribs[i], peer, &state, prefix,
> prefixlen, vstate);
> - } else if (prefix_withdraw(&ribs[i].rib, peer, prefix,
> + } else if (prefix_withdraw(&ribs[i], peer, prefix,
> prefixlen)) {
> rde_update_log(wmsg, i, peer,
> NULL, prefix, prefixlen);
> @@ -1463,13 +1463,13 @@ rde_update_withdraw(struct rde_peer *pee
> for (i = RIB_LOC_START; i < rib_size; i++) {
> if (!rib_valid(i))
> continue;
> - if (prefix_withdraw(&ribs[i].rib, peer, prefix, prefixlen))
> + if (prefix_withdraw(&ribs[i], peer, prefix, prefixlen))
> rde_update_log("withdraw", i, peer, NULL, prefix,
> prefixlen);
> }
>
> /* remove original path form the Adj-RIB-In */
> - if (prefix_withdraw(&ribs[RIB_ADJ_IN].rib, peer, prefix, prefixlen))
> + if (prefix_withdraw(&ribs[RIB_ADJ_IN], peer, prefix, prefixlen))
> peer->prefix_cnt--;
>
> peer->prefix_rcvd_withdraw++;
> @@ -3126,10 +3126,10 @@ rde_reload_done(void)
>
> switch (ribs[rid].state) {
> case RECONF_DELETE:
> - rib_free(&ribs[rid].rib);
> + rib_free(&ribs[rid]);
> break;
> case RECONF_RELOAD:
> - rib_update(&ribs[rid].rib);
> + rib_update(&ribs[rid]);
> ribs[rid].state = RECONF_KEEP;
> /* FALLTHROUGH */
> case RECONF_KEEP:
> @@ -3251,7 +3251,7 @@ rde_softreconfig_in_done(void *arg, u_in
> static void
> rde_softreconfig_out_done(void *arg, u_int8_t aid)
> {
> - struct rib_desc *rib = arg;
> + struct rib *rib = arg;
>
> /* this RIB dump is done */
> log_info("softreconfig out done for %s", rib->name);
> @@ -3281,7 +3281,7 @@ static void
> rde_softreconfig_in(struct rib_entry *re, void *bula)
> {
> struct filterstate state;
> - struct rib_desc *rib;
> + struct rib *rib;
> struct prefix *p;
> struct pt_entry *pt;
> struct rde_peer *peer;
> @@ -3328,11 +3328,11 @@ rde_softreconfig_in(struct rib_entry *re
>
> if (action == ACTION_ALLOW) {
> /* update Local-RIB */
> - prefix_update(&rib->rib, peer, &state, &prefix,
> + prefix_update(rib, peer, &state, &prefix,
> pt->prefixlen, p->validation_state);
> } else if (action == ACTION_DENY) {
> /* remove from Local-RIB */
> - prefix_withdraw(&rib->rib, peer, &prefix,
> + prefix_withdraw(rib, peer, &prefix,
> pt->prefixlen);
> }
>
> @@ -3362,9 +3362,9 @@ rde_softreconfig_sync_reeval(struct rib_
> {
> struct prefix_list prefixes;
> struct prefix *p, *next;
> - struct rib_desc *rd = arg;
> + struct rib *rib = arg;
>
> - if (rd->rib.flags & F_RIB_NOEVALUATE) {
> + if (rib->flags & F_RIB_NOEVALUATE) {
> /*
> * evaluation process is turned off
> * so remove all prefixes from adj-rib-out
> @@ -3375,7 +3375,7 @@ rde_softreconfig_sync_reeval(struct rib_
> nexthop_unlink(p);
> }
> if (re->active) {
> - rde_generate_updates(re_rib(re), NULL, re->active);
> + rde_generate_updates(rib, NULL, re->active);
> re->active = NULL;
> }
> return;
> @@ -3405,14 +3405,14 @@ rde_softreconfig_sync_fib(struct rib_ent
> static void
> rde_softreconfig_sync_done(void *arg, u_int8_t aid)
> {
> - struct rib_desc *rd = arg;
> + struct rib *rib = arg;
>
> /* this RIB dump is done */
> - if (rd->fibstate == RECONF_RELOAD)
> - log_info("fib sync done for %s", rd->name);
> + if (rib->fibstate == RECONF_RELOAD)
> + log_info("fib sync done for %s", rib->name);
> else
> - log_info("re-evaluation done for %s", rd->name);
> - rd->fibstate = RECONF_NONE;
> + log_info("re-evaluation done for %s", rib->name);
> + rib->fibstate = RECONF_NONE;
>
> /* check if other dumps are still running */
> if (--softreconfig == 0)
> @@ -3726,7 +3726,7 @@ peer_flush_upcall(struct rib_entry *re,
> for (i = RIB_LOC_START; i < rib_size; i++) {
> if (!rib_valid(i))
> continue;
> - rp = prefix_get(&ribs[i].rib, peer, &addr, prefixlen);
> + rp = prefix_get(&ribs[i], peer, &addr, prefixlen);
> if (rp) {
> asp = prefix_aspath(rp);
> if (asp->pftableid)
> @@ -3963,7 +3963,7 @@ network_add(struct network_config *nc, s
>
> vstate = rde_roa_validity(&conf->rde_roa, &nc->prefix,
> nc->prefixlen, aspath_origin(state->aspath.aspath));
> - if (prefix_update(&ribs[RIB_ADJ_IN].rib, peerself, state, &nc->prefix,
> + if (prefix_update(&ribs[RIB_ADJ_IN], peerself, state, &nc->prefix,
> nc->prefixlen, vstate) == 1)
> peerself->prefix_cnt++;
> for (i = RIB_LOC_START; i < rib_size; i++) {
> @@ -3972,7 +3972,7 @@ network_add(struct network_config *nc, s
> rde_update_log("announce", i, peerself,
> state->nexthop ? &state->nexthop->exit_nexthop : NULL,
> &nc->prefix, nc->prefixlen);
> - prefix_update(&ribs[i].rib, peerself, state, &nc->prefix,
> + prefix_update(&ribs[i], peerself, state, &nc->prefix,
> nc->prefixlen, vstate);
> }
> filterset_free(&nc->attrset);
> @@ -4033,12 +4033,12 @@ network_delete(struct network_config *nc
> for (i = RIB_LOC_START; i < rib_size; i++) {
> if (!rib_valid(i))
> continue;
> - if (prefix_withdraw(&ribs[i].rib, peerself, &nc->prefix,
> + if (prefix_withdraw(&ribs[i], peerself, &nc->prefix,
> nc->prefixlen))
> rde_update_log("withdraw announce", i, peerself,
> NULL, &nc->prefix, nc->prefixlen);
> }
> - if (prefix_withdraw(&ribs[RIB_ADJ_IN].rib, peerself, &nc->prefix,
> + if (prefix_withdraw(&ribs[RIB_ADJ_IN], peerself, &nc->prefix,
> nc->prefixlen))
> peerself->prefix_cnt--;
> }
> @@ -4099,7 +4099,7 @@ network_flush_upcall(struct rib_entry *r
> for (i = RIB_LOC_START; i < rib_size; i++) {
> if (!rib_valid(i))
> continue;
> - rp = prefix_get(&ribs[i].rib, peer, &addr, prefixlen);
> + rp = prefix_get(&ribs[i], peer, &addr, prefixlen);
> if (rp) {
> prefix_destroy(rp);
> rde_update_log("flush announce", i, peer,
> Index: rde.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
> retrieving revision 1.224
> diff -u -p -r1.224 rde.h
> --- rde.h 13 Aug 2019 12:16:20 -0000 1.224
> +++ rde.h 14 Aug 2019 07:00:12 -0000
> @@ -51,25 +51,21 @@ struct rib_entry {
>
> struct rib {
> struct rib_tree tree;
> + char name[PEER_DESCR_LEN];
> + struct filter_head *in_rules;
> + struct filter_head *in_rules_tmp;
> u_int rtableid;
> u_int rtableid_tmp;
> u_int16_t flags;
> u_int16_t flags_tmp;
> u_int16_t id;
> + enum reconf_action state, fibstate;
> };
>
> #define RIB_ADJ_IN 0
> #define RIB_LOC_START 1
> #define RIB_NOTFOUND 0xffff
>
> -struct rib_desc {
> - char name[PEER_DESCR_LEN];
> - struct rib rib;
> - struct filter_head *in_rules;
> - struct filter_head *in_rules_tmp;
> - enum reconf_action state, fibstate;
> -};
> -
> /*
> * How do we identify peers between the session handler and the rde?
> * Currently I assume that we can do that with the neighbor_ip...
> @@ -507,13 +503,12 @@ pt_unref(struct pt_entry *pt)
>
> /* rde_rib.c */
> extern u_int16_t rib_size;
> -extern struct rib_desc *ribs;
> +extern struct rib *ribs;
>
> struct rib *rib_new(char *, u_int, u_int16_t);
> void rib_update(struct rib *);
> struct rib *rib_byid(u_int16_t);
> u_int16_t rib_find(char *);
> -struct rib_desc *rib_desc(struct rib *);
> void rib_free(struct rib *);
> void rib_shutdown(void);
> struct rib_entry *rib_get(struct rib *, struct bgpd_addr *, int);
> Index: rde_rib.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
> retrieving revision 1.204
> diff -u -p -r1.204 rde_rib.c
> --- rde_rib.c 9 Aug 2019 14:12:21 -0000 1.204
> +++ rde_rib.c 14 Aug 2019 07:10:15 -0000
> @@ -37,7 +37,7 @@
> * This is achieved by heavily linking the different parts together.
> */
> u_int16_t rib_size;
> -struct rib_desc *ribs;
> +struct rib *ribs;
>
> struct rib_entry *rib_add(struct rib *, struct bgpd_addr *, int);
> static inline int rib_compare(const struct rib_entry *,
> @@ -136,8 +136,8 @@ rib_compare(const struct rib_entry *a, c
> struct rib *
> rib_new(char *name, u_int rtableid, u_int16_t flags)
> {
> - struct rib_desc *xribs;
> - u_int16_t id;
> + struct rib *xribs;
> + u_int16_t id;
>
> for (id = 0; id < rib_size; id++) {
> if (!rib_valid(id))
> @@ -146,7 +146,7 @@ rib_new(char *name, u_int rtableid, u_in
>
> if (id >= rib_size) {
> if ((xribs = reallocarray(ribs, id + 1,
> - sizeof(struct rib_desc))) == NULL) {
> + sizeof(struct rib))) == NULL) {
> /* XXX this is not clever */
> fatal(NULL);
> }
> @@ -154,13 +154,13 @@ rib_new(char *name, u_int rtableid, u_in
> rib_size = id + 1;
> }
>
> - memset(&ribs[id], 0, sizeof(struct rib_desc));
> + memset(&ribs[id], 0, sizeof(struct rib));
> strlcpy(ribs[id].name, name, sizeof(ribs[id].name));
> - RB_INIT(rib_tree(&ribs[id].rib));
> + RB_INIT(rib_tree(&ribs[id]));
> ribs[id].state = RECONF_REINIT;
> - ribs[id].rib.id = id;
> - ribs[id].rib.flags = flags;
> - ribs[id].rib.rtableid = rtableid;
> + ribs[id].id = id;
> + ribs[id].flags = flags;
> + ribs[id].rtableid = rtableid;
>
> ribs[id].in_rules = calloc(1, sizeof(struct filter_head));
> if (ribs[id].in_rules == NULL)
> @@ -168,7 +168,7 @@ rib_new(char *name, u_int rtableid, u_in
> TAILQ_INIT(ribs[id].in_rules);
>
> log_debug("%s: %s -> %u", __func__, name, id);
> - return (&ribs[id].rib);
> + return (&ribs[id]);
> }
>
> /*
> @@ -181,8 +181,6 @@ rib_new(char *name, u_int rtableid, u_in
> void
> rib_update(struct rib *rib)
> {
> - struct rib_desc *rd = rib_desc(rib);
> -
> /* flush fib first if there was one */
> if ((rib->flags & (F_RIB_NOFIB | F_RIB_NOEVALUATE)) == 0)
> rde_send_kroute_flush(rib);
> @@ -190,22 +188,22 @@ rib_update(struct rib *rib)
> /* if no evaluate changes then a full reinit is needed */
> if ((rib->flags & F_RIB_NOEVALUATE) !=
> (rib->flags_tmp & F_RIB_NOEVALUATE))
> - rd->fibstate = RECONF_REINIT;
> + rib->fibstate = RECONF_REINIT;
>
> rib->flags = rib->flags_tmp;
> rib->rtableid = rib->rtableid_tmp;
>
> /* reload fib if there is no reinit pending and there will be a fib */
> - if (rd->fibstate != RECONF_REINIT &&
> + if (rib->fibstate != RECONF_REINIT &&
> (rib->flags & (F_RIB_NOFIB | F_RIB_NOEVALUATE)) == 0)
> - rd->fibstate = RECONF_RELOAD;
> + rib->fibstate = RECONF_RELOAD;
> }
>
> struct rib *
> rib_byid(u_int16_t rid)
> {
> if (rib_valid(rid))
> - return &ribs[rid].rib;
> + return &ribs[rid];
> return NULL;
> }
>
> @@ -226,16 +224,9 @@ rib_find(char *name)
> return RIB_NOTFOUND;
> }
>
> -struct rib_desc *
> -rib_desc(struct rib *rib)
> -{
> - return (&ribs[rib->id]);
> -}
> -
> void
> rib_free(struct rib *rib)
> {
> - struct rib_desc *rd = rib_desc(rib);
> struct rib_entry *re, *xre;
> struct prefix *p;
>
> @@ -273,9 +264,9 @@ rib_free(struct rib *rib)
> }
> if (rib->id <= RIB_LOC_START)
> return; /* never remove the default ribs */
> - filterlist_free(rd->in_rules_tmp);
> - filterlist_free(rd->in_rules);
> - memset(rd, 0, sizeof(struct rib_desc));
> + filterlist_free(rib->in_rules_tmp);
> + filterlist_free(rib->in_rules);
> + memset(rib, 0, sizeof(struct rib));
> }
>
> void
> @@ -286,17 +277,16 @@ rib_shutdown(void)
> for (id = 0; id < rib_size; id++) {
> if (!rib_valid(id))
> continue;
> - if (!RB_EMPTY(rib_tree(&ribs[id].rib))) {
> + if (!RB_EMPTY(rib_tree(&ribs[id]))) {
> log_warnx("%s: rib %s is not empty", __func__,
> ribs[id].name);
> }
> - rib_free(&ribs[id].rib);
> + rib_free(&ribs[id]);
> }
> for (id = 0; id <= RIB_LOC_START; id++) {
> - struct rib_desc *rd = &ribs[id];
> - filterlist_free(rd->in_rules_tmp);
> - filterlist_free(rd->in_rules);
> - memset(rd, 0, sizeof(struct rib_desc));
> + filterlist_free(ribs[id].in_rules_tmp);
> + filterlist_free(ribs[id].in_rules);
> + memset(&ribs[id], 0, sizeof(struct rib));
> }
> free(ribs);
> }
>