Claudio Jeker(cje...@diehard.n-r-g.com) 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);
>  }
> 

Reply via email to