diff options
| -rw-r--r-- | ospfd/ChangeLog | 24 | ||||
| -rw-r--r-- | ospfd/ospf_lsa.c | 6 | ||||
| -rw-r--r-- | ospfd/ospf_opaque.c | 154 | ||||
| -rw-r--r-- | ospfd/ospf_opaque.h | 4 | ||||
| -rw-r--r-- | ospfd/ospf_packet.c | 70 |
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)", |