From: Daniel Walton <dwalton@cumulusnetworks.com>
Date: Wed, 18 May 2016 13:36:38 +0000 (+0000)
Subject: OSPF LSA may not be refreshed within 1800s, ANVL OSPF test 12.4 fails
X-Git-Tag: frr-2.0-rc1~908
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=535c84b16b2c82ad8e50b40c0a6afd855d193de3;p=mirror%2Ffrr.git

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 1bf3e9e97a..aa4d7ab165 100644
--- 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 69a76dd222..75300fa351 100644
--- 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 93610b1218..4c140cf5bc 100644
--- 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;