ospf6d: fix neighbor state machine (faster lsdb sync, RFC compliance)

The OSPFv3 code doesn't do the following things right as part of an adjacency
bringup:
 - Transmit DbDesc frames appropriately to ensure faster state transition to
   Loading state
 - Transmit LsReq frames when switching to exchange state and on receipt of
   an LS update in Loading state
 - Requesting LSAs multiple times in LsReq.

It currently uses retransmit timer expiry to send the LsReq and DbDesc frames
which significantly slows down large lsdb syncs.

Signed-off-by: Dinesh G Dutt <ddutt at cumulusnetworks.com>
Reviewed-by: Scott Feldman <sfeldma at cumulusnetworks.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

4 files changed, 88 insertions, 44 deletions

diff --git a/ospf6d/ospf6_flood.c b/ospf6d/ospf6_flood.c
index beae69927..e02a432f9 100644
--- a/ospf6d/ospf6_flood.c
+++ b/ospf6d/ospf6_flood.c
@@ -206,8 +206,8 @@ ospf6_decrement_retrans_count (struct ospf6_lsa *lsa)
 void
 ospf6_install_lsa (struct ospf6_lsa *lsa)
 {
-  struct ospf6_lsa *old;
   struct timeval now;
+  struct ospf6_lsa *old;
 
   if (IS_OSPF6_DEBUG_LSA_TYPE (lsa->header->type) ||
       IS_OSPF6_DEBUG_EXAMIN_TYPE (lsa->header->type))
@@ -290,7 +290,7 @@ ospf6_flood_interface (struct ospf6_neighbor *from,
               if (ospf6_lsa_compare (lsa, req) > 0)
                 {
                   if (is_debug)
-                    zlog_debug ("Requesting is newer, next neighbor");
+                    zlog_debug ("Requesting is older, next neighbor");
                   continue;
                 }
 
@@ -298,18 +298,30 @@ ospf6_flood_interface (struct ospf6_neighbor *from,
                  examin next neighbor */
               if (ospf6_lsa_compare (lsa, req) == 0)
                 {
-                  if (is_debug)
-                    zlog_debug ("Requesting the same, remove it, next neighbor");
+		  if (is_debug)
+		    zlog_debug ("Requesting the same, remove it, next neighbor");
+		  if (req == on->last_ls_req)
+		    {
+		      ospf6_lsa_unlock (req);
+		      on->last_ls_req = NULL;
+		    }
                   ospf6_lsdb_remove (req, on->request_list);
+		  ospf6_check_nbr_loading (on);
                   continue;
                 }
 
               /* If the new LSA is more recent, delete from request-list */
               if (ospf6_lsa_compare (lsa, req) < 0)
                 {
-                  if (is_debug)
-                    zlog_debug ("Received is newer, remove requesting");
+		  if (is_debug)
+		    zlog_debug ("Received is newer, remove requesting");
+		  if (req == on->last_ls_req)
+		    {
+		      ospf6_lsa_unlock (req);
+		      on->last_ls_req = NULL;
+		    }
                   ospf6_lsdb_remove (req, on->request_list);
+		  ospf6_check_nbr_loading (on);
                   /* fall through */
                 }
             }
@@ -796,7 +808,7 @@ ospf6_receive_lsa (struct ospf6_neighbor *from,
     {
       /* log */
       if (is_debug)
-        zlog_debug ("Drop MaxAge LSA with direct acknowledgement.");
+	zlog_debug ("Drop MaxAge LSA with direct acknowledgement.");
 
       /* a) Acknowledge back to neighbor (Direct acknowledgement, 13.5) */
       ospf6_lsdb_add (ospf6_lsa_copy (new), from->lsack_list);
@@ -950,8 +962,8 @@ ospf6_receive_lsa (struct ospf6_neighbor *from,
               zlog_debug ("The LSA is in Seqnumber Wrapping");
               zlog_debug ("MaxAge & MaxSeqNum, discard");
             }
-          ospf6_lsa_delete (new);
-          return;
+	  ospf6_lsa_delete (new);
+	  return;
         }
 
       /* Otherwise, Send database copy of this LSA to this neighbor */
@@ -968,8 +980,8 @@ ospf6_receive_lsa (struct ospf6_neighbor *from,
           if (from->thread_send_lsupdate == NULL)
             from->thread_send_lsupdate =
               thread_add_event (master, ospf6_lsupdate_send_neighbor, from, 0);
-          ospf6_lsa_delete (new);
-          return;
+	  ospf6_lsa_delete (new);
+	  return;
         }
       return;
     }
diff --git a/ospf6d/ospf6_message.c b/ospf6d/ospf6_message.c
index 31db9a4ba..dcbb36bf5 100644
--- a/ospf6d/ospf6_message.c
+++ b/ospf6d/ospf6_message.c
@@ -517,20 +517,20 @@ ospf6_dbdesc_recv_master (struct ospf6_header *oh,
         {
           if (IS_OSPF6_DEBUG_MESSAGE (oh->type, RECV))
             zlog_debug ("Add request (No database copy)");
-          ospf6_lsdb_add (his, on->request_list);
+          ospf6_lsdb_add (ospf6_lsa_copy(his), on->request_list);
         }
       else if (ospf6_lsa_compare (his, mine) < 0)
         {
           if (IS_OSPF6_DEBUG_MESSAGE (oh->type, RECV))
             zlog_debug ("Add request (Received MoreRecent)");
-          ospf6_lsdb_add (his, on->request_list);
+          ospf6_lsdb_add (ospf6_lsa_copy(his), on->request_list);
         }
       else
         {
           if (IS_OSPF6_DEBUG_MESSAGE (oh->type, RECV))
             zlog_debug ("Discard (Existing MoreRecent)");
-          ospf6_lsa_delete (his);
         }
+      ospf6_lsa_delete (his);
     }
 
   assert (p == OSPF6_MESSAGE_END (oh));
@@ -539,7 +539,7 @@ ospf6_dbdesc_recv_master (struct ospf6_header *oh,
   on->dbdesc_seqnum ++;
 
   /* schedule send lsreq */
-  if (on->thread_send_lsreq == NULL)
+  if (on->request_list->count && (on->thread_send_lsreq == NULL))
     on->thread_send_lsreq =
       thread_add_event (master, ospf6_lsreq_send, on, 0);
 
@@ -735,10 +735,9 @@ ospf6_dbdesc_recv_slave (struct ospf6_header *oh,
         {
           if (IS_OSPF6_DEBUG_MESSAGE (oh->type, RECV))
             zlog_debug ("Add request-list: %s", his->name);
-          ospf6_lsdb_add (his, on->request_list);
+          ospf6_lsdb_add (ospf6_lsa_copy(his), on->request_list);
         }
-      else
-        ospf6_lsa_delete (his);
+      ospf6_lsa_delete (his);
     }
 
   assert (p == OSPF6_MESSAGE_END (oh));
@@ -747,7 +746,8 @@ ospf6_dbdesc_recv_slave (struct ospf6_header *oh,
   on->dbdesc_seqnum = ntohl (dbdesc->seqnum);
 
   /* schedule send lsreq */
-  if (on->thread_send_lsreq == NULL)
+  if ((on->thread_send_lsreq == NULL) &&
+      (on->request_list->count))
     on->thread_send_lsreq =
       thread_add_event (master, ospf6_lsreq_send, on, 0);
 
@@ -1351,19 +1351,6 @@ ospf6_lsupdate_recv (struct in6_addr *src, struct in6_addr *dst,
 
   assert (p == OSPF6_MESSAGE_END (oh));
 
-  /* RFC2328 Section 10.9: When the neighbor responds to these requests
-     with the proper Link State Update packet(s), the Link state request
-     list is truncated and a new Link State Request packet is sent. */
-  /* send new Link State Request packet if this LS Update packet
-     can be recognized as a response to our previous LS Request */
-  if (! IN6_IS_ADDR_MULTICAST (dst) &&
-      (on->state == OSPF6_NEIGHBOR_EXCHANGE ||
-       on->state == OSPF6_NEIGHBOR_LOADING))
-    {
-      THREAD_OFF (on->thread_send_lsreq);
-      on->thread_send_lsreq =
-        thread_add_event (master, ospf6_lsreq_send, on, 0);
-    }
 }
 
 static void
@@ -1907,7 +1894,7 @@ ospf6_lsreq_send (struct thread *thread)
   struct ospf6_header *oh;
   struct ospf6_lsreq_entry *e;
   u_char *p;
-  struct ospf6_lsa *lsa;
+  struct ospf6_lsa *lsa, *last_req;
 
   on = (struct ospf6_neighbor *) THREAD_ARG (thread);
   on->thread_send_lsreq = (struct thread *) NULL;
@@ -1929,13 +1916,9 @@ ospf6_lsreq_send (struct thread *thread)
       return 0;
     }
 
-  /* set next thread */
-  on->thread_send_lsreq =
-    thread_add_timer (master, ospf6_lsreq_send, on,
-                      on->ospf6_if->rxmt_interval);
-
   memset (sendbuf, 0, iobuflen);
   oh = (struct ospf6_header *) sendbuf;
+  last_req = NULL;
 
   /* set Request entries in lsreq */
   p = (u_char *)((caddr_t) oh + sizeof (struct ospf6_header));
@@ -1954,6 +1937,17 @@ ospf6_lsreq_send (struct thread *thread)
       e->id = lsa->header->id;
       e->adv_router = lsa->header->adv_router;
       p += sizeof (struct ospf6_lsreq_entry);
+      last_req = lsa;
+    }
+
+  if (last_req != NULL)
+    {
+      if (on->last_ls_req != NULL)
+	{
+	  ospf6_lsa_unlock (on->last_ls_req);
+	}
+      ospf6_lsa_lock (last_req);
+      on->last_ls_req = last_req;
     }
 
   oh->type = OSPF6_MESSAGE_TYPE_LSREQ;
@@ -1966,6 +1960,14 @@ ospf6_lsreq_send (struct thread *thread)
     ospf6_send (on->ospf6_if->linklocal_addr, &on->linklocal_addr,
 		on->ospf6_if, oh);
 
+  /* set next thread */
+  if (on->request_list->count != 0)
+    {
+      on->thread_send_lsreq =
+	thread_add_timer (master, ospf6_lsreq_send, on,
+			  on->ospf6_if->rxmt_interval);
+    }
+
   return 0;
 }
 
diff --git a/ospf6d/ospf6_neighbor.c b/ospf6d/ospf6_neighbor.c
index 806767dd5..84f0b002c 100644
--- a/ospf6d/ospf6_neighbor.c
+++ b/ospf6d/ospf6_neighbor.c
@@ -98,7 +98,6 @@ ospf6_neighbor_create (u_int32_t router_id, struct ospf6_interface *oi)
   on->retrans_list = ospf6_lsdb_create (on);
 
   on->dbdesc_list = ospf6_lsdb_create (on);
-  on->lsreq_list = ospf6_lsdb_create (on);
   on->lsupdate_list = ospf6_lsdb_create (on);
   on->lsack_list = ospf6_lsdb_create (on);
 
@@ -121,7 +120,6 @@ ospf6_neighbor_delete (struct ospf6_neighbor *on)
     }
 
   ospf6_lsdb_remove_all (on->dbdesc_list);
