2005-05-11 Paul Jakma <paul.jakma@sun.com>

	* (general) Fix memory leaks in opaque AS-scope LSAs, reported and
	  with much debugging done by by scott collins <scollins@agile.tv>.
	  (possible backport candidate?)
	* ospf_lsa.c: (ospf_discard_from_db) dont call
	  ospf_ase_unregister_external_lsa for opaque-lsa's, opaques are
	  never registered with ase in the first place.
	* ospf_packet.c: (general) Disabuse opaque related code of its
          tendency to try gather up things into temporary lists.
          (ospf_ls_upd) remove the temporary lists opaque uses, call
          opaque functions inline, just like all other types.
          (ospf_ls_ack) ditto.
          (ospf_recv_packet) fixup sign warning.
        * ospf_opaque.c: (general) fix the unneeded use of lists, and
          untwist some of the logic.
          (ospf_opaque_self_originated_lsa_received) take a single LSA
          as argument, not a list of them. Remove the list loop. Logic
          otherwise unchanged.
          (ospf_opaque_ls_ack_received) Mostly ditto. But untwist the logic,
          move the actions up into the switch block, remove the goto's and
          sanitise the logic near the end a bit.
        * ospf_opaque.h: Adjust definitions of aforementioned functions
          in ospf_opaque.c to match.

5 files changed, 111 insertions, 147 deletions

diff --git a/ospfd/ChangeLog b/ospfd/ChangeLog
index 209027e7..774658ce 100644
--- a/ospfd/ChangeLog
+++ b/ospfd/ChangeLog
@@ -1,3 +1,27 @@
+2005-05-11 Paul Jakma <paul.jakma@sun.com>
+
+	* (general) Fix memory leaks in opaque AS-scope LSAs, reported and
+	  with much debugging done by by scott collins <scollins@agile.tv>.
+	* ospf_lsa.c: (ospf_discard_from_db) dont call
+	  ospf_ase_unregister_external_lsa for opaque-lsa's, opaques are
+	  never registered with ase in the first place.
+	* ospf_packet.c: (general) Disabuse opaque related code of its
+          tendency to try gather up things into temporary lists.
+          (ospf_ls_upd) remove the temporary lists opaque uses, call
+          opaque functions inline, just like all other types.
+          (ospf_ls_ack) ditto.
+          (ospf_recv_packet) fixup sign warning.
+        * ospf_opaque.c: (general) fix the unneeded use of lists, and
+          untwist some of the logic.
+          (ospf_opaque_self_originated_lsa_received) take a single LSA
+          as argument, not a list of them. Remove the list loop. Logic 
+          otherwise unchanged.
+          (ospf_opaque_ls_ack_received) Mostly ditto. But untwist the logic,
+          move the actions up into the switch block, remove the goto's and
+          sanitise the logic near the end a bit.
+        * ospf_opaque.h: Adjust definitions of aforementioned functions
+          in ospf_opaque.c to match.
+         
 2005-05-07 Yar Tikhiy <yar@comp.chem.msu.su>
 
 	* ospf_network.c: Log ifindex on multicast membership leave/join
