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.
diff --git a/ospfd/ChangeLog b/ospfd/ChangeLog
index 209027e..774658c 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 e6c7fdc..708fa1c 100644
--- a/ospfd/ospf_lsa.c
+++ b/ospfd/ospf_lsa.c
@@ -2523,12 +2523,14 @@
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 6cc0987..8eca9ee 100644
--- a/ospfd/ospf_opaque.c
+++ b/ospfd/ospf_opaque.c
@@ -2234,11 +2234,9 @@
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 @@
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 @@
}
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))
+ return;
+
+ if (!IS_OPAQUE_LSA_ORIGINATION_BLOCKED (top->opaque))
+ return;
+
+ switch (lsa->data->type)
{
- 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;
- }
+ 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);
+ /* 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);
+ /* 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);
+ /* 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))
{
- int delay;
- struct ospf_interface *oi;
-
- if (type9_lsa_rcv
- && 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))
- ospf_opaque_type10_lsa_rxmt_nbr_check (nbr->oi->area);
-
- if (type11_lsa_rcv
- && 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. */
-
if (IS_DEBUG_OSPF_EVENT)
zlog_debug ("Block Opaque-LSA origination: ON -> OFF");
-
- 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;
-
- 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);
- }
+ return; /* Blocking still in progress. */
}
+
+ if (! CHECK_FLAG (top->config, OSPF_OPAQUE_CAPABLE))
+ return; /* Opaque capability condition must have changed. */
-out:
+ /* 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;
+
+ ospf_opaque_lsa_originate_schedule (oi, &delay);
+ }
+
return;
}
diff --git a/ospfd/ospf_opaque.h b/ospfd/ospf_opaque.h
index e33fb65..fc8d6ff 100644
--- a/ospfd/ospf_opaque.h
+++ b/ospfd/ospf_opaque.h
@@ -155,9 +155,9 @@
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 e223b56..1906cc1 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 @@
{
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 @@
#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 @@
* 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 @@
* 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_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 @@
}
}
-#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 @@
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 @@
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 @@
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_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 @@
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)",