On Sat, Oct 13, 2012 at 08:40:35PM +0200, Claudio Jeker wrote:
> Hi,
>
> this is the first step in making bgpd faster and non locking on reloads.
> The filters are now split into input and output chains. The input chains
> are also split per RIB. This should result in faster filtering and allows
> to only run softreconfig on those tables that need it. People with
> multiple RIBs will benefit the most at the moment.
>
> I did some basic tests and I'm fairly confident that it should work but
> this is a huge change in the reload logic and needs a good broad testing
> and I don't mind some review of the code.
>
No comments on this? Should I just commit it?
> --
> :wq Claudio
>
> Index: bgpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.273
> diff -u -p -r1.273 bgpd.h
> --- bgpd.h 18 Sep 2012 10:10:00 -0000 1.273
> +++ bgpd.h 20 Sep 2012 08:06:06 -0000
> @@ -103,6 +103,7 @@ enum reconf_action {
> RECONF_NONE,
> RECONF_KEEP,
> RECONF_REINIT,
> + RECONF_RELOAD,
> RECONF_DELETE
> };
>
> Index: rde.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.321
> diff -u -p -r1.321 rde.c
> --- rde.c 18 Sep 2012 10:10:00 -0000 1.321
> +++ rde.c 12 Oct 2012 06:27:52 -0000
> @@ -85,12 +85,11 @@ void rde_dump_mrt_new(struct mrt *, pi
> void rde_dump_done(void *);
>
> int rde_rdomain_import(struct rde_aspath *, struct rdomain *);
> -void rde_up_dump_upcall(struct rib_entry *, void *);
> +void rde_reload_done(void);
> void rde_softreconfig_out(struct rib_entry *, void *);
> void rde_softreconfig_in(struct rib_entry *, void *);
> -void rde_softreconfig_load(struct rib_entry *, void *);
> -void rde_softreconfig_load_peer(struct rib_entry *, void *);
> void rde_softreconfig_unload_peer(struct rib_entry *, void *);
> +void rde_up_dump_upcall(struct rib_entry *, void *);
> void rde_update_queue_runner(void);
> void rde_update6_queue_runner(u_int8_t);
>
> @@ -119,7 +118,7 @@ struct bgpd_config *conf, *nconf;
> time_t reloadtime;
> struct rde_peer_head peerlist;
> struct rde_peer *peerself;
> -struct filter_head *rules_l, *newrules;
> +struct filter_head *out_rules, *out_rules_tmp;
> struct rdomain_head *rdomains_l, *newdomains;
> struct imsgbuf *ibuf_se;
> struct imsgbuf *ibuf_se_ctl;
> @@ -224,10 +223,10 @@ rde_main(int pipe_m2r[2], int pipe_s2r[2
> nexthop_init(nexthophashsize);
> peer_init(peerhashsize);
>
> - rules_l = calloc(1, sizeof(struct filter_head));
> - if (rules_l == NULL)
> + out_rules = calloc(1, sizeof(struct filter_head));
> + if (out_rules == NULL)
> fatal(NULL);
> - TAILQ_INIT(rules_l);
> + TAILQ_INIT(out_rules);
> rdomains_l = calloc(1, sizeof(struct rdomain_head));
> if (rdomains_l == NULL)
> fatal(NULL);
> @@ -645,11 +644,11 @@ rde_dispatch_imsg_parent(struct imsgbuf
> struct rde_rib rn;
> struct rde_peer *peer;
> struct peer_config *pconf;
> + struct filter_head *nr;
> struct filter_rule *r;
> struct filter_set *s;
> struct nexthop *nh;
> - int n, fd, reconf_in = 0, reconf_out = 0,
> - reconf_rib = 0;
> + int n, fd;
> u_int16_t rid;
>
> if ((n = imsg_read(ibuf)) == -1)
> @@ -693,10 +692,10 @@ rde_dispatch_imsg_parent(struct imsgbuf
> sizeof(struct bgpd_config))
> fatalx("IMSG_RECONF_CONF bad len");
> reloadtime = time(NULL);
> - newrules = calloc(1, sizeof(struct filter_head));
> - if (newrules == NULL)
> + out_rules_tmp = calloc(1, sizeof(struct filter_head));
> + if (out_rules_tmp == NULL)
> fatal(NULL);
> - TAILQ_INIT(newrules);
> + TAILQ_INIT(out_rules_tmp);
> newdomains = calloc(1, sizeof(struct rdomain_head));
> if (newdomains == NULL)
> fatal(NULL);
> @@ -746,7 +745,19 @@ rde_dispatch_imsg_parent(struct imsgbuf
> TAILQ_INIT(&r->set);
> r->peer.ribid = rib_find(r->rib);
> parent_set = &r->set;
> - TAILQ_INSERT_TAIL(newrules, r, entry);
> + if (r->dir == DIR_IN) {
> + nr = ribs[r->peer.ribid].in_rules_tmp;
> + if (nr == NULL) {
> + nr = calloc(1,
> + sizeof(struct filter_head));
> + if (nr == NULL)
> + fatal(NULL);
> + TAILQ_INIT(nr);
> + ribs[r->peer.ribid].in_rules_tmp = nr;
> + }
> + TAILQ_INSERT_TAIL(nr, r, entry);
> + } else
> + TAILQ_INSERT_TAIL(out_rules_tmp, r, entry);
> break;
> case IMSG_RECONF_RDOMAIN:
> if (imsg.hdr.len - IMSG_HEADER_SIZE !=
> @@ -781,110 +792,9 @@ rde_dispatch_imsg_parent(struct imsgbuf
> case IMSG_RECONF_DONE:
> if (nconf == NULL)
> fatalx("got IMSG_RECONF_DONE but no config");
> - if ((nconf->flags & BGPD_FLAG_NO_EVALUATE)
> - != (conf->flags & BGPD_FLAG_NO_EVALUATE)) {
> - log_warnx("change to/from route-collector "
> - "mode ignored");
> - if (conf->flags & BGPD_FLAG_NO_EVALUATE)
> - nconf->flags |= BGPD_FLAG_NO_EVALUATE;
> - else
> - nconf->flags &= ~BGPD_FLAG_NO_EVALUATE;
> - }
> - memcpy(conf, nconf, sizeof(struct bgpd_config));
> - conf->listen_addrs = NULL;
> - conf->csock = NULL;
> - conf->rcsock = NULL;
> - free(nconf);
> - nconf = NULL;
> parent_set = NULL;
> - /* sync peerself with conf */
> - peerself->remote_bgpid = ntohl(conf->bgpid);
> - peerself->conf.local_as = conf->as;
> - peerself->conf.remote_as = conf->as;
> - peerself->short_as = conf->short_as;
> -
> - /* apply new set of rdomain, sync will be done later */
> - while ((rd = SIMPLEQ_FIRST(rdomains_l)) != NULL) {
> - SIMPLEQ_REMOVE_HEAD(rdomains_l, entry);
> - filterset_free(&rd->import);
> - filterset_free(&rd->export);
> - free(rd);
> - }
> - free(rdomains_l);
> - rdomains_l = newdomains;
> -
> - /* check if filter changed */
> - LIST_FOREACH(peer, &peerlist, peer_l) {
> - if (peer->conf.id == 0)
> - continue;
> - peer->reconf_out = 0;
> - peer->reconf_in = 0;
> - peer->reconf_rib = 0;
> - if (peer->conf.softreconfig_in &&
> - !rde_filter_equal(rules_l, newrules, peer,
> - DIR_IN)) {
> - peer->reconf_in = 1;
> - reconf_in = 1;
> - }
> - if (peer->ribid != rib_find(peer->conf.rib)) {
> - rib_dump(&ribs[peer->ribid],
> - rde_softreconfig_unload_peer, peer,
> - AID_UNSPEC);
> - peer->ribid = rib_find(peer->conf.rib);
> - peer->reconf_rib = 1;
> - reconf_rib = 1;
> - continue;
> - }
> - if (peer->conf.softreconfig_out &&
> - !rde_filter_equal(rules_l, newrules, peer,
> - DIR_OUT)) {
> - peer->reconf_out = 1;
> - reconf_out = 1;
> - }
> - }
> - /* bring ribs in sync before softreconfig dance */
> - for (rid = 0; rid < rib_size; rid++) {
> - if (ribs[rid].state == RECONF_DELETE)
> - rib_free(&ribs[rid]);
> - else if (ribs[rid].state == RECONF_REINIT)
> - rib_dump(&ribs[0],
> - rde_softreconfig_load, &ribs[rid],
> - AID_UNSPEC);
> - }
> - /* sync local-RIBs first */
> - if (reconf_in)
> - rib_dump(&ribs[0], rde_softreconfig_in, NULL,
> - AID_UNSPEC);
> - /* then sync peers */
> - if (reconf_out) {
> - int i;
> - for (i = 1; i < rib_size; i++) {
> - if (ribs[i].state == RECONF_REINIT)
> - /* already synced by _load */
> - continue;
> - rib_dump(&ribs[i], rde_softreconfig_out,
> - NULL, AID_UNSPEC);
> - }
> - }
> - if (reconf_rib) {
> - LIST_FOREACH(peer, &peerlist, peer_l) {
> - rib_dump(&ribs[peer->ribid],
> - rde_softreconfig_load_peer,
> - peer, AID_UNSPEC);
> - }
> - }
> -
> - while ((r = TAILQ_FIRST(rules_l)) != NULL) {
> - TAILQ_REMOVE(rules_l, r, entry);
> - filterset_free(&r->set);
> - free(r);
> - }
> - free(rules_l);
> - rules_l = newrules;
>
> - log_info("RDE reconfigured");
> - imsg_compose(ibuf_main, IMSG_RECONF_DONE, 0, 0,
> - -1, NULL, 0);
> + rde_reload_done();
> break;
> case IMSG_NEXTHOP_UPDATE:
> nexthop_update(imsg.data);
> @@ -1377,8 +1287,8 @@ rde_update_update(struct rde_peer *peer,
>
> for (i = 1; i < rib_size; i++) {
> /* input filter */
> - action = rde_filter(i, &fasp, rules_l, peer, asp, prefix,
> - prefixlen, peer, DIR_IN);
> + action = rde_filter(ribs[i].in_rules, &fasp, peer, asp, prefix,
> + prefixlen, peer);
>
> if (fasp == NULL)
> fasp = asp;
> @@ -2310,8 +2220,8 @@ rde_dump_filterout(struct rde_peer *peer
> return;
>
> pt_getaddr(p->prefix, &addr);
> - a = rde_filter(1 /* XXX */, &asp, rules_l, peer, p->aspath, &addr,
> - p->prefix->prefixlen, p->aspath->peer, DIR_OUT);
> + a = rde_filter(out_rules, &asp, peer, p->aspath, &addr,
> + p->prefix->prefixlen, p->aspath->peer);
> if (asp)
> asp->peer = p->aspath->peer;
> else
> @@ -2647,196 +2557,228 @@ rde_send_nexthop(struct bgpd_addr *next,
> * soft reconfig specific functions
> */
> void
> -rde_softreconfig_out(struct rib_entry *re, void *ptr)
> +rde_reload_done(void)
> {
> - struct prefix *p = re->active;
> - struct pt_entry *pt;
> + struct rdomain *rd;
> struct rde_peer *peer;
> - struct rde_aspath *oasp, *nasp;
> - enum filter_actions oa, na;
> - struct bgpd_addr addr;
> + struct filter_head *fh;
> + u_int16_t rid;
>
> - if (p == NULL)
> - return;
> + /* first merge the main config */
> + if ((nconf->flags & BGPD_FLAG_NO_EVALUATE)
> + != (conf->flags & BGPD_FLAG_NO_EVALUATE)) {
> + log_warnx("change to/from route-collector "
> + "mode ignored");
> + if (conf->flags & BGPD_FLAG_NO_EVALUATE)
> + nconf->flags |= BGPD_FLAG_NO_EVALUATE;
> + else
> + nconf->flags &= ~BGPD_FLAG_NO_EVALUATE;
> + }
> + memcpy(conf, nconf, sizeof(struct bgpd_config));
> + conf->listen_addrs = NULL;
> + conf->csock = NULL;
> + conf->rcsock = NULL;
> + free(nconf);
> + nconf = NULL;
> +
> + /* sync peerself with conf */
> + peerself->remote_bgpid = ntohl(conf->bgpid);
> + peerself->conf.local_as = conf->as;
> + peerself->conf.remote_as = conf->as;
> + peerself->short_as = conf->short_as;
> +
> + /* apply new set of rdomain, sync will be done later */
> + while ((rd = SIMPLEQ_FIRST(rdomains_l)) != NULL) {
> + SIMPLEQ_REMOVE_HEAD(rdomains_l, entry);
> + filterset_free(&rd->import);
> + filterset_free(&rd->export);
> + free(rd);
> + }
> + free(rdomains_l);
> + rdomains_l = newdomains;
> + /* XXX WHERE IS THE SYNC ??? */
>
> - pt = re->prefix;
> - pt_getaddr(pt, &addr);
> + /*
> + * make the new filter rules the active one but keep the old for
> + * softrconfig. This is needed so that changes happening are using
> + * the right filters.
> + */
> + fh = out_rules;
> + out_rules = out_rules_tmp;
> + out_rules_tmp = fh;
> +
> + /* check if filter changed */
> LIST_FOREACH(peer, &peerlist, peer_l) {
> if (peer->conf.id == 0)
> continue;
> - if (peer->ribid != re->ribid)
> - continue;
> - if (peer->reconf_out == 0)
> + peer->reconf_out = 0;
> + peer->reconf_rib = 0;
> + if (peer->ribid != rib_find(peer->conf.rib)) {
> + rib_dump(&ribs[peer->ribid],
> + rde_softreconfig_unload_peer, peer, AID_UNSPEC);
> + peer->ribid = rib_find(peer->conf.rib);
> + peer->reconf_rib = 1;
> continue;
> - if (up_test_update(peer, p) != 1)
> - continue;
> -
> - oa = rde_filter(re->ribid, &oasp, rules_l, peer, p->aspath,
> - &addr, pt->prefixlen, p->aspath->peer, DIR_OUT);
> - na = rde_filter(re->ribid, &nasp, newrules, peer, p->aspath,
> - &addr, pt->prefixlen, p->aspath->peer, DIR_OUT);
> - oasp = oasp != NULL ? oasp : p->aspath;
> - nasp = nasp != NULL ? nasp : p->aspath;
> -
> - if (oa == ACTION_DENY && na == ACTION_DENY)
> - /* nothing todo */
> - goto done;
> - if (oa == ACTION_DENY && na == ACTION_ALLOW) {
> - /* send update */
> - up_generate(peer, nasp, &addr, pt->prefixlen);
> - goto done;
> }
> - if (oa == ACTION_ALLOW && na == ACTION_DENY) {
> - /* send withdraw */
> - up_generate(peer, NULL, &addr, pt->prefixlen);
> - goto done;
> - }
> - if (oa == ACTION_ALLOW && na == ACTION_ALLOW) {
> - if (path_compare(nasp, oasp) == 0)
> - goto done;
> - /* send update */
> - up_generate(peer, nasp, &addr, pt->prefixlen);
> + if (peer->conf.softreconfig_out &&
> + !rde_filter_equal(out_rules, out_rules_tmp, peer)) {
> + peer->reconf_out = 1;
> + }
> + }
> + /* bring ribs in sync */
> + for (rid = 0; rid < rib_size; rid++) {
> + /* flip rules, make new active */
> + fh = ribs[rid].in_rules;
> + ribs[rid].in_rules = ribs[rid].in_rules_tmp;
> + ribs[rid].in_rules_tmp = fh;
> +
> + switch (ribs[rid].state) {
> + case RECONF_DELETE:
> + rib_free(&ribs[rid]);
> + break;
> + case RECONF_KEEP:
> + if (rde_filter_equal(ribs[rid].in_rules,
> + ribs[rid].in_rules_tmp, NULL))
> + /* rib is in sync */
> + break;
> + ribs[rid].state = RECONF_RELOAD;
> + /* FALLTHROUGH */
> + case RECONF_REINIT:
> + rib_dump(&ribs[0], rde_softreconfig_in, &ribs[rid],
> + AID_UNSPEC);
> + break;
> + case RECONF_RELOAD:
> + case RECONF_NONE:
> + log_warnx("Bad rib reload state");
> + break;
> }
> -
> -done:
> - if (oasp != p->aspath)
> - path_put(oasp);
> - if (nasp != p->aspath)
> - path_put(nasp);
> }
> + LIST_FOREACH(peer, &peerlist, peer_l) {
> + if (peer->reconf_out)
> + rib_dump(&ribs[peer->ribid], rde_softreconfig_out,
> + peer, AID_UNSPEC);
> + else if (peer->reconf_rib)
> + /* dump the full table to neighbors that changed rib */
> + peer_dump(peer->conf.id, AID_UNSPEC);
> + }
> + rde_free_filter(out_rules_tmp);
> + out_rules_tmp = NULL;
> + for (rid = 0; rid <= rib_size; rid++) {
> + rde_free_filter(ribs[rid].in_rules_tmp);
> + ribs[rid].in_rules_tmp = NULL;
> + ribs[rid].state = RECONF_NONE;
> + }
> +
> + log_info("RDE reconfigured");
> + imsg_compose(ibuf_main, IMSG_RECONF_DONE, 0, 0,
> + -1, NULL, 0);
> }
>
> void
> rde_softreconfig_in(struct rib_entry *re, void *ptr)
> {
> + struct rib *rib = ptr;
> struct prefix *p, *np;
> struct pt_entry *pt;
> struct rde_peer *peer;
> struct rde_aspath *asp, *oasp, *nasp;
> enum filter_actions oa, na;
> struct bgpd_addr addr;
> - u_int16_t i;
>
> pt = re->prefix;
> pt_getaddr(pt, &addr);
> for (p = LIST_FIRST(&re->prefix_h); p != NULL; p = np) {
> + /*
> + * prefix_remove() and path_update() may change the object
> + * so cache the values.
> + */
> np = LIST_NEXT(p, rib_l);
> -
> - /* store aspath as prefix may change till we're done */
> asp = p->aspath;
> peer = asp->peer;
>
> - /* XXX how can this happen ??? */
> - if (peer->reconf_in == 0)
> - continue;
> -
> - for (i = 1; i < rib_size; i++) {
> - /* only active ribs need a softreconfig rerun */
> - if (ribs[i].state != RECONF_KEEP)
> - continue;
> -
> - /* check if prefix changed */
> - oa = rde_filter(i, &oasp, rules_l, peer, asp, &addr,
> - pt->prefixlen, peer, DIR_IN);
> - na = rde_filter(i, &nasp, newrules, peer, asp, &addr,
> - pt->prefixlen, peer, DIR_IN);
> + /* check if prefix changed */
> + if (rib->state == RECONF_RELOAD) {
> + oa = rde_filter(rib->in_rules_tmp, &oasp, peer,
> + asp, &addr, pt->prefixlen, peer);
> oasp = oasp != NULL ? oasp : asp;
> - nasp = nasp != NULL ? nasp : asp;
> -
> - if (oa == ACTION_DENY && na == ACTION_DENY)
> - /* nothing todo */
> - goto done;
> - if (oa == ACTION_DENY && na == ACTION_ALLOW) {
> - /* update Local-RIB */
> - path_update(&ribs[i], peer, nasp, &addr,
> - pt->prefixlen);
> - goto done;
> - }
> - if (oa == ACTION_ALLOW && na == ACTION_DENY) {
> - /* remove from Local-RIB */
> - prefix_remove(&ribs[i], peer, &addr,
> - pt->prefixlen, 0);
> - goto done;
> - }
> - if (oa == ACTION_ALLOW && na == ACTION_ALLOW) {
> - if (path_compare(nasp, oasp) == 0)
> - goto done;
> - /* send update */
> - path_update(&ribs[i], peer, nasp, &addr,
> - pt->prefixlen);
> - }
> -
> -done:
> - if (oasp != asp)
> - path_put(oasp);
> - if (nasp != asp)
> - path_put(nasp);
> + } else {
> + /* make sure we update everything for RECONF_REINIT */
> + oa = ACTION_DENY;
> + oasp = asp;
> }
> - }
> -}
> -
> -void
> -rde_softreconfig_load(struct rib_entry *re, void *ptr)
> -{
> - struct rib *rib = ptr;
> - struct prefix *p, *np;
> - struct pt_entry *pt;
> - struct rde_peer *peer;
> - struct rde_aspath *asp, *nasp;
> - enum filter_actions action;
> - struct bgpd_addr addr;
> -
> - pt = re->prefix;
> - pt_getaddr(pt, &addr);
> - for (p = LIST_FIRST(&re->prefix_h); p != NULL; p = np) {
> - np = LIST_NEXT(p, rib_l);
> -
> - /* store aspath as prefix may change till we're done */
> - asp = p->aspath;
> - peer = asp->peer;
> -
> - action = rde_filter(rib->id, &nasp, newrules, peer, asp, &addr,
> - pt->prefixlen, peer, DIR_IN);
> + na = rde_filter(rib->in_rules, &nasp, peer, asp,
> + &addr, pt->prefixlen, peer);
> nasp = nasp != NULL ? nasp : asp;
>
> - if (action == ACTION_ALLOW) {
> + /* go through all 4 possible combinations */
> + /* if (oa == ACTION_DENY && na == ACTION_DENY) */
> + /* nothing todo */
> + if (oa == ACTION_DENY && na == ACTION_ALLOW) {
> /* update Local-RIB */
> path_update(rib, peer, nasp, &addr, pt->prefixlen);
> + } else if (oa == ACTION_ALLOW && na == ACTION_DENY) {
> + /* remove from Local-RIB */
> + prefix_remove(rib, peer, &addr, pt->prefixlen, 0);
> + } else if (oa == ACTION_ALLOW && na == ACTION_ALLOW) {
> + if (path_compare(nasp, oasp) != 0)
> + /* send update */
> + path_update(rib, peer, nasp, &addr,
> + pt->prefixlen);
> }
>
> + if (oasp != asp)
> + path_put(oasp);
> if (nasp != asp)
> path_put(nasp);
> }
> }
>
> void
> -rde_softreconfig_load_peer(struct rib_entry *re, void *ptr)
> +rde_softreconfig_out(struct rib_entry *re, void *ptr)
> {
> - struct rde_peer *peer = ptr;
> struct prefix *p = re->active;
> struct pt_entry *pt;
> - struct rde_aspath *nasp;
> - enum filter_actions na;
> + struct rde_peer *peer = ptr;
> + struct rde_aspath *oasp, *nasp;
> + enum filter_actions oa, na;
> struct bgpd_addr addr;
>
> + if (peer->conf.id == 0)
> + fatalx("King Bula troubled by bad peer");
> +
> + if (p == NULL)
> + return;
> +
> pt = re->prefix;
> pt_getaddr(pt, &addr);
>
> - /* check if prefix was announced */
> if (up_test_update(peer, p) != 1)
> return;
>
> - na = rde_filter(re->ribid, &nasp, newrules, peer, p->aspath,
> - &addr, pt->prefixlen, p->aspath->peer, DIR_OUT);
> + oa = rde_filter(out_rules_tmp, &oasp, peer, p->aspath,
> + &addr, pt->prefixlen, p->aspath->peer);
> + na = rde_filter(out_rules, &nasp, peer, p->aspath,
> + &addr, pt->prefixlen, p->aspath->peer);
> + oasp = oasp != NULL ? oasp : p->aspath;
> nasp = nasp != NULL ? nasp : p->aspath;
>
> - if (na == ACTION_DENY)
> + /* go through all 4 possible combinations */
> + /* if (oa == ACTION_DENY && na == ACTION_DENY) */
> /* nothing todo */
> - goto done;
> + if (oa == ACTION_DENY && na == ACTION_ALLOW) {
> + /* send update */
> + up_generate(peer, nasp, &addr, pt->prefixlen);
> + } else if (oa == ACTION_ALLOW && na == ACTION_DENY) {
> + /* send withdraw */
> + up_generate(peer, NULL, &addr, pt->prefixlen);
> + } else if (oa == ACTION_ALLOW && na == ACTION_ALLOW) {
> + /* send update if path attributes changed */
> + if (path_compare(nasp, oasp) != 0)
> + up_generate(peer, nasp, &addr, pt->prefixlen);
> + }
>
> - /* send update */
> - up_generate(peer, nasp, &addr, pt->prefixlen);
> -done:
> + if (oasp != p->aspath)
> + path_put(oasp);
> if (nasp != p->aspath)
> path_put(nasp);
> }
> @@ -2858,8 +2800,8 @@ rde_softreconfig_unload_peer(struct rib_
> if (up_test_update(peer, p) != 1)
> return;
>
> - oa = rde_filter(re->ribid, &oasp, rules_l, peer, p->aspath,
> - &addr, pt->prefixlen, p->aspath->peer, DIR_OUT);
> + oa = rde_filter(out_rules_tmp, &oasp, peer, p->aspath,
> + &addr, pt->prefixlen, p->aspath->peer);
> oasp = oasp != NULL ? oasp : p->aspath;
>
> if (oa == ACTION_DENY)
> @@ -2887,7 +2829,7 @@ rde_up_dump_upcall(struct rib_entry *re,
> fatalx("King Bula: monstrous evil horror.");
> if (re->active == NULL)
> return;
> - up_generate_updates(rules_l, peer, re->active, NULL);
> + up_generate_updates(out_rules, peer, re->active, NULL);
> }
>
> void
> @@ -2910,7 +2852,7 @@ rde_generate_updates(u_int16_t ribid, st
> continue;
> if (peer->state != PEER_UP)
> continue;
> - up_generate_updates(rules_l, peer, new, old);
> + up_generate_updates(out_rules, peer, new, old);
> }
> }
>
> @@ -3342,7 +3284,7 @@ peer_dump(u_int32_t id, u_int8_t aid)
> }
>
> if (peer->conf.announce_type == ANNOUNCE_DEFAULT_ROUTE)
> - up_generate_default(rules_l, peer, aid);
> + up_generate_default(out_rules, peer, aid);
> else
> rib_dump(&ribs[peer->ribid], rde_up_dump_upcall, peer, aid);
> if (peer->capa.grestart.restart)
> @@ -3543,7 +3485,6 @@ void
> rde_shutdown(void)
> {
> struct rde_peer *p;
> - struct filter_rule *r;
> u_int32_t i;
>
> /*
> @@ -3559,12 +3500,10 @@ rde_shutdown(void)
> peer_down(p->conf.id);
>
> /* free filters */
> - while ((r = TAILQ_FIRST(rules_l)) != NULL) {
> - TAILQ_REMOVE(rules_l, r, entry);
> - filterset_free(&r->set);
> - free(r);
> + rde_free_filter(out_rules);
> + for (i = 0; i <= rib_size; i++) {
> + rde_free_filter(ribs[i].in_rules);
> }
> - free(rules_l);
>
> nexthop_shutdown();
> path_shutdown();
> Index: rde.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
> retrieving revision 1.144
> diff -u -p -r1.144 rde.h
> --- rde.h 12 Sep 2012 05:56:22 -0000 1.144
> +++ rde.h 12 Oct 2012 06:28:07 -0000
> @@ -76,7 +76,6 @@ struct rde_peer {
> u_int16_t ribid;
> u_int16_t short_as;
> u_int16_t mrt_idx;
> - u_int8_t reconf_in; /* in filter changed */
> u_int8_t reconf_out; /* out filter changed */
> u_int8_t reconf_rib; /* rib changed */
> };
> @@ -287,6 +286,8 @@ struct rib_entry {
> struct rib {
> char name[PEER_DESCR_LEN];
> struct rib_tree rib;
> + struct filter_head *in_rules;
> + struct filter_head *in_rules_tmp;
> u_int rtableid;
> u_int16_t flags;
> u_int16_t id;
> @@ -325,6 +326,7 @@ u_int32_t rde_local_as(void);
> int rde_noevaluate(void);
> int rde_decisionflags(void);
> int rde_as4byte(struct rde_peer *);
> +void rde_free_filter(struct filter_head *);
>
> /* rde_attr.c */
> int attr_write(void *, u_int16_t, u_int8_t, u_int8_t, void *,
> @@ -379,14 +381,13 @@ int community_ext_conv(struct filter_e
> void prefix_evaluate(struct prefix *, struct rib_entry *);
>
> /* rde_filter.c */
> -enum filter_actions rde_filter(u_int16_t, struct rde_aspath **,
> - struct filter_head *, struct rde_peer *,
> - struct rde_aspath *, struct bgpd_addr *, u_int8_t,
> - struct rde_peer *, enum directions);
> +enum filter_actions rde_filter(struct filter_head *, struct rde_aspath **,
> + struct rde_peer *, struct rde_aspath *,
> + struct bgpd_addr *, u_int8_t, struct rde_peer *);
> void rde_apply_set(struct rde_aspath *, struct filter_set_head *,
> u_int8_t, struct rde_peer *, struct rde_peer *);
> int rde_filter_equal(struct filter_head *, struct filter_head *,
> - struct rde_peer *, enum directions);
> + struct rde_peer *);
>
> /* rde_prefix.c */
> #define pt_empty(pt) ((pt)->refcnt == 0)
> Index: rde_filter.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_filter.c,v
> retrieving revision 1.67
> diff -u -p -r1.67 rde_filter.c
> --- rde_filter.c 20 Sep 2011 21:19:06 -0000 1.67
> +++ rde_filter.c 12 Oct 2012 06:25:28 -0000
> @@ -30,9 +30,9 @@ int rde_filter_match(struct filter_rule
> int filterset_equal(struct filter_set_head *, struct filter_set_head *);
>
> enum filter_actions
> -rde_filter(u_int16_t ribid, struct rde_aspath **new, struct filter_head
> *rules,
> +rde_filter(struct filter_head *rules, struct rde_aspath **new,
> struct rde_peer *peer, struct rde_aspath *asp, struct bgpd_addr *prefix,
> - u_int8_t prefixlen, struct rde_peer *from, enum directions dir)
> + u_int8_t prefixlen, struct rde_peer *from)
> {
> struct filter_rule *f;
> enum filter_actions action = ACTION_ALLOW; /* default allow */
> @@ -47,11 +47,10 @@ rde_filter(u_int16_t ribid, struct rde_a
> */
> return (ACTION_DENY);
>
> + if (rules == NULL)
> + return (action);
> +
> TAILQ_FOREACH(f, rules, entry) {
> - if (dir != f->dir)
> - continue;
> - if (dir == DIR_IN && f->peer.ribid != ribid)
> - continue;
> if (f->peer.groupid != 0 &&
> f->peer.groupid != peer->conf.groupid)
> continue;
> @@ -391,42 +390,32 @@ rde_filter_match(struct filter_rule *f,
>
> int
> rde_filter_equal(struct filter_head *a, struct filter_head *b,
> - struct rde_peer *peer, enum directions dir)
> + struct rde_peer *peer)
> {
> struct filter_rule *fa, *fb;
>
> - fa = TAILQ_FIRST(a);
> - fb = TAILQ_FIRST(b);
> + fa = a ? TAILQ_FIRST(a) : NULL;
> + fb = b ? TAILQ_FIRST(b) : NULL;
>
> while (fa != NULL || fb != NULL) {
> - /* skip all rules with wrong direction */
> - if (fa != NULL && dir != fa->dir) {
> - fa = TAILQ_NEXT(fa, entry);
> - continue;
> - }
> - if (fb != NULL && dir != fb->dir) {
> - fb = TAILQ_NEXT(fb, entry);
> - continue;
> - }
> -
> /* skip all rules with wrong peer */
> - if (fa != NULL && fa->peer.groupid != 0 &&
> + if (peer != NULL && fa != NULL && fa->peer.groupid != 0 &&
> fa->peer.groupid != peer->conf.groupid) {
> fa = TAILQ_NEXT(fa, entry);
> continue;
> }
> - if (fa != NULL && fa->peer.peerid != 0 &&
> + if (peer != NULL && fa != NULL && fa->peer.peerid != 0 &&
> fa->peer.peerid != peer->conf.id) {
> fa = TAILQ_NEXT(fa, entry);
> continue;
> }
>
> - if (fb != NULL && fb->peer.groupid != 0 &&
> + if (peer != NULL && fb != NULL && fb->peer.groupid != 0 &&
> fb->peer.groupid != peer->conf.groupid) {
> fb = TAILQ_NEXT(fb, entry);
> continue;
> }
> - if (fb != NULL && fb->peer.peerid != 0 &&
> + if (peer != NULL && fb != NULL && fb->peer.peerid != 0 &&
> fb->peer.peerid != peer->conf.id) {
> fb = TAILQ_NEXT(fb, entry);
> continue;
> @@ -450,6 +439,22 @@ rde_filter_equal(struct filter_head *a,
> fb = TAILQ_NEXT(fb, entry);
> }
> return (1);
> +}
> +
> +void
> +rde_free_filter(struct filter_head *fh)
> +{
> + struct filter_rule *r;
> +
> + if (fh == NULL)
> + return;
> +
> + while ((r = TAILQ_FIRST(fh)) != NULL) {
> + TAILQ_REMOVE(fh, r, entry);
> + filterset_free(&r->set);
> + free(r);
> + }
> + free(fh);
> }
>
> /* free a filterset and take care of possible name2id references */
> Index: rde_rib.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
> retrieving revision 1.134
> diff -u -p -r1.134 rde_rib.c
> --- rde_rib.c 12 Sep 2012 05:56:22 -0000 1.134
> +++ rde_rib.c 19 Sep 2012 10:48:42 -0000
> @@ -82,6 +82,11 @@ rib_new(char *name, u_int rtableid, u_in
> ribs[id].flags = flags;
> ribs[id].rtableid = rtableid;
>
> + ribs[id].in_rules = calloc(1, sizeof(struct filter_head));
> + if (ribs[id].in_rules == NULL)
> + fatal(NULL);
> + TAILQ_INIT(ribs[id].in_rules);
> +
> return (id);
> }
>
> Index: rde_update.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 rde_update.c
> --- rde_update.c 12 Apr 2012 17:31:05 -0000 1.80
> +++ rde_update.c 19 Sep 2012 10:58:21 -0000
> @@ -411,9 +411,8 @@ up_generate_updates(struct filter_head *
> return;
>
> pt_getaddr(old->prefix, &addr);
> - if (rde_filter(peer->ribid, NULL, rules, peer, old->aspath,
> - &addr, old->prefix->prefixlen, old->aspath->peer,
> - DIR_OUT) == ACTION_DENY)
> + if (rde_filter(rules, NULL, peer, old->aspath, &addr,
> + old->prefix->prefixlen, old->aspath->peer) == ACTION_DENY)
> return;
>
> /* withdraw prefix */
> @@ -430,9 +429,8 @@ up_generate_updates(struct filter_head *
> }
>
> pt_getaddr(new->prefix, &addr);
> - if (rde_filter(peer->ribid, &asp, rules, peer, new->aspath,
> - &addr, new->prefix->prefixlen, new->aspath->peer,
> - DIR_OUT) == ACTION_DENY) {
> + if (rde_filter(rules, &asp, peer, new->aspath, &addr,
> + new->prefix->prefixlen, new->aspath->peer) == ACTION_DENY) {
> path_put(asp);
> up_generate_updates(rules, peer, NULL, old);
> return;
> @@ -474,8 +472,8 @@ up_generate_default(struct filter_head *
> bzero(&addr, sizeof(addr));
> addr.aid = aid;
>
> - if (rde_filter(peer->ribid, &fasp, rules, peer, asp, &addr, 0, NULL,
> - DIR_OUT) == ACTION_DENY) {
> + if (rde_filter(rules, &fasp, peer, asp, &addr, 0, NULL) ==
> + ACTION_DENY) {
> path_put(fasp);
> path_put(asp);
> return;
>
--
:wq Claudio