-  ospf6_lsdb_remove_all (on->lsreq_list);
   ospf6_lsdb_remove_all (on->lsupdate_list);
   ospf6_lsdb_remove_all (on->lsack_list);
 
@@ -130,7 +128,6 @@ ospf6_neighbor_delete (struct ospf6_neighbor *on)
   ospf6_lsdb_delete (on->retrans_list);
 
   ospf6_lsdb_delete (on->dbdesc_list);
-  ospf6_lsdb_delete (on->lsreq_list);
   ospf6_lsdb_delete (on->lsupdate_list);
   ospf6_lsdb_delete (on->lsack_list);
 
@@ -360,11 +357,41 @@ exchange_done (struct thread *thread)
   if (on->request_list->count == 0)
     ospf6_neighbor_state_change (OSPF6_NEIGHBOR_FULL, on);
   else
-    ospf6_neighbor_state_change (OSPF6_NEIGHBOR_LOADING, on);
+    {
+      ospf6_neighbor_state_change (OSPF6_NEIGHBOR_LOADING, on);
+
+      if (on->thread_send_lsreq == NULL)
+	on->thread_send_lsreq =
+	  thread_add_event (master, ospf6_lsreq_send, on, 0);
+    }
 
   return 0;
 }
 
+/* Check loading state. */
+void
+ospf6_check_nbr_loading (struct ospf6_neighbor *on)
+{
+
+  /* RFC2328 Section 10.9: When the neighbor responds to these requests
+     with the proper Link State Update packet(s), the Link state request
+     list is truncated and a new Link State Request packet is sent.
+  */
+  if ((on->state == OSPF6_NEIGHBOR_LOADING) ||
+      (on->state == OSPF6_NEIGHBOR_EXCHANGE))
+    {
+      if (on->request_list->count == 0)
+	thread_add_event (master, loading_done, on, 0);
+      else if (on->last_ls_req == NULL)
+	{
+	  if (on->thread_send_lsreq != NULL)
+	    THREAD_OFF (on->thread_send_lsreq);
+	  on->thread_send_lsreq =
+	    thread_add_event (master, ospf6_lsreq_send, on, 0);
+	}
+    }
+}
+
 int
 loading_done (struct thread *thread)
 {
@@ -727,10 +754,10 @@ ospf6_neighbor_show_detail (struct vty *vty, struct ospf6_neighbor *on)
     timersub (&on->thread_send_lsreq->u.sands, &now, &res);
   timerstring (&res, duration, sizeof (duration));
   vty_out (vty, "    %d Pending LSAs for LSReq in Time %s [thread %s]%s",
-           on->lsreq_list->count, duration,
+           on->request_list->count, duration,
            (on->thread_send_lsreq ? "on" : "off"),
            VNL);
-  for (lsa = ospf6_lsdb_head (on->lsreq_list); lsa;
+  for (lsa = ospf6_lsdb_head (on->request_list); lsa;
        lsa = ospf6_lsdb_next (lsa))
     vty_out (vty, "      %s%s", lsa->name, VNL);
 
diff --git a/ospf6d/ospf6_neighbor.h b/ospf6d/ospf6_neighbor.h
index 5f46c6f38..750e1b242 100644
--- a/ospf6d/ospf6_neighbor.h
+++ b/ospf6d/ospf6_neighbor.h
@@ -86,6 +86,8 @@ struct ospf6_neighbor
   struct ospf6_lsdb *lsupdate_list;
   struct ospf6_lsdb *lsack_list;
 
+  struct ospf6_lsa *last_ls_req;
+
   /* Inactivity timer */
   struct thread *inactivity_timer;
 
@@ -130,6 +132,7 @@ extern int seqnumber_mismatch (struct thread *);
 extern int bad_lsreq (struct thread *);
 extern int oneway_received (struct thread *);
 extern int inactivity_timer (struct thread *);
+extern void ospf6_check_nbr_loading (struct ospf6_neighbor *);
 
 extern void ospf6_neighbor_init (void);
 extern int config_write_ospf6_debug_neighbor (struct vty *vty);