On Sat, Feb 15, 2020 at 11:37:12AM +0100, Denis Fondras wrote:
> 3 changes in rde_lsdb.c
> - lsa_find_lsid() has redondant parameters
> - call to lsa_self() can be simplified (== ospfd)
> - update debug messages to be more suitable
> 

ok remi@

> Index: rde.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/rde.c,v
> retrieving revision 1.83
> diff -u -p -r1.83 rde.c
> --- rde.c     21 Jan 2020 15:17:12 -0000      1.83
> +++ rde.c     27 Jan 2020 17:11:52 -0000
> @@ -455,17 +455,10 @@ rde_dispatch_imsg(int fd, short event, v
>  
>                               rde_req_list_del(nbr, &lsa->hdr);
>  
> -                             self = lsa_self(lsa);
> -                             if (self) {
> -                                     if (v == NULL)
> -                                             /* LSA is no longer announced,
> -                                              * remove by premature aging. */
> -                                             lsa_flush(nbr, lsa);
> -                                     else
> -                                             lsa_reflood(v, lsa);
> -                             } else if (lsa_add(nbr, lsa))
> -                                     /* delayed lsa, don't flood yet */
> -                                     break;
> +                             if (!(self = lsa_self(nbr, lsa, v)))
> +                                     if (lsa_add(nbr, lsa))
> +                                             /* delayed lsa */
> +                                             break;
>  
>                               /* flood and perhaps ack LSA */
>                               imsg_compose_event(iev_ospfe, IMSG_LS_FLOOD,
> @@ -1683,8 +1676,7 @@ orig_asext_lsa(struct kroute *kr, u_int1
>       memcpy((char *)lsa + sizeof(struct lsa_hdr) + sizeof(struct lsa_asext),
>           &kr->prefix, LSA_PREFIXSIZE(kr->prefixlen));
>  
> -     lsa->hdr.ls_id = lsa_find_lsid(&asext_tree, lsa->hdr.type,
> -         lsa->hdr.adv_rtr, comp_asext, lsa);
> +     lsa->hdr.ls_id = lsa_find_lsid(&asext_tree, comp_asext, lsa);
>  
>       if (age == MAX_AGE) {
>               /* inherit metric and ext_tag from the current LSA,
> Index: rde.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/rde.h,v
> retrieving revision 1.24
> diff -u -p -r1.24 rde.h
> --- rde.h     21 Jan 2020 15:17:12 -0000      1.24
> +++ rde.h     27 Jan 2020 17:11:52 -0000
> @@ -145,9 +145,7 @@ void               vertex_nexthop_add(struct vertex 
>                   const struct in6_addr *, u_int32_t);
>  int           lsa_newer(struct lsa_hdr *, struct lsa_hdr *);
>  int           lsa_check(struct rde_nbr *, struct lsa *, u_int16_t);
> -int           lsa_self(struct lsa *);
> -void          lsa_flush(struct rde_nbr *, struct lsa *);
> -void          lsa_reflood(struct vertex *, struct lsa*);
> +int           lsa_self(struct rde_nbr *, struct lsa *, struct vertex *);
>  int           lsa_add(struct rde_nbr *, struct lsa *);
>  void          lsa_del(struct rde_nbr *, struct lsa_hdr *);
>  void          lsa_age(struct vertex *);
> @@ -156,7 +154,7 @@ struct vertex     *lsa_find_rtr(struct area 
>  struct vertex        *lsa_find_rtr_frag(struct area *, u_int32_t, unsigned 
> int);
>  struct vertex        *lsa_find_tree(struct lsa_tree *, u_int16_t, u_int32_t,
>                   u_int32_t);
> -u_int32_t     lsa_find_lsid(struct lsa_tree *, u_int16_t, u_int32_t,
> +u_int32_t     lsa_find_lsid(struct lsa_tree *,
>                   int (*)(struct lsa *, struct lsa *), struct lsa *);
>  u_int16_t     lsa_num_links(struct vertex *);
>  void          lsa_snap(struct rde_nbr *);
> Index: rde_lsdb.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/rde_lsdb.c,v
> retrieving revision 1.42
> diff -u -p -r1.42 rde_lsdb.c
> --- rde_lsdb.c        21 Jan 2020 15:17:13 -0000      1.42
> +++ rde_lsdb.c        27 Jan 2020 17:11:52 -0000
> @@ -192,7 +192,7 @@ lsa_check(struct rde_nbr *nbr, struct ls
>               return (0);
>       }
>       if (ntohs(lsa->hdr.len) != len) {
> -             log_warnx("lsa_check: bad packet size");
> +             log_warnx("lsa_check: bad packet length");
>               return (0);
>       }
>  
> @@ -244,7 +244,7 @@ lsa_check(struct rde_nbr *nbr, struct ls
>               }
>               metric = ntohl(lsa->data.pref_sum.metric);
>               if (metric & ~LSA_METRIC_MASK) {
> -                     log_warnx("lsa_check: bad LSA summary metric");
> +                     log_warnx("lsa_check: bad LSA prefix summary metric");
>                       return (0);
>               }
>               if (lsa_get_prefix(((char *)lsa) + sizeof(lsa->hdr) +
> @@ -263,7 +263,7 @@ lsa_check(struct rde_nbr *nbr, struct ls
>               }
>               metric = ntohl(lsa->data.rtr_sum.metric);
>               if (metric & ~LSA_METRIC_MASK) {
> -                     log_warnx("lsa_check: bad LSA summary metric");
> +                     log_warnx("lsa_check: bad LSA router summary metric");
>                       return (0);
>               }
>               break;
> @@ -379,7 +379,7 @@ lsa_asext_check(struct lsa *lsa, u_int16
>  
>       if ((len % sizeof(u_int32_t)) ||
>           len < sizeof(lsa->hdr) + sizeof(*asext)) {
> -             log_warnx("lsa_asext_check: bad LSA as-external packet");
> +             log_warnx("lsa_asext_check: bad LSA as-external packet size");
>               return (0);
>       }
>  
> @@ -421,38 +421,44 @@ lsa_asext_check(struct lsa *lsa, u_int16
>  }
>  
>  int
> -lsa_self(struct lsa *lsa)
> +lsa_self(struct rde_nbr *nbr, struct lsa *lsa, struct vertex *v)
>  {
> -     return rde_router_id() == lsa->hdr.adv_rtr;
> -}
> +     struct lsa      *dummy;
>  
> -void
> -lsa_flush(struct rde_nbr *nbr, struct lsa *lsa)
> -{
> -     struct lsa      *copy;
> +     if (nbr->self)
> +             return (0);
>  
> -     /*
> -      * The LSA may not be altered because the caller may still
> -      * use it, so a copy needs to be added to the LSDB.
> -      * The copy will be reflooded via the default timeout handler.
> -      */
> -     if ((copy = malloc(ntohs(lsa->hdr.len))) == NULL)
> -             fatal("lsa_flush");
> -     memcpy(copy, lsa, ntohs(lsa->hdr.len));
> -     copy->hdr.age = htons(MAX_AGE);
> -     (void)lsa_add(rde_nbr_self(nbr->area), copy);
> -}
> +     if (rde_router_id() != lsa->hdr.adv_rtr)
> +             return (0);
> +
> +     if (v == NULL) {
> +             /* LSA is no longer announced, remove by premature aging.
> +              * The LSA may not be altered because the caller may still
> +              * use it, so a copy needs to be added to the LSDB.
> +              * The copy will be reflooded via the default timeout handler.
> +              */
> +             if ((dummy = malloc(ntohs(lsa->hdr.len))) == NULL)
> +                     fatal("lsa_self");
> +             memcpy(dummy, lsa, ntohs(lsa->hdr.len));
> +             dummy->hdr.age = htons(MAX_AGE);
> +             /*
> +              * The clue is that by using the remote nbr as originator
> +              * the dummy LSA will be reflooded via the default timeout
> +              * handler.
> +              */
> +             (void)lsa_add(rde_nbr_self(nbr->area), dummy);
> +             return (1);
> +     }
>  
> -void
> -lsa_reflood(struct vertex *v, struct lsa *new)
> -{
>       /*
> -      * We only need to create a new instance by setting the LSA
> -      * sequence number equal to the one of 'new' and calling
> -      * lsa_refresh(). Actual flooding will be done by the caller.
> +      * LSA is still originated, just reflood it. But we need to create
> +      * a new instance by setting the LSA sequence number equal to the
> +      * one of new and calling lsa_refresh(). Flooding will be done by the
> +      * caller.
>        */
> -     v->lsa->hdr.seq_num = new->hdr.seq_num;
> +     v->lsa->hdr.seq_num = lsa->hdr.seq_num;
>       lsa_refresh(v);
> +     return (1);
>  }
>  
>  int
> @@ -461,7 +467,6 @@ lsa_add(struct rde_nbr *nbr, struct lsa 
>       struct lsa_tree *tree;
>       struct vertex   *new, *old;
>       struct timeval   tv, now, res;
> -     int              update = 1;
>  
>       if (LSA_IS_SCOPE_AS(ntohs(lsa->hdr.type)))
>               tree = &asext_tree;
> @@ -470,7 +475,7 @@ lsa_add(struct rde_nbr *nbr, struct lsa 
>       else if (LSA_IS_SCOPE_LLOCAL(ntohs(lsa->hdr.type)))
>               tree = &nbr->iface->lsa_tree;
>       else
> -             fatalx("unknown scope type");
> +             fatalx("%s: unknown scope type", __func__);
>  
>       new = vertex_get(lsa, nbr, tree);
>       old = RB_INSERT(lsa_tree, tree, new);
> @@ -490,13 +495,16 @@ lsa_add(struct rde_nbr *nbr, struct lsa 
>                               fatal("lsa_add");
>                       return (1);
>               }
> -             if (lsa_equal(new->lsa, old->lsa))
> -                     update = 0;
> +             if (!lsa_equal(new->lsa, old->lsa)) {
> +                     if (ntohs(lsa->hdr.type) == LSA_TYPE_LINK)
> +                             orig_intra_area_prefix_lsas(nbr->area);
> +                     if (ntohs(lsa->hdr.type) != LSA_TYPE_EXTERNAL)
> +                             nbr->area->dirty = 1;
> +                     start_spf_timer();
> +             }
>               vertex_free(old);
>               RB_INSERT(lsa_tree, tree, new);
> -     }
> -
> -     if (update) {
> +     } else {
>               if (ntohs(lsa->hdr.type) == LSA_TYPE_LINK)
>                       orig_intra_area_prefix_lsas(nbr->area);
>               if (ntohs(lsa->hdr.type) != LSA_TYPE_EXTERNAL)
> @@ -513,7 +521,7 @@ lsa_add(struct rde_nbr *nbr, struct lsa 
>               tv.tv_sec = MAX_AGE - ntohs(new->lsa->hdr.age);
>  
>       if (evtimer_add(&new->ev, &tv) != 0)
> -             fatal("lsa_add");
> +             fatal("lsa_add: evtimer_add()");
>       return (0);
>  }
>  
> @@ -651,8 +659,8 @@ lsa_find_rtr_frag(struct area *area, u_i
>  }
>  
>  u_int32_t
> -lsa_find_lsid(struct lsa_tree *tree, u_int16_t type, u_int32_t adv_rtr,
> -    int (*cmp)(struct lsa *, struct lsa *), struct lsa *lsa)
> +lsa_find_lsid(struct lsa_tree *tree, int (*cmp)(struct lsa *, struct lsa *),
> +    struct lsa *lsa)
>  {
>  #define MIN(x, y)    ((x) < (y) ? (x) : (y))
>       struct vertex   *v;
> @@ -660,8 +668,8 @@ lsa_find_lsid(struct lsa_tree *tree, u_i
>       u_int32_t        min, cur;
>  
>       key.ls_id = 0;
> -     key.adv_rtr = ntohl(adv_rtr);
> -     key.type = ntohs(type);
> +     key.adv_rtr = ntohl(lsa->hdr.adv_rtr);
> +     key.type = ntohs(lsa->hdr.type);
>  
>       cur = 0;
>       min = 0xffffffffU;
> 

Reply via email to