diff --git a/ospfd/ospf_lsa.c b/ospfd/ospf_lsa.c
index e6c7fdc3..708fa1c5 100644
--- a/ospfd/ospf_lsa.c
+++ b/ospfd/ospf_lsa.c
@@ -2523,12 +2523,14 @@ ospf_discard_from_db (struct ospf *ospf,
   switch (old->data->type)
     {
     case OSPF_AS_EXTERNAL_LSA:
+      ospf_ase_unregister_external_lsa (old, ospf);
+      ospf_ls_retransmit_delete_nbr_as (ospf, old);
+      break;
 #ifdef HAVE_OPAQUE_LSA
     case OSPF_OPAQUE_AS_LSA:
-#endif /* HAVE_OPAQUE_LSA */
       ospf_ls_retransmit_delete_nbr_as (ospf, old);
-      ospf_ase_unregister_external_lsa (old, ospf);
       break;
+#endif /* HAVE_OPAQUE_LSA */
     case OSPF_AS_NSSA_LSA:
       ospf_ls_retransmit_delete_nbr_area (old->area, old);
       ospf_ase_unregister_external_lsa (old, ospf);
diff --git a/ospfd/ospf_opaque.c b/ospfd/ospf_opaque.c
index 6cc09878..8eca9eea 100644
--- a/ospfd/ospf_opaque.c
+++ b/ospfd/ospf_opaque.c
@@ -2234,11 +2234,9 @@ ospf_opaque_exclude_lsa_from_lsreq (struct route_table *nbrs,
 
 void
 ospf_opaque_self_originated_lsa_received (struct ospf_neighbor *nbr, 
-                                          struct list *lsas)
+                                          struct ospf_lsa *lsa)
 {
   struct ospf *top;
-  struct listnode *node, *next;
-  struct ospf_lsa *lsa;
   u_char before;
 
   if ((top = oi_to_top (nbr->oi)) == NULL)
@@ -2246,37 +2244,32 @@ ospf_opaque_self_originated_lsa_received (struct ospf_neighbor *nbr,
 
   before = IS_OPAQUE_LSA_ORIGINATION_BLOCKED (top->opaque);
 
-  for (ALL_LIST_ELEMENTS (lsas, node, next, lsa))
+  /*
+   * Since these LSA entries are not yet installed into corresponding
+   * LSDB, just flush them without calling ospf_ls_maxage() afterward.
+   */
+  lsa->data->ls_age = htons (OSPF_LSA_MAXAGE);
+  switch (lsa->data->type)
     {
-      listnode_delete (lsas, lsa);
-
-      /*
-       * Since these LSA entries are not yet installed into corresponding
-       * LSDB, just flush them without calling ospf_ls_maxage() afterward.
-       */
-      lsa->data->ls_age = htons (OSPF_LSA_MAXAGE);
-      switch (lsa->data->type)
-        {
-        case OSPF_OPAQUE_LINK_LSA:
-          SET_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_09_LSA_BIT);
-          ospf_flood_through_area (nbr->oi->area, NULL/*inbr*/, lsa);
-          break;
-        case OSPF_OPAQUE_AREA_LSA:
-          SET_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_10_LSA_BIT);
-          ospf_flood_through_area (nbr->oi->area, NULL/*inbr*/, lsa);
-          break;
-        case OSPF_OPAQUE_AS_LSA:
-          SET_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_11_LSA_BIT);
-          ospf_flood_through_as (top, NULL/*inbr*/, lsa);
-          break;
-        default:
-          zlog_warn ("ospf_opaque_self_originated_lsa_received: Unexpected LSA-type(%u)", lsa->data->type);
-          goto out;
-        }
-
-      ospf_lsa_discard (lsa); /* List "lsas" will be deleted by caller. */
+    case OSPF_OPAQUE_LINK_LSA:
+      SET_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_09_LSA_BIT);
+      ospf_flood_through_area (nbr->oi->area, NULL/*inbr*/, lsa);
+      break;
+    case OSPF_OPAQUE_AREA_LSA:
+      SET_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_10_LSA_BIT);
+      ospf_flood_through_area (nbr->oi->area, NULL/*inbr*/, lsa);
+      break;
+    case OSPF_OPAQUE_AS_LSA:
+      SET_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_11_LSA_BIT);
+      ospf_flood_through_as (top, NULL/*inbr*/, lsa);
+      break;
+    default:
+      zlog_warn ("ospf_opaque_self_originated_lsa_received: Unexpected LSA-type(%u)", lsa->data->type);
+      goto out;
     }
 
+  ospf_lsa_discard (lsa); /* List "lsas" will be deleted by caller. */
+
   if (before == 0 && IS_OPAQUE_LSA_ORIGINATION_BLOCKED (top->opaque))
     {
       if (IS_DEBUG_OSPF_EVENT)
@@ -2288,78 +2281,63 @@ out:
 }
 
 void
