OSPF LSA may not be refreshed within 1800s, ANVL OSPF test 12.4 fails

Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by:   Donald Sharp <sharpd@cumulusnetworks.com>
Ticket: CM-10948

The previous formula for calculating delay was:

    delay = OSPF_LS_REFRESH_TIME - LS_AGE (lsa) - OSPF_LS_REFRESH_JITTER
              + (random () % (2*OSPF_LS_REFRESH_JITTER));

If LS_AGE() is 0 we get:

  delay = 1800 - 0 - 60 + (random() % 2*60)

So delay will be 1740 + 0..120, so we could easily pick a delay that is
greater than OSPF_LS_REFRESH_TIME (1800).  For the normal case where
we hit this else the LS_AGE will be > 0 so that reduces the chances of
delay being greater than 1800.

diff --git a/ospfd/ospf_lsa.c b/ospfd/ospf_lsa.c
index 1bf3e9e97aefa6b5c85b901ed6cc107978852c3f..aa4d7ab165e89d1fd8e2bb702ab5690a0e0bb0c1 100644 (file)
--- a/ospfd/ospf_lsa.c
+++ b/ospfd/ospf_lsa.c
@@ -3729,18 +3729,14 @@ ospf_refresher_register_lsa (struct ospf *ospf, struct ospf_lsa *lsa)
   if (lsa->refresh_list < 0)
     {
       int delay;
+      int min_delay = OSPF_LS_REFRESH_TIME - (2 * OSPF_LS_REFRESH_JITTER);
+      int max_delay = OSPF_LS_REFRESH_TIME - OSPF_LS_REFRESH_JITTER;
 
-      if (LS_AGE (lsa) == 0 &&
-         ntohl (lsa->data->ls_seqnum) == OSPF_INITIAL_SEQUENCE_NUMBER)
-       /* Randomize first update by  OSPF_LS_REFRESH_SHIFT factor */ 
-       delay = OSPF_LS_REFRESH_SHIFT + (random () % OSPF_LS_REFRESH_TIME);
-      else
-       /* Randomize another updates by +-OSPF_LS_REFRESH_JITTER factor */
-       delay = OSPF_LS_REFRESH_TIME - LS_AGE (lsa) - OSPF_LS_REFRESH_JITTER
-         + (random () % (2*OSPF_LS_REFRESH_JITTER)); 
-
-      if (delay < 0)
-       delay = 0;
+      /* We want to refresh the LSA within OSPF_LS_REFRESH_TIME which is
+       * 1800s. Use jitter so that we send the LSA sometime between 1680s
+       * and 1740s.
+       */
+      delay = (random() % (max_delay - min_delay)) + min_delay;
 
       current_index = ospf->lsa_refresh_queue.index + (quagga_time (NULL)
                 - ospf->lsa_refresher_started)/OSPF_LSA_REFRESHER_GRANULARITY;
@@ -3749,17 +3745,20 @@ ospf_refresher_register_lsa (struct ospf *ospf, struct ospf_lsa *lsa)
              % (OSPF_LSA_REFRESHER_SLOTS);
 
       if (IS_DEBUG_OSPF (lsa, LSA_REFRESH))
-       zlog_debug ("LSA[Refresh]: lsa %s with age %d added to index %d",
-                  inet_ntoa (lsa->data->id), LS_AGE (lsa), index);
+       zlog_debug ("LSA[Refresh:Type%d:%s]: age %d, added to index %d",
+                   lsa->data->type, inet_ntoa (lsa->data->id), LS_AGE (lsa), index);
+
       if (!ospf->lsa_refresh_queue.qs[index])
        ospf->lsa_refresh_queue.qs[index] = list_new ();
+
       listnode_add (ospf->lsa_refresh_queue.qs[index],
                     ospf_lsa_lock (lsa)); /* lsa_refresh_queue */
       lsa->refresh_list = index;
+
       if (IS_DEBUG_OSPF (lsa, LSA_REFRESH))
-        zlog_debug ("LSA[Refresh:%s]: ospf_refresher_register_lsa(): "
-                   "setting refresh_list on lsa %p (slod %d)", 
-                   inet_ntoa (lsa->data->id), lsa, index);
+        zlog_debug ("LSA[Refresh:Type%d:%s]: ospf_refresher_register_lsa() "
+                   "setting refresh_list on lsa %p (slot %d)",
+                   lsa->data->type, inet_ntoa (lsa->data->id), lsa, index);
     }
 }
 
@@ -3793,7 +3792,7 @@ ospf_lsa_refresh_walker (struct thread *t)
   struct list *lsa_to_refresh = list_new ();
 
   if (IS_DEBUG_OSPF (lsa, LSA_REFRESH))
-    zlog_debug ("LSA[Refresh]:ospf_lsa_refresh_walker(): start");
+    zlog_debug ("LSA[Refresh]: ospf_lsa_refresh_walker(): start");
 
   
   i = ospf->lsa_refresh_queue.index;
@@ -3829,9 +3828,9 @@ ospf_lsa_refresh_walker (struct thread *t)
          for (ALL_LIST_ELEMENTS (refresh_list, node, nnode, lsa))
            {
              if (IS_DEBUG_OSPF (lsa, LSA_REFRESH))
-               zlog_debug ("LSA[Refresh:%s]: ospf_lsa_refresh_walker(): "
+               zlog_debug ("LSA[Refresh:Type%d:%s]: ospf_lsa_refresh_walker(): "
                           "refresh lsa %p (slot %d)", 
-                          inet_ntoa (lsa->data->id), lsa, i);
+                          lsa->data->type, inet_ntoa (lsa->data->id), lsa, i);
              
              assert (lsa->lock > 0);
              list_delete_node (refresh_list, node);

diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c
index 69a76dd222fe17a316d091691829be9a40811bb3..75300fa351ceb553f263cceddc1b79ba69f9ea90 100644 (file)
--- a/ospfd/ospf_vty.c
+++ b/ospfd/ospf_vty.c
@@ -2838,7 +2838,7 @@ DEFUN (ospf_refresh_timer, ospf_refresh_timer_cmd,
     return CMD_SUCCESS;
 
   VTY_GET_INTEGER_RANGE ("refresh timer", interval, argv[0], 10, 1800);
-  interval = (interval / 10) * 10;
+  interval = (interval / OSPF_LSA_REFRESHER_GRANULARITY) * OSPF_LSA_REFRESHER_GRANULARITY;
 
   ospf_timers_refresh_set (ospf, interval);
 

diff --git a/ospfd/ospfd.h b/ospfd/ospfd.h
index 93610b12185985eabb56b74de6f4e1f8ccfeccc1..4c140cf5bcd4fe80c06ab068814c953776a40d2e 100644 (file)
--- a/ospfd/ospfd.h
+++ b/ospfd/ospfd.h
@@ -284,7 +284,7 @@ struct ospf
 
 #define OSPF_LSA_REFRESHER_GRANULARITY 10
 #define OSPF_LSA_REFRESHER_SLOTS ((OSPF_LS_REFRESH_TIME + \
-                                  OSPF_LS_REFRESH_SHIFT)/10 + 1)
+                                  OSPF_LS_REFRESH_SHIFT)/OSPF_LSA_REFRESHER_GRANULARITY + 1)
   struct
   {
     u_int16_t index;