On Wed, Jul 20, 2022 at 12:40:18PM +0200, Claudio Jeker wrote:
> The it's just a rtlabel refcount leak diff turned into a somewhat larger
> diff. First I noticed that expand_networks() was not used for l3vpns which
> will cause problems down the line. So alter expand_networks to also handle
> l3vpn network settings.
>
> Then I looked at kr_net_reload() and kr_net_find() and noticed that
> neither rtlabel nor priority network statements are handled properly. So
> make sure we match the network statements also against rtlabel and
> priority.
>
> I named network_free() after filterset_free() and not free_network() which
> is used in config.c. One day I may go an make the naming consistent.
This looks much better.
ok tb
> --
> :wq Claudio
>
> Index: bgpd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.c,v
> retrieving revision 1.248
> diff -u -p -r1.248 bgpd.c
> --- bgpd.c 23 Jun 2022 13:09:03 -0000 1.248
> +++ bgpd.c 20 Jul 2022 09:50:45 -0000
> @@ -599,7 +599,9 @@ send_config(struct bgpd_config *conf)
>
> reconfpending = 3; /* one per child */
>
> - expand_networks(conf);
> + expand_networks(conf, &conf->networks);
> + SIMPLEQ_FOREACH(vpn, &conf->l3vpns, entry)
> + expand_networks(conf, &vpn->net_l);
>
> cflags = conf->flags;
>
> Index: bgpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.441
> diff -u -p -r1.441 bgpd.h
> --- bgpd.h 11 Jul 2022 17:08:21 -0000 1.441
> +++ bgpd.h 20 Jul 2022 10:02:17 -0000
> @@ -476,8 +476,8 @@ struct network_config {
> struct filter_set_head attrset;
> char psname[SET_NAME_LEN];
> uint64_t rd;
> - uint16_t rtlabel;
> enum network_type type;
> + uint16_t rtlabel;
> uint8_t prefixlen;
> uint8_t priority;
> uint8_t old; /* used for reloading */
> @@ -1275,6 +1275,7 @@ int control_imsg_relay(struct imsg *);
> /* config.c */
> struct bgpd_config *new_config(void);
> void copy_config(struct bgpd_config *, struct bgpd_config *);
> +void network_free(struct network *);
> void free_l3vpns(struct l3vpn_head *);
> void free_config(struct bgpd_config *);
> void free_prefixsets(struct prefixset_head *);
> @@ -1285,7 +1286,7 @@ void free_rtrs(struct rtr_config_head *
> void filterlist_free(struct filter_head *);
> int host(const char *, struct bgpd_addr *, uint8_t *);
> uint32_t get_bgpid(void);
> -void expand_networks(struct bgpd_config *);
> +void expand_networks(struct bgpd_config *, struct network_head *);
> RB_PROTOTYPE(prefixset_tree, prefixset_item, entry, prefixset_cmp);
> int roa_cmp(struct roa *, struct roa *);
> RB_PROTOTYPE(roa_tree, roa, entry, roa_cmp);
> Index: config.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/config.c,v
> retrieving revision 1.102
> diff -u -p -r1.102 config.c
> --- config.c 5 Jun 2022 12:43:13 -0000 1.102
> +++ config.c 20 Jul 2022 09:50:16 -0000
> @@ -86,14 +86,21 @@ copy_config(struct bgpd_config *to, stru
> }
>
> void
> +network_free(struct network *n)
> +{
> + rtlabel_unref(n->net.rtlabel);
> + filterset_free(&n->net.attrset);
> + free(n);
> +}
> +
> +void
> free_networks(struct network_head *networks)
> {
> struct network *n;
>
> while ((n = TAILQ_FIRST(networks)) != NULL) {
> TAILQ_REMOVE(networks, n, entry);
> - filterset_free(&n->net.attrset);
> - free(n);
> + network_free(n);
> }
> }
>
> @@ -431,7 +438,8 @@ host_ip(const char *s, struct bgpd_addr
> sa2addr(res->ai_addr, h, NULL);
> freeaddrinfo(res);
> } else { /* ie. for 10/8 parsing */
> - if ((bits = inet_net_pton(AF_INET, s, &h->v4, sizeof(h->v4)))
> == -1)
> + if ((bits = inet_net_pton(AF_INET, s, &h->v4,
> + sizeof(h->v4))) == -1)
> return (0);
> *len = bits;
> h->aid = AID_INET;
> @@ -502,10 +510,9 @@ prepare_listeners(struct bgpd_config *co
> }
>
> void
> -expand_networks(struct bgpd_config *c)
> +expand_networks(struct bgpd_config *c, struct network_head *nw)
> {
> struct network *n, *m, *tmp;
> - struct network_head *nw = &c->networks;
> struct prefixset *ps;
> struct prefixset_item *psi;
>
> @@ -527,8 +534,7 @@ expand_networks(struct bgpd_config *c)
> &m->net.attrset);
> TAILQ_INSERT_TAIL(nw, m, entry);
> }
> - filterset_free(&n->net.attrset);
> - free(n);
> + network_free(n);
> }
> }
> }
> Index: kroute.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> retrieving revision 1.274
> diff -u -p -r1.274 kroute.c
> --- kroute.c 19 Jul 2022 10:26:19 -0000 1.274
> +++ kroute.c 20 Jul 2022 10:05:24 -0000
> @@ -141,7 +141,6 @@ int kr4_delete(struct ktable *, struct k
> int kr6_delete(struct ktable *, struct kroute_full *);
> int krVPN4_delete(struct ktable *, struct kroute_full *);
> int krVPN6_delete(struct ktable *, struct kroute_full *);
> -void kr_net_delete(struct network *);
> int kr_net_match(struct ktable *, struct network_config *, uint16_t, int);
> struct network *kr_net_find(struct ktable *, struct network *);
> void kr_net_clear(struct ktable *);
> @@ -1243,13 +1242,6 @@ kr_ifinfo(char *ifname)
> }
> }
>
> -void
> -kr_net_delete(struct network *n)
> -{
> - filterset_free(&n->net.attrset);
> - free(n);
> -}
> -
> static int
> kr_net_redist_add(struct ktable *kt, struct network_config *net,
> struct filter_set_head *attr, int dynamic)
> @@ -1370,7 +1362,9 @@ kr_net_find(struct ktable *kt, struct ne
> TAILQ_FOREACH(xn, &kt->krn, entry) {
> if (n->net.type != xn->net.type ||
> n->net.prefixlen != xn->net.prefixlen ||
> - n->net.rd != xn->net.rd)
> + n->net.rd != xn->net.rd ||
> + n->net.rtlabel != xn->net.rtlabel ||
> + n->net.priority != xn->net.priority)
> continue;
> if (memcmp(&n->net.prefix, &xn->net.prefix,
> sizeof(n->net.prefix)) == 0)
> @@ -1397,7 +1391,7 @@ kr_net_reload(u_int rtableid, uint64_t r
> xn->net.old = 0;
> filterset_free(&xn->net.attrset);
> filterset_move(&n->net.attrset, &xn->net.attrset);
> - kr_net_delete(n);
> + network_free(n);
> } else
> TAILQ_INSERT_TAIL(&kt->krn, n, entry);
> }
> @@ -1412,7 +1406,7 @@ kr_net_clear(struct ktable *kt)
> TAILQ_REMOVE(&kt->krn, n, entry);
> if (n->net.type == NETWORK_DEFAULT)
> kr_net_redist_del(kt, &n->net, 0);
> - kr_net_delete(n);
> + network_free(n);
> }
> }
>
> @@ -1556,7 +1550,7 @@ ktable_postload(void)
> TAILQ_REMOVE(&kt->krn, n, entry);
> if (n->net.type == NETWORK_DEFAULT)
> kr_net_redist_del(kt, &n->net, 0);
> - kr_net_delete(n);
> + network_free(n);
> }
> }
> }
>