-ospf_opaque_ls_ack_received (struct ospf_neighbor *nbr, struct list *acks)
+ospf_opaque_ls_ack_received (struct ospf_neighbor *nbr, struct ospf_lsa *lsa)
 {
   struct ospf *top;
+  int delay;
+  struct ospf_interface *oi;
   struct listnode *node, *nnode;
-  struct ospf_lsa *lsa;
-  char type9_lsa_rcv = 0, type10_lsa_rcv = 0, type11_lsa_rcv = 0;
 
   if ((top = oi_to_top (nbr->oi)) == NULL)
-    goto out;
-
-  for (ALL_LIST_ELEMENTS (acks, node, nnode, lsa))
-    {
-      switch (lsa->data->type)
-        {
-        case OSPF_OPAQUE_LINK_LSA:
-          type9_lsa_rcv = 1;
-          /* Callback function... */
-          break;
-        case OSPF_OPAQUE_AREA_LSA:
-          type10_lsa_rcv = 1;
-          /* Callback function... */
-          break;
-        case OSPF_OPAQUE_AS_LSA:
-          type11_lsa_rcv = 1;
-          /* Callback function... */
-          break;
-        default:
-          zlog_warn ("ospf_opaque_ls_ack_received: Unexpected LSA-type(%u)", lsa->data->type);
-          goto out;
-        }
-    }
-
-  if (IS_OPAQUE_LSA_ORIGINATION_BLOCKED (top->opaque))
+    return;
+  
+  if (!IS_OPAQUE_LSA_ORIGINATION_BLOCKED (top->opaque))
+    return;
+  
+  switch (lsa->data->type)
     {
-      int delay;
-      struct ospf_interface *oi;
-
-      if (type9_lsa_rcv
-      &&  CHECK_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_09_LSA_BIT))
+    case OSPF_OPAQUE_LINK_LSA:
+      if (CHECK_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_09_LSA_BIT))
         ospf_opaque_type9_lsa_rxmt_nbr_check (nbr->oi);
-
-      if (type10_lsa_rcv
-      &&  CHECK_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_10_LSA_BIT))
+      /* Callback function... */
+      break;
+    case OSPF_OPAQUE_AREA_LSA:
+      if (CHECK_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_10_LSA_BIT))
         ospf_opaque_type10_lsa_rxmt_nbr_check (nbr->oi->area);
-
-      if (type11_lsa_rcv
-      &&  CHECK_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_11_LSA_BIT))
+      /* Callback function... */
+      break;
+    case OSPF_OPAQUE_AS_LSA:
+      if (CHECK_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_11_LSA_BIT))
         ospf_opaque_type11_lsa_rxmt_nbr_check (top);
-
-      if (IS_OPAQUE_LSA_ORIGINATION_BLOCKED (top->opaque))
-        goto out; /* Blocking still in progress. */
-
+      /* Callback function... */
+      break;
+    default:
+      zlog_warn ("ospf_opaque_ls_ack_received: Unexpected LSA-type(%u)", lsa->data->type);
+      return;
+    }
+  
+  if (IS_OPAQUE_LSA_ORIGINATION_BLOCKED (top->opaque))
+    {
       if (IS_DEBUG_OSPF_EVENT)
         zlog_debug ("Block Opaque-LSA origination: ON -> OFF");
+      return; /* Blocking still in progress. */
+    }
+  
+  if (! CHECK_FLAG (top->config, OSPF_OPAQUE_CAPABLE))
+    return; /* Opaque capability condition must have changed. */
 
-      if (! CHECK_FLAG (top->config, OSPF_OPAQUE_CAPABLE))
-        goto out; /* Opaque capability condition must have changed. */
-
-      /* Ok, let's start origination of Opaque-LSAs. */
-      delay = OSPF_MIN_LS_INTERVAL;
+  /* Ok, let's start origination of Opaque-LSAs. */
+  delay = OSPF_MIN_LS_INTERVAL;
 
-      for (ALL_LIST_ELEMENTS (top->oiflist, node, nnode, oi))
-        {
-          if (! ospf_if_is_enable (oi)
-              || ospf_nbr_count_opaque_capable (oi) == 0)
-            continue;
+  for (ALL_LIST_ELEMENTS (top->oiflist, node, nnode, oi))
+    {
+      if (! ospf_if_is_enable (oi)
+          || ospf_nbr_count_opaque_capable (oi) == 0)
+        continue;
 
-          ospf_opaque_lsa_originate_schedule (oi, &delay);
-        }
+      ospf_opaque_lsa_originate_schedule (oi, &delay);
     }
-
-out:
+    
   return;
 }
 
