ospf6d: rework rde_lsdb.c

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

ospf6d: rework rde_lsdb.c

Denis Fondras
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

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 | Threaded
Open this post in threaded view
|

Re: ospf6d: rework rde_lsdb.c

Remi Locherer
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;
>