On Sat, Mar 21, 2020 at 05:25:45PM +0100, Denis Fondras wrote:
> Biggest chunk is rework of rde_asext_get()/rde_asext_put().
> Also change get_net_link() and get_rtr_link() to work like ospfd couterpart.
Reads good to me and I didn't spot any issues running tests with it.
One question: why "if 0" the "Dump SPF tree to log"?
>
> Index: rde.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/rde.c,v
> retrieving revision 1.84
> diff -u -p -r1.84 rde.c
> --- rde.c 17 Feb 2020 08:12:22 -0000 1.84
> +++ rde.c 21 Mar 2020 16:04:47 -0000
> @@ -59,8 +59,9 @@ int rde_req_list_exists(struct rde_nbr
> void rde_req_list_del(struct rde_nbr *, struct lsa_hdr *);
> void rde_req_list_free(struct rde_nbr *);
>
> -struct lsa *rde_asext_get(struct kroute *);
> -struct lsa *rde_asext_put(struct kroute *);
> +struct iface *rde_asext_lookup(struct in6_addr, int);
> +void rde_asext_get(struct kroute *);
> +void rde_asext_put(struct kroute *);
>
> int comp_asext(struct lsa *, struct lsa *);
> struct lsa *orig_asext_lsa(struct kroute *, u_int16_t);
> @@ -217,6 +218,7 @@ __dead void
> rde_shutdown(void)
> {
> struct area *a;
> + struct vertex *v, *nv;
>
> /* close pipes */
> msgbuf_clear(&iev_ospfe->ibuf.w);
> @@ -232,6 +234,10 @@ rde_shutdown(void)
> LIST_REMOVE(a, entry);
> area_del(a);
> }
> + for (v = RB_MIN(lsa_tree, &asext_tree); v != NULL; v = nv) {
> + nv = RB_NEXT(lsa_tree, &asext_tree, v);
> + vertex_free(v);
> + }
> rde_nbr_free();
>
> free(iev_ospfe);
> @@ -643,8 +649,6 @@ rde_dispatch_parent(int fd, short event,
> struct kroute kr;
> struct imsgev *iev = bula;
> struct imsgbuf *ibuf = &iev->ibuf;
> - struct lsa *lsa;
> - struct vertex *v;
> ssize_t n;
> int shut = 0, link_ok, prev_link_ok, orig_lsa;
> unsigned int ifindex;
> @@ -676,13 +680,7 @@ rde_dispatch_parent(int fd, short event,
> break;
> }
> memcpy(&kr, imsg.data, sizeof(kr));
> -
> - if ((lsa = rde_asext_get(&kr)) != NULL) {
> - v = lsa_find(NULL, lsa->hdr.type,
> - lsa->hdr.ls_id, lsa->hdr.adv_rtr);
> -
> - lsa_merge(nbrself, lsa, v);
> - }
> + rde_asext_get(&kr);
> break;
> case IMSG_NETWORK_DEL:
> if (imsg.hdr.len != IMSG_HEADER_SIZE + sizeof(kr)) {
> @@ -691,20 +689,7 @@ rde_dispatch_parent(int fd, short event,
> break;
> }
> memcpy(&kr, imsg.data, sizeof(kr));
> -
> - if ((lsa = rde_asext_put(&kr)) != NULL) {
> - v = lsa_find(NULL, lsa->hdr.type,
> - lsa->hdr.ls_id, lsa->hdr.adv_rtr);
> -
> - /*
> - * if v == NULL no LSA is in the table and
> - * nothing has to be done.
> - */
> - if (v)
> - lsa_merge(nbrself, lsa, v);
> - else
> - free(lsa);
> - }
> + rde_asext_put(&kr);
> break;
> case IMSG_IFINFO:
> if (imsg.hdr.len != IMSG_HEADER_SIZE +
> @@ -1202,48 +1187,77 @@ rde_req_list_free(struct rde_nbr *nbr)
> /*
> * as-external LSA handling
> */
> -struct lsa *
> -rde_asext_get(struct kroute *kr)
> +struct iface *
> +rde_asext_lookup(struct in6_addr prefix, int plen)
> {
> +
> struct area *area;
> struct iface *iface;
> struct iface_addr *ia;
> - struct in6_addr addr;
> -
> - LIST_FOREACH(area, &rdeconf->area_list, entry)
> - LIST_FOREACH(iface, &area->iface_list, entry)
> + struct in6_addr ina, inb;
> +
> + LIST_FOREACH(area, &rdeconf->area_list, entry) {
> + LIST_FOREACH(iface, &area->iface_list, entry) {
> TAILQ_FOREACH(ia, &iface->ifa_list, entry) {
> if (IN6_IS_ADDR_LINKLOCAL(&ia->addr))
> continue;
>
> - inet6applymask(&addr, &ia->addr,
> - kr->prefixlen);
> - if (!memcmp(&addr, &kr->prefix,
> - sizeof(addr)) && kr->prefixlen ==
> - ia->prefixlen) {
> - /* already announced as Prefix LSA */
> - log_debug("rde_asext_get: %s/%d is "
> - "part of prefix LSA",
> - log_in6addr(&kr->prefix),
> - kr->prefixlen);
> - return (NULL);
> - }
> + inet6applymask(&ina, &ia->addr, ia->prefixlen);
> + inet6applymask(&inb, &prefix, ia->prefixlen);
> + if (IN6_ARE_ADDR_EQUAL(&ina, &inb) &&
> + (plen == -1 || plen == ia->prefixlen))
> + return (iface);
> }
> + }
> + }
> + return (NULL);
> +}
> +
> +void
> +rde_asext_get(struct kroute *kr)
> +{
> + struct vertex *v;
> + struct lsa *lsa;
> +
> + if (rde_asext_lookup(kr->prefix, kr->prefixlen)) {
> + /* already announced as (stub) net LSA */
> + log_debug("rde_asext_get: %s/%d is net LSA",
> + log_in6addr(&kr->prefix), kr->prefixlen);
> + return;
> + }
>
> /* update of seqnum is done by lsa_merge */
> - return (orig_asext_lsa(kr, DEFAULT_AGE));
> + if ((lsa = orig_asext_lsa(kr, DEFAULT_AGE))) {
> + v = lsa_find(NULL, lsa->hdr.type, lsa->hdr.ls_id,
> + lsa->hdr.adv_rtr);
> + lsa_merge(nbrself, lsa, v);
> + }
> }
>
> -struct lsa *
> +void
> rde_asext_put(struct kroute *kr)
> {
> + struct vertex *v;
> + struct lsa *lsa;
> /*
> * just try to remove the LSA. If the prefix is announced as
> * stub net LSA lsa_find() will fail later and nothing will happen.
> */
>
> /* remove by reflooding with MAX_AGE */
> - return (orig_asext_lsa(kr, MAX_AGE));
> + if ((lsa = orig_asext_lsa(kr, MAX_AGE))) {
> + v = lsa_find(NULL, lsa->hdr.type, lsa->hdr.ls_id,
> + lsa->hdr.adv_rtr);
> +
> + /*
> + * if v == NULL no LSA is in the table and
> + * nothing has to be done.
> + */
> + if (v)
> + lsa_merge(nbrself, lsa, v);
> + else
> + free(lsa);
> + }
> }
>
> /*
> @@ -1706,8 +1720,7 @@ orig_asext_lsa(struct kroute *kr, u_int1
> }
>
> lsa->hdr.ls_chksum = 0;
> - lsa->hdr.ls_chksum =
> - htons(iso_cksum(lsa, len, LS_CKSUM_OFFSET));
> + lsa->hdr.ls_chksum = htons(iso_cksum(lsa, len, LS_CKSUM_OFFSET));
>
> return (lsa);
> }
> Index: rde_spf.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/rde_spf.c,v
> retrieving revision 1.26
> diff -u -p -r1.26 rde_spf.c
> --- rde_spf.c 22 Dec 2019 11:19:07 -0000 1.26
> +++ rde_spf.c 21 Mar 2020 16:04:47 -0000
> @@ -44,10 +44,11 @@ void calc_nexthop(struct vertex *, str
> struct area *, struct lsa_rtr_link *);
> void rt_nexthop_clear(struct rt_node *);
> void rt_nexthop_add(struct rt_node *, struct v_nexthead *,
> - struct in_addr);
> + u_int16_t, struct in_addr);
> void rt_update(struct in6_addr *, u_int8_t, struct v_nexthead *,
> - u_int32_t, u_int32_t, struct in_addr, struct in_addr,
> - enum path_type, enum dst_type, u_int8_t, u_int32_t);
> + u_int16_t, u_int32_t, u_int32_t, struct in_addr,
> + struct in_addr, enum path_type, enum dst_type, u_int8_t,
> + u_int32_t);
> struct rt_node *rt_lookup(enum dst_type, struct in6_addr *);
> void rt_invalidate(struct area *);
> int linked(struct vertex *, struct vertex *);
> @@ -167,6 +168,7 @@ spf_calc(struct area *area)
> /* spf_dump(area); */
> log_debug("spf_calc: area %s calculated", inet_ntoa(area->id));
>
> +#if 0
> /* Dump SPF tree to log */
> RB_FOREACH(v, lsa_tree, &area->lsa_tree) {
> struct v_nexthop *vn;
> @@ -191,6 +193,7 @@ spf_calc(struct area *area)
> v == spf_root ? "*" : " ", log_rtr_id(htonl(v->adv_rtr)),
> v->type, log_rtr_id(htonl(v->ls_id)), v->cost, hops);
> }
> +#endif
>
> area->num_spf_calc++;
> start_spf_timer();
> @@ -225,7 +228,7 @@ rt_calc(struct vertex *v, struct area *a
> adv_rtr.s_addr = htonl(v->adv_rtr);
> bcopy(&adv_rtr, &ia6.s6_addr[12], sizeof(adv_rtr));
>
> - rt_update(&ia6, 128, &v->nexthop, v->cost, 0, area->id,
> + rt_update(&ia6, 128, &v->nexthop, v->type, v->cost, 0, area->id,
> adv_rtr, PT_INTER_AREA, DT_RTR, flags, 0);
> break;
> case LSA_TYPE_INTRA_A_PREFIX:
> @@ -279,7 +282,7 @@ rt_calc(struct vertex *v, struct area *a
> adv_rtr.s_addr = htonl(w->adv_rtr);
>
> rt_update(&ia6, prefix->prefixlen, &w->nexthop,
> - w->cost + ntohs(prefix->metric), 0,
> + v->type, w->cost + ntohs(prefix->metric), 0,
> area->id, adv_rtr, PT_INTRA_AREA, DT_NET,
> flags, 0);
> }
> @@ -315,9 +318,10 @@ rt_calc(struct vertex *v, struct area *a
> bzero(&ia6, sizeof(ia6));
> bcopy(prefix + 1, &ia6, LSA_PREFIXSIZE(prefix->prefixlen));
>
> - rt_update(&ia6, prefix->prefixlen, &w->nexthop, w->cost +
> - (ntohs(v->lsa->data.rtr_sum.metric) & LSA_METRIC_MASK), 0,
> - area->id, adv_rtr, PT_INTER_AREA, DT_NET, 0, 0);
> + rt_update(&ia6, prefix->prefixlen, &w->nexthop, v->type,
> + w->cost + (ntohs(v->lsa->data.rtr_sum.metric) &
> + LSA_METRIC_MASK), 0, area->id, adv_rtr, PT_INTER_AREA,
> + DT_NET, 0, 0);
> break;
> case LSA_TYPE_INTER_A_ROUTER:
> /* XXX if ABR only look at area 0.0.0.0 LSA */
> @@ -343,7 +347,7 @@ rt_calc(struct vertex *v, struct area *a
> bcopy(&v->lsa->data.rtr_sum.dest_rtr_id, &ia6.s6_addr[12],
> 4);
>
> - rt_update(&ia6, 128, &w->nexthop, w->cost +
> + rt_update(&ia6, 128, &w->nexthop, v->type, w->cost +
> (ntohs(v->lsa->data.rtr_sum.metric) & LSA_METRIC_MASK), 0,
> area->id, adv_rtr, PT_INTER_AREA, DT_RTR, 0, 0);
> break;
> @@ -434,8 +438,8 @@ asext_calc(struct vertex *v)
> rn->ifindex);
> }
>
> - rt_update(&addr, prefix->prefixlen, &v->nexthop, v->cost, cost2,
> - area, adv_rtr, type, DT_NET, 0, ext_tag);
> + rt_update(&addr, prefix->prefixlen, &v->nexthop, v->type,
> + v->cost, cost2, area, adv_rtr, type, DT_NET, 0, ext_tag);
> break;
> default:
> fatalx("asext_calc: invalid LSA type");
> @@ -863,7 +867,7 @@ rt_nexthop_clear(struct rt_node *r)
> }
>
> void
> -rt_nexthop_add(struct rt_node *r, struct v_nexthead *vnh,
> +rt_nexthop_add(struct rt_node *r, struct v_nexthead *vnh, u_int16_t type,
> struct in_addr adv_rtr)
> {
> struct v_nexthop *vn;
> @@ -876,7 +880,9 @@ rt_nexthop_add(struct rt_node *r, struct
> continue;
>
> rn->adv_rtr.s_addr = adv_rtr.s_addr;
> - rn->connected = vn->prev == spf_root;
> + rn->connected = (type == LSA_TYPE_NETWORK &&
> + vn->prev == spf_root) ||
> + (IN6_IS_ADDR_UNSPECIFIED(&vn->nexthop));
> rn->invalid = 0;
>
> r->invalid = 0;
> @@ -973,7 +979,7 @@ rt_dump(struct in_addr area, pid_t pid,
>
> void
> rt_update(struct in6_addr *prefix, u_int8_t prefixlen, struct v_nexthead
> *vnh,
> - u_int32_t cost, u_int32_t cost2, struct in_addr area,
> + u_int16_t v_type, u_int32_t cost, u_int32_t cost2, struct in_addr area,
> struct in_addr adv_rtr, enum path_type p_type, enum dst_type d_type,
> u_int8_t flags, u_int32_t tag)
> {
> @@ -999,7 +1005,7 @@ rt_update(struct in6_addr *prefix, u_int
> rte->flags = flags;
> rte->ext_tag = tag;
>
> - rt_nexthop_add(rte, vnh, adv_rtr);
> + rt_nexthop_add(rte, vnh, v_type, adv_rtr);
>
> rt_insert(rte);
> } else {
> @@ -1054,7 +1060,7 @@ rt_update(struct in6_addr *prefix, u_int
> }
>
> if (equal || better)
> - rt_nexthop_add(rte, vnh, adv_rtr);
> + rt_nexthop_add(rte, vnh, v_type, adv_rtr);
> }
> }
>
> @@ -1115,7 +1121,7 @@ get_rtr_link(struct vertex *v, unsigned
> v = lsa_find_rtr_frag(v->area, htonl(v->adv_rtr), frag++);
> } while (v);
>
> - return (NULL);
> + fatalx("get_rtr_link: index not found");
> }
>
> /* network LSA links */
> @@ -1124,21 +1130,23 @@ get_net_link(struct vertex *v, unsigned
> {
> struct lsa_net_link *net_link = NULL;
> char *buf = (char *)v->lsa;
> - unsigned int i;
> + unsigned int i, nlinks;
>
> if (v->type != LSA_TYPE_NETWORK)
> fatalx("get_net_link: invalid LSA type");
>
> - /* number of links validated earlier by lsa_check() */
> net_link = (struct lsa_net_link *)(buf + sizeof(v->lsa->hdr) +
> sizeof(struct lsa_net));
> - for (i = 0; i < lsa_num_links(v); i++) {
> +
> + /* number of links validated earlier by lsa_check() */
> + nlinks = lsa_num_links(v);
> + for (i = 0; i < nlinks; i++) {
> if (i == idx)
> return (net_link);
> net_link++;
> }
>
> - return (NULL);
> + fatalx("get_net_link: index not found");
> }
>
> /* misc */
>