diff --git a/ospfd/ospf_opaque.h b/ospfd/ospf_opaque.h
index e33fb65b..fc8d6ff6 100644
--- a/ospfd/ospf_opaque.h
+++ b/ospfd/ospf_opaque.h
@@ -155,9 +155,9 @@ extern void ospf_opaque_adjust_lsreq (struct ospf_neighbor *nbr,
 				      struct list *lsas);
 extern void ospf_opaque_self_originated_lsa_received (struct ospf_neighbor
 						      *nbr,
-						      struct list *lsas);
+						      struct ospf_lsa *lsa);
 extern void ospf_opaque_ls_ack_received (struct ospf_neighbor *nbr,
-					 struct list *acks);
+					 struct ospf_lsa *lsa);
 
 extern void htonf (float *src, float *dst);
 extern void ntohf (float *src, float *dst);
diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index e223b56d..1906cc1c 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -48,9 +48,6 @@
 #include "ospfd/ospf_flood.h"
 #include "ospfd/ospf_dump.h"
 
-static void ospf_ls_ack_send_list (struct ospf_interface *, struct list *,
-				   struct in_addr);
-
 /* Packet Type String. */
 const char *ospf_packet_type_str[] =
 {
@@ -1595,9 +1592,6 @@ ospf_ls_upd (struct ip *iph, struct ospf_header *ospfh,
 {
   struct ospf_neighbor *nbr;
   struct list *lsas;
-#ifdef HAVE_OPAQUE_LSA
-  struct list *mylsa_acks, *mylsa_upds;
-#endif /* HAVE_OPAQUE_LSA */
   struct listnode *node, *nnode;
   struct ospf_lsa *lsa = NULL;
   /* unsigned long ls_req_found = 0; */
@@ -1634,13 +1628,6 @@ ospf_ls_upd (struct ip *iph, struct ospf_header *ospfh,
 
 #ifdef HAVE_OPAQUE_LSA
   /*
-   * Prepare two kinds of lists to clean up unwanted self-originated
-   * Opaque-LSAs from the routing domain as soon as possible.
-   */
-  mylsa_acks = list_new (); /* Let the sender cease retransmission. */
-  mylsa_upds = list_new (); /* Flush target LSAs if necessary. */
-
-  /*
    * If self-originated Opaque-LSAs that have flooded before restart
    * are contained in the received LSUpd message, corresponding LSReq
    * messages to be sent may have to be modified.
@@ -1648,6 +1635,9 @@ ospf_ls_upd (struct ip *iph, struct ospf_header *ospfh,
    * updating for the same LSA would take place alternately, this trick
    * must be done before entering to the loop below.
    */
+   /* XXX: Why is this Opaque specific? Either our core code is deficient
+    * and this should be fixed generally, or Opaque is inventing strawman
+    * problems */
    ospf_opaque_adjust_lsreq (nbr, lsas);
 #endif /* HAVE_OPAQUE_LSA */
 
@@ -1768,18 +1758,23 @@ ospf_ls_upd (struct ip *iph, struct ospf_header *ospfh,
            * Otherwise, the LSA instance remains in the routing domain
            * until its age reaches to MaxAge.
            */
+          /* XXX: We should deal with this for *ALL* LSAs, not just opaque */
           if (current == NULL)
             {
               if (IS_DEBUG_OSPF_EVENT)
-                zlog_debug ("LSA[%s]: Previously originated Opaque-LSA, not found in the LSDB.", dump_lsa_key (lsa));
+                zlog_debug ("LSA[%s]: Previously originated Opaque-LSA,"
+                            "not found in the LSDB.", dump_lsa_key (lsa));
 
               SET_FLAG (lsa->flags, OSPF_LSA_SELF);
-              listnode_add (mylsa_upds, ospf_lsa_dup  (lsa));
-              listnode_add (mylsa_acks, ospf_lsa_lock (lsa));
+              
+              ospf_opaque_self_originated_lsa_received (nbr, lsa);
+              ospf_ls_ack_send (nbr, lsa);
+              
               continue;
             }
         }
 #endif /* HAVE_OPAQUE_LSA */
+
       /* It might be happen that received LSA is self-originated network LSA, but
        * router ID is cahnged. So, we should check if LSA is a network-LSA whose
        * Link State ID is one of the router's own IP interface addresses but whose
@@ -1848,10 +1843,6 @@ ospf_ls_upd (struct ip *iph, struct ospf_header *ospfh,
           ospf_upd_list_clean (lsas);
 	  /* this lsa is not on lsas list already. */
 	  ospf_lsa_discard (lsa);
-#ifdef HAVE_OPAQUE_LSA
-          list_delete (mylsa_acks);
-          list_delete (mylsa_upds);
-#endif /* HAVE_OPAQUE_LSA */
 	  return;
 	}
 
@@ -1929,23 +1920,6 @@ ospf_ls_upd (struct ip *iph, struct ospf_header *ospfh,
 	}
     }
   
-#ifdef HAVE_OPAQUE_LSA
-  /*
-   * Now that previously originated Opaque-LSAs those which not yet
-   * installed into LSDB are captured, take several steps to clear
-   * them completely from the routing domain, before proceeding to
-   * origination for the current target Opaque-LSAs.
-   */
-  while (listcount (mylsa_acks) > 0)
-    ospf_ls_ack_send_list (oi, mylsa_acks, nbr->address.u.prefix4);
-
-  if (listcount (mylsa_upds) > 0)
-    ospf_opaque_self_originated_lsa_received (nbr, mylsa_upds);
-
-  list_delete (mylsa_upds);
-  list_delete (mylsa_acks);
-#endif /* HAVE_OPAQUE_LSA */
-
   assert (listcount (lsas) == 0);
   list_delete (lsas);
 }
@@ -1956,10 +1930,7 @@ ospf_ls_ack (struct ip *iph, struct ospf_header *ospfh,
 	     struct stream *s, struct ospf_interface *oi, u_int16_t size)
 {
   struct ospf_neighbor *nbr;
-#ifdef HAVE_OPAQUE_LSA
-  struct list *opaque_acks;
-#endif /* HAVE_OPAQUE_LSA */
-
+  
   /* increment statistics. */
   oi->ls_ack_in++;
 
@@ -1979,11 +1950,7 @@ ospf_ls_ack (struct ip *iph, struct ospf_header *ospfh,
 		 LOOKUP(ospf_nsm_state_msg, nbr->state));
       return;
     }
-
-#ifdef HAVE_OPAQUE_LSA
-  opaque_acks = list_new ();
-#endif /* HAVE_OPAQUE_LSA */
-
+  
   while (size >= OSPF_LSA_HEADER_SIZE)
     {
       struct ospf_lsa *lsa, *lsr;
@@ -2007,9 +1974,8 @@ ospf_ls_ack (struct ip *iph, struct ospf_header *ospfh,
       if (lsr != NULL && lsr->data->ls_seqnum == lsa->data->ls_seqnum)
         {
 #ifdef HAVE_OPAQUE_LSA
-          /* Keep this LSA entry for later reference. */
           if (IS_OPAQUE_LSA (lsr->data->type))
-            listnode_add (opaque_acks, ospf_lsa_dup (lsr));
+            ospf_opaque_ls_ack_received (nbr, lsr);
 #endif /* HAVE_OPAQUE_LSA */
 
           ospf_ls_retransmit_delete (nbr, lsr);
@@ -2019,13 +1985,7 @@ ospf_ls_ack (struct ip *iph, struct ospf_header *ospfh,
       ospf_lsa_discard (lsa);
     }
 
-#ifdef HAVE_OPAQUE_LSA
-  if (listcount (opaque_acks) > 0)
-    ospf_opaque_ls_ack_received (nbr, opaque_acks);
-
-  list_delete (opaque_acks);
   return;
-#endif /* HAVE_OPAQUE_LSA */
 }
 
 static struct stream *
@@ -2052,7 +2012,7 @@ ospf_recv_packet (int fd, struct interface **ifp, struct stream *ibuf)
       zlog_warn("stream_recvmsg failed: %s", safe_strerror(errno));
       return NULL;
     }
-  if (ret < sizeof(iph))
+  if ((unsigned int)ret < sizeof(iph)) /* ret must be > 0 now */
     {
       zlog_warn("ospf_recv_packet: discarding runt packet of length %d "
 		"(ip header size is %u)",