From f573ec607ce0ebcab1282edd4e35756f6d6e6de7 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 6 Nov 2019 17:12:51 -0500 Subject: [PATCH 1/6] ospfd: Remove ORIGINAL_CODING check We have a bunch of places that look for ORIGINAL_CODING. There is nothing in our configure system to define this value and a quick git blame shows this code as being original to the import a very very long time ago. This is dead code, removing. Signed-off-by: Donald Sharp --- ospfd/ospf_api.c | 16 -------------- ospfd/ospf_api.h | 6 ----- ospfd/ospf_dump.h | 5 ----- ospfd/ospf_flood.c | 54 ++++++++------------------------------------- ospfd/ospf_lsa.c | 40 --------------------------------- ospfd/ospf_packet.c | 4 ---- 6 files changed, 9 insertions(+), 116 deletions(-) diff --git a/ospfd/ospf_api.c b/ospfd/ospf_api.c index f06e45392e..1ace0977bc 100644 --- a/ospfd/ospf_api.c +++ b/ospfd/ospf_api.c @@ -246,13 +246,6 @@ void msg_print(struct msg *msg) return; } -#ifdef ORIGINAL_CODING - zlog_debug( - "msg=%p msgtype=%d msglen=%d msgseq=%d streamdata=%p streamsize=%lu\n", - msg, msg->hdr.msgtype, ntohs(msg->hdr.msglen), - ntohl(msg->hdr.msgseq), STREAM_DATA(msg->s), - STREAM_SIZE(msg->s)); -#else /* ORIGINAL_CODING */ /* API message common header part. */ zlog_debug("API-msg [%s]: type(%d),len(%d),seq(%lu),data(%p),size(%zd)", ospf_api_typename(msg->hdr.msgtype), msg->hdr.msgtype, @@ -260,16 +253,7 @@ void msg_print(struct msg *msg) (unsigned long)ntohl(msg->hdr.msgseq), STREAM_DATA(msg->s), STREAM_SIZE(msg->s)); -/* API message body part. */ -#ifdef ndef - /* Generic Hex/Ascii dump */ - DumpBuf(STREAM_DATA(msg->s), STREAM_SIZE(msg->s)); /* Sorry, deleted! */ -#else /* ndef */ -/* Message-type dependent dump function. */ -#endif /* ndef */ - return; -#endif /* ORIGINAL_CODING */ } void msg_free(struct msg *msg) diff --git a/ospfd/ospf_api.h b/ospfd/ospf_api.h index c99923e7b8..0fc683a5db 100644 --- a/ospfd/ospf_api.h +++ b/ospfd/ospf_api.h @@ -140,16 +140,10 @@ struct msg_unregister_opaque_type { * Power2[0] is not used. */ -#ifdef ORIGINAL_CODING -static const uint16_t Power2[] = {0x0, 0x1, 0x2, 0x4, 0x8, 0x10, - 0x20, 0x40, 0x80, 0x100, 0x200, 0x400, - 0x800, 0x1000, 0x2000, 0x4000, 0x8000}; -#else static const uint16_t Power2[] = { 0, (1 << 0), (1 << 1), (1 << 2), (1 << 3), (1 << 4), (1 << 5), (1 << 6), (1 << 7), (1 << 8), (1 << 9), (1 << 10), (1 << 11), (1 << 12), (1 << 13), (1 << 14), (1 << 15)}; -#endif /* ORIGINAL_CODING */ struct lsa_filter_type { uint16_t typemask; /* bitmask for selecting LSA types (1..16) */ diff --git a/ospfd/ospf_dump.h b/ospfd/ospf_dump.h index 397f666f69..09c1811c3c 100644 --- a/ospfd/ospf_dump.h +++ b/ospfd/ospf_dump.h @@ -108,11 +108,6 @@ (conf_debug_ospf_packet[a] & OSPF_DEBUG_##b) #define IS_CONF_DEBUG_OSPF(a, b) (conf_debug_ospf_##a & OSPF_DEBUG_##b) -#ifdef ORIGINAL_CODING -#else /* ORIGINAL_CODING */ -struct stream; -#endif /* ORIGINAL_CODING */ - #define AREA_NAME(A) ospf_area_name_string ((A)) #define IF_NAME(I) ospf_if_name_string ((I)) diff --git a/ospfd/ospf_flood.c b/ospfd/ospf_flood.c index 1d85a04984..a72caa8415 100644 --- a/ospfd/ospf_flood.c +++ b/ospfd/ospf_flood.c @@ -454,13 +454,8 @@ static int ospf_flood_through_interface(struct ospf_interface *oi, } } -/* If the new LSA was received from this neighbor, - examine the next neighbor. */ -#ifdef ORIGINAL_CODING - if (inbr) - if (IPV4_ADDR_SAME(&inbr->router_id, &onbr->router_id)) - continue; -#else /* ORIGINAL_CODING */ + /* If the new LSA was received from this neighbor, + examine the next neighbor. */ if (inbr) { /* * Triggered by LSUpd message parser "ospf_ls_upd ()". @@ -486,7 +481,6 @@ static int ospf_flood_through_interface(struct ospf_interface *oi, continue; } } -#endif /* ORIGINAL_CODING */ /* Add the new LSA to the Link state retransmission list for the adjacency. The LSA will be retransmitted @@ -691,43 +685,14 @@ int ospf_flood_through(struct ospf *ospf, struct ospf_neighbor *inbr, { int lsa_ack_flag = 0; -/* Type-7 LSA's for NSSA are flooded throughout the AS here, and - upon return are updated in the LSDB for Type-7's. Later, - re-fresh will re-send them (and also, if ABR, packet code will - translate to Type-5's) + /* Type-7 LSA's for NSSA are flooded throughout the AS here, and + upon return are updated in the LSDB for Type-7's. Later, + re-fresh will re-send them (and also, if ABR, packet code will + translate to Type-5's) - As usual, Type-5 LSA's (if not DISCARDED because we are STUB or - NSSA) are flooded throughout the AS, and are updated in the - global table. */ -#ifdef ORIGINAL_CODING - switch (lsa->data->type) { - case OSPF_ROUTER_LSA: - case OSPF_NETWORK_LSA: - case OSPF_SUMMARY_LSA: - case OSPF_ASBR_SUMMARY_LSA: - case OSPF_OPAQUE_LINK_LSA: /* ospf_flood_through_interface ? */ - case OSPF_OPAQUE_AREA_LSA: - lsa_ack_flag = - ospf_flood_through_area(inbr->oi->area, inbr, lsa); - break; - case OSPF_AS_EXTERNAL_LSA: /* Type-5 */ - case OSPF_OPAQUE_AS_LSA: - lsa_ack_flag = ospf_flood_through_as(ospf, inbr, lsa); - break; - /* Type-7 Only received within NSSA, then flooded */ - case OSPF_AS_NSSA_LSA: - /* Any P-bit was installed with the Type-7. */ - lsa_ack_flag = - ospf_flood_through_area(inbr->oi->area, inbr, lsa); - - if (IS_DEBUG_OSPF_NSSA) - zlog_debug( - "ospf_flood_through: LOCAL NSSA FLOOD of Type-7."); - break; - default: - break; - } -#else /* ORIGINAL_CODING */ + As usual, Type-5 LSA's (if not DISCARDED because we are STUB or + NSSA) are flooded throughout the AS, and are updated in the + global table. */ /* * At the common sub-sub-function "ospf_flood_through_interface()", * a parameter "inbr" will be used to distinguish the called context @@ -757,7 +722,6 @@ int ospf_flood_through(struct ospf *ospf, struct ospf_neighbor *inbr, lsa_ack_flag = ospf_flood_through_area(lsa->area, inbr, lsa); break; } -#endif /* ORIGINAL_CODING */ return (lsa_ack_flag); } diff --git a/ospfd/ospf_lsa.c b/ospfd/ospf_lsa.c index 5ab0927e71..3e3f288023 100644 --- a/ospfd/ospf_lsa.c +++ b/ospfd/ospf_lsa.c @@ -3202,45 +3202,6 @@ int ospf_lsa_different(struct ospf_lsa *l1, struct ospf_lsa *l2) return 0; } -#ifdef ORIGINAL_CODING -void ospf_lsa_flush_self_originated(struct ospf_neighbor *nbr, - struct ospf_lsa *self, struct ospf_lsa *new) -{ - uint32_t seqnum; - - /* Adjust LS Sequence Number. */ - seqnum = ntohl(new->data->ls_seqnum) + 1; - self->data->ls_seqnum = htonl(seqnum); - - /* Recalculate LSA checksum. */ - ospf_lsa_checksum(self->data); - - /* Reflooding LSA. */ - /* RFC2328 Section 13.3 - On non-broadcast networks, separate Link State Update - packets must be sent, as unicasts, to each adjacent neighbor - (i.e., those in state Exchange or greater). The destination - IP addresses for these packets are the neighbors' IP - addresses. */ - if (nbr->oi->type == OSPF_IFTYPE_NBMA) { - struct route_node *rn; - struct ospf_neighbor *onbr; - - for (rn = route_top(nbr->oi->nbrs); rn; rn = route_next(rn)) - if ((onbr = rn->info) != NULL) - if (onbr != nbr->oi->nbr_self - && onbr->status >= NSM_Exchange) - ospf_ls_upd_send_lsa( - onbr, self, - OSPF_SEND_PACKET_DIRECT); - } else - ospf_ls_upd_send_lsa(nbr, self, OSPF_SEND_PACKET_INDIRECT); - - if (IS_DEBUG_OSPF(lsa, LSA_GENERATE)) - zlog_debug("LSA[Type%d:%s]: Flush self-originated LSA", - self->data->type, inet_ntoa(self->data->id)); -} -#else /* ORIGINAL_CODING */ int ospf_lsa_flush_schedule(struct ospf *ospf, struct ospf_lsa *lsa) { if (lsa == NULL || !IS_LSA_SELF(lsa)) @@ -3345,7 +3306,6 @@ void ospf_flush_self_originated_lsas_now(struct ospf *ospf) return; } -#endif /* ORIGINAL_CODING */ /* If there is self-originated LSA, then return 1, otherwise return 0. */ /* An interface-independent version of ospf_lsa_is_self_originated */ diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c index 453135cb4d..6c7531a362 100644 --- a/ospfd/ospf_packet.c +++ b/ospfd/ospf_packet.c @@ -1505,10 +1505,6 @@ static void ospf_db_desc(struct ip *iph, struct ospf_header *ospfh, /* Check DD Options. */ if (dd->options != nbr->options) { -#ifdef ORIGINAL_CODING - /* Save the new options for debugging */ - nbr->options = dd->options; -#endif /* ORIGINAL_CODING */ flog_warn(EC_OSPF_PACKET, "Packet[DD]: Neighbor %s options mismatch.", inet_ntoa(nbr->router_id)); From 88b6b28ef33ff3c80ca4504c882735d3847279fc Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 12 Nov 2019 14:17:14 -0500 Subject: [PATCH 2/6] ospfd: Add a function to return the name of the vrf we are in. Add a helper function to return the name of the vrf we are in so it can be used as part of debug strings. Signed-off-by: Donald Sharp --- ospfd/ospfd.c | 8 ++++++++ ospfd/ospfd.h | 1 + 2 files changed, 9 insertions(+) diff --git a/ospfd/ospfd.c b/ospfd/ospfd.c index b12fa63723..5058886f36 100644 --- a/ospfd/ospfd.c +++ b/ospfd/ospfd.c @@ -2173,3 +2173,11 @@ const char *ospf_vrf_id_to_name(vrf_id_t vrf_id) return vrf ? vrf->name : "NIL"; } + +const char *ospf_get_name(const struct ospf *ospf) +{ + if (ospf->name) + return ospf->name; + else + return VRF_DEFAULT_NAME; +} diff --git a/ospfd/ospfd.h b/ospfd/ospfd.h index b31ad30375..937d363b4c 100644 --- a/ospfd/ospfd.h +++ b/ospfd/ospfd.h @@ -572,4 +572,5 @@ extern void ospf_vrf_unlink(struct ospf *ospf, struct vrf *vrf); const char *ospf_vrf_id_to_name(vrf_id_t vrf_id); int ospf_area_nssa_no_summary_set(struct ospf *, struct in_addr); +const char *ospf_get_name(const struct ospf *ospf); #endif /* _ZEBRA_OSPFD_H */ From 868a0861d2619ee4681132ece26349ea1ddd2d43 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 6 Nov 2019 19:49:06 -0500 Subject: [PATCH 3/6] ospfd: Add/fix some debugs to handle vrf This commit has: The received packet path in ospf, had absolutely no debugs associated with it. This makes it extremely hard to know when we receive packets for consumption. Add some breadcrumbs to this end. Large chunks of commands have no ability to debug what is happening in what vrf. With ip overlap X vrf this becomes a bit of a problem Add some breadcrumbs here. Signed-off-by: Donald Sharp --- ospfd/ospf_asbr.c | 9 +++++--- ospfd/ospf_flood.c | 47 ++++++++++++++++++++++++++---------------- ospfd/ospf_interface.c | 4 ++-- ospfd/ospf_nsm.c | 32 +++++++++++++++++----------- ospfd/ospf_packet.c | 13 +++++++++++- 5 files changed, 69 insertions(+), 36 deletions(-) diff --git a/ospfd/ospf_asbr.c b/ospfd/ospf_asbr.c index ea919017d3..a60af36564 100644 --- a/ospfd/ospf_asbr.c +++ b/ospfd/ospf_asbr.c @@ -238,20 +238,23 @@ struct ospf_lsa *ospf_external_info_find_lsa(struct ospf *ospf, /* Update ASBR status. */ void ospf_asbr_status_update(struct ospf *ospf, uint8_t status) { - zlog_info("ASBR[Status:%d]: Update", status); + zlog_info("ASBR[%s:Status:%d]: Update", + ospf_get_name(ospf), status); /* ASBR on. */ if (status) { /* Already ASBR. */ if (IS_OSPF_ASBR(ospf)) { - zlog_info("ASBR[Status:%d]: Already ASBR", status); + zlog_info("ASBR[%s:Status:%d]: Already ASBR", + ospf_get_name(ospf), status); return; } SET_FLAG(ospf->flags, OSPF_FLAG_ASBR); } else { /* Already non ASBR. */ if (!IS_OSPF_ASBR(ospf)) { - zlog_info("ASBR[Status:%d]: Already non ASBR", status); + zlog_info("ASBR[%s:Status:%d]: Already non ASBR", + ospf_get_name(ospf), status); return; } UNSET_FLAG(ospf->flags, OSPF_FLAG_ASBR); diff --git a/ospfd/ospf_flood.c b/ospfd/ospf_flood.c index a72caa8415..381fb6820f 100644 --- a/ospfd/ospf_flood.c +++ b/ospfd/ospf_flood.c @@ -157,9 +157,9 @@ static void ospf_process_self_originated_lsa(struct ospf *ospf, if (IS_DEBUG_OSPF_EVENT) zlog_debug( - "LSA[Type%d:%s]: Process self-originated LSA seq 0x%x", - new->data->type, inet_ntoa(new->data->id), - ntohl(new->data->ls_seqnum)); + "%s:LSA[Type%d:%s]: Process self-originated LSA seq 0x%x", + ospf_get_name(ospf), new->data->type, + inet_ntoa(new->data->id), ntohl(new->data->ls_seqnum)); /* If we're here, we installed a self-originated LSA that we received from a neighbor, i.e. it's more recent. We must see whether we want @@ -276,8 +276,8 @@ int ospf_flood(struct ospf *ospf, struct ospf_neighbor *nbr, if (IS_DEBUG_OSPF_EVENT) zlog_debug( - "LSA[Flooding]: start, NBR %s (%s), cur(%p), New-LSA[%s]", - inet_ntoa(nbr->router_id), + "%s:LSA[Flooding]: start, NBR %s (%s), cur(%p), New-LSA[%s]", + ospf_get_name(ospf), inet_ntoa(nbr->router_id), lookup_msg(ospf_nsm_state_msg, nbr->state, NULL), (void *)current, dump_lsa_key(new)); @@ -295,15 +295,16 @@ int ospf_flood(struct ospf *ospf, struct ospf_neighbor *nbr, == OSPF_INITIAL_SEQUENCE_NUMBER)) { if (IS_DEBUG_OSPF_EVENT) zlog_debug( - "LSA[Flooding]: Got a self-originated LSA, " - "while local one is initial instance."); + "%s:LSA[Flooding]: Got a self-originated LSA, while local one is initial instance.", + ospf_get_name(ospf)); ; /* Accept this LSA for quick LSDB resynchronization. */ } else if (monotime_since(¤t->tv_recv, NULL) < ospf->min_ls_arrival * 1000LL) { if (IS_DEBUG_OSPF_EVENT) zlog_debug( - "LSA[Flooding]: LSA is received recently."); + "%s:LSA[Flooding]: LSA is received recently.", + ospf_get_name(ospf)); return -1; } } @@ -376,9 +377,8 @@ static int ospf_flood_through_interface(struct ospf_interface *oi, if (IS_DEBUG_OSPF_EVENT) zlog_debug( - "ospf_flood_through_interface(): " - "considering int %s, INBR(%s), LSA[%s] AGE %u", - IF_NAME(oi), inbr ? inet_ntoa(inbr->router_id) : "NULL", + "%s:ospf_flood_through_interface(): considering int %s, INBR(%s), LSA[%s] AGE %u", + ospf_get_name(oi->ospf), IF_NAME(oi), inbr ? inet_ntoa(inbr->router_id) : "NULL", dump_lsa_key(lsa), ntohs(lsa->data->ls_age)); if (!ospf_if_is_enable(oi)) @@ -399,8 +399,9 @@ static int ospf_flood_through_interface(struct ospf_interface *oi, onbr = rn->info; if (IS_DEBUG_OSPF_EVENT) zlog_debug( - "ospf_flood_through_interface(): considering nbr %s (%s)", + "ospf_flood_through_interface(): considering nbr %s(%s) (%s)", inet_ntoa(onbr->router_id), + ospf_get_name(oi->ospf), lookup_msg(ospf_nsm_state_msg, onbr->state, NULL)); @@ -737,9 +738,10 @@ void ospf_ls_request_add(struct ospf_neighbor *nbr, struct ospf_lsa *lsa) * the common function "ospf_lsdb_add()" -- endo. */ if (IS_DEBUG_OSPF(lsa, LSA_FLOODING)) - zlog_debug("RqstL(%lu)++, NBR(%s), LSA[%s]", + zlog_debug("RqstL(%lu)++, NBR(%s(%s)), LSA[%s]", ospf_ls_request_count(nbr), - inet_ntoa(nbr->router_id), dump_lsa_key(lsa)); + inet_ntoa(nbr->router_id), + ospf_get_name(nbr->oi->ospf), dump_lsa_key(lsa)); ospf_lsdb_add(&nbr->ls_req, lsa); } @@ -763,9 +765,10 @@ void ospf_ls_request_delete(struct ospf_neighbor *nbr, struct ospf_lsa *lsa) } if (IS_DEBUG_OSPF(lsa, LSA_FLOODING)) /* -- endo. */ - zlog_debug("RqstL(%lu)--, NBR(%s), LSA[%s]", + zlog_debug("RqstL(%lu)--, NBR(%s(%s)), LSA[%s]", ospf_ls_request_count(nbr), - inet_ntoa(nbr->router_id), dump_lsa_key(lsa)); + inet_ntoa(nbr->router_id), + ospf_get_name(nbr->oi->ospf), dump_lsa_key(lsa)); ospf_lsdb_delete(&nbr->ls_req, lsa); } @@ -823,6 +826,12 @@ void ospf_ls_retransmit_add(struct ospf_neighbor *nbr, struct ospf_lsa *lsa) if (ospf_lsa_more_recent(old, lsa) < 0) { if (old) { old->retransmit_counter--; + if (IS_DEBUG_OSPF(lsa, LSA_FLOODING)) + zlog_debug("RXmtL(%lu)--, NBR(%s(%s)), LSA[%s]", + ospf_ls_retransmit_count(nbr), + inet_ntoa(nbr->router_id), + ospf_get_name(nbr->oi->ospf), + dump_lsa_key(old)); ospf_lsdb_delete(&nbr->ls_rxmt, old); } lsa->retransmit_counter++; @@ -835,9 +844,10 @@ void ospf_ls_retransmit_add(struct ospf_neighbor *nbr, struct ospf_lsa *lsa) * the common function "ospf_lsdb_add()" -- endo. */ if (IS_DEBUG_OSPF(lsa, LSA_FLOODING)) - zlog_debug("RXmtL(%lu)++, NBR(%s), LSA[%s]", + zlog_debug("RXmtL(%lu)++, NBR(%s(%s)), LSA[%s]", ospf_ls_retransmit_count(nbr), inet_ntoa(nbr->router_id), + ospf_get_name(nbr->oi->ospf), dump_lsa_key(lsa)); ospf_lsdb_add(&nbr->ls_rxmt, lsa); } @@ -849,9 +859,10 @@ void ospf_ls_retransmit_delete(struct ospf_neighbor *nbr, struct ospf_lsa *lsa) if (ospf_ls_retransmit_lookup(nbr, lsa)) { lsa->retransmit_counter--; if (IS_DEBUG_OSPF(lsa, LSA_FLOODING)) /* -- endo. */ - zlog_debug("RXmtL(%lu)--, NBR(%s), LSA[%s]", + zlog_debug("RXmtL(%lu)--, NBR(%s(%s)), LSA[%s]", ospf_ls_retransmit_count(nbr), inet_ntoa(nbr->router_id), + ospf_get_name(nbr->oi->ospf), dump_lsa_key(lsa)); ospf_lsdb_delete(&nbr->ls_rxmt, lsa); } diff --git a/ospfd/ospf_interface.c b/ospfd/ospf_interface.c index 5459e3b87c..7ddffbcdbd 100644 --- a/ospfd/ospf_interface.c +++ b/ospfd/ospf_interface.c @@ -273,7 +273,7 @@ struct ospf_interface *ospf_if_new(struct ospf *ospf, struct interface *ifp, if (IS_DEBUG_OSPF_EVENT) zlog_debug("%s: ospf interface %s vrf %s id %u created", __PRETTY_FUNCTION__, ifp->name, - ospf_vrf_id_to_name(ospf->vrf_id), ospf->vrf_id); + ospf_get_name(ospf), ospf->vrf_id); return oi; } @@ -832,7 +832,7 @@ struct ospf_interface *ospf_vl_new(struct ospf *ospf, struct prefix_ipv4 *p; if (IS_DEBUG_OSPF_EVENT) - zlog_debug("ospf_vl_new(): Start"); + zlog_debug("ospf_vl_new()(%s): Start", ospf_get_name(ospf)); if (vlink_count == OSPF_VL_MAX_COUNT) { if (IS_DEBUG_OSPF_EVENT) zlog_debug( diff --git a/ospfd/ospf_nsm.c b/ospfd/ospf_nsm.c index 00174b638e..110738802c 100644 --- a/ospfd/ospf_nsm.c +++ b/ospfd/ospf_nsm.c @@ -65,8 +65,9 @@ static int ospf_inactivity_timer(struct thread *thread) nbr->t_inactivity = NULL; if (IS_DEBUG_OSPF(nsm, NSM_TIMERS)) - zlog_debug("NSM[%s:%s]: Timer (Inactivity timer expire)", - IF_NAME(nbr->oi), inet_ntoa(nbr->router_id)); + zlog_debug("NSM[%s:%s:%s]: Timer (Inactivity timer expire)", + IF_NAME(nbr->oi), inet_ntoa(nbr->router_id), + ospf_get_name(nbr->oi->ospf)); OSPF_NSM_EVENT_SCHEDULE(nbr, NSM_InactivityTimer); @@ -81,8 +82,9 @@ static int ospf_db_desc_timer(struct thread *thread) nbr->t_db_desc = NULL; if (IS_DEBUG_OSPF(nsm, NSM_TIMERS)) - zlog_debug("NSM[%s:%s]: Timer (DD Retransmit timer expire)", - IF_NAME(nbr->oi), inet_ntoa(nbr->src)); + zlog_debug("NSM[%s:%s:%s]: Timer (DD Retransmit timer expire)", + IF_NAME(nbr->oi), inet_ntoa(nbr->src), + ospf_get_name(nbr->oi->ospf)); /* resent last send DD packet. */ assert(nbr->last_send); @@ -387,9 +389,10 @@ static int nsm_kill_nbr(struct ospf_neighbor *nbr) if (IS_DEBUG_OSPF(nsm, NSM_EVENTS)) zlog_debug( - "NSM[%s:%s]: Down (PollIntervalTimer scheduled)", + "NSM[%s:%s:%s]: Down (PollIntervalTimer scheduled)", IF_NAME(nbr->oi), - inet_ntoa(nbr->address.u.prefix4)); + inet_ntoa(nbr->address.u.prefix4), + ospf_get_name(nbr->oi->ospf)); } return 0; @@ -585,8 +588,9 @@ static void nsm_notice_state_change(struct ospf_neighbor *nbr, int next_state, { /* Logging change of status. */ if (IS_DEBUG_OSPF(nsm, NSM_STATUS)) - zlog_debug("NSM[%s:%s]: State change %s -> %s (%s)", + zlog_debug("NSM[%s:%s:%s]: State change %s -> %s (%s)", IF_NAME(nbr->oi), inet_ntoa(nbr->router_id), + ospf_get_name(nbr->oi->ospf), lookup_msg(ospf_nsm_state_msg, nbr->state, NULL), lookup_msg(ospf_nsm_state_msg, next_state, NULL), ospf_nsm_event_str[event]); @@ -595,8 +599,9 @@ static void nsm_notice_state_change(struct ospf_neighbor *nbr, int next_state, if (CHECK_FLAG(nbr->oi->ospf->config, OSPF_LOG_ADJACENCY_CHANGES) && (CHECK_FLAG(nbr->oi->ospf->config, OSPF_LOG_ADJACENCY_DETAIL) || (next_state == NSM_Full) || (next_state < nbr->state))) - zlog_notice("AdjChg: Nbr %s on %s: %s -> %s (%s)", - inet_ntoa(nbr->router_id), IF_NAME(nbr->oi), + zlog_notice("AdjChg: Nbr %s(%s) on %s: %s -> %s (%s)", + inet_ntoa(nbr->router_id), + ospf_get_name(nbr->oi->ospf), IF_NAME(nbr->oi), lookup_msg(ospf_nsm_state_msg, nbr->state, NULL), lookup_msg(ospf_nsm_state_msg, next_state, NULL), ospf_nsm_event_str[event]); @@ -677,9 +682,10 @@ static void nsm_change_state(struct ospf_neighbor *nbr, int state) if (CHECK_FLAG(oi->ospf->config, OSPF_LOG_ADJACENCY_DETAIL)) zlog_info( - "%s:(%s, %s -> %s): " + "%s:[%s:%s], %s -> %s): " "scheduling new router-LSA origination", __PRETTY_FUNCTION__, inet_ntoa(nbr->router_id), + ospf_get_name(oi->ospf), lookup_msg(ospf_nsm_state_msg, old_state, NULL), lookup_msg(ospf_nsm_state_msg, state, NULL)); @@ -753,8 +759,9 @@ int ospf_nsm_event(struct thread *thread) event = THREAD_VAL(thread); if (IS_DEBUG_OSPF(nsm, NSM_EVENTS)) - zlog_debug("NSM[%s:%s]: %s (%s)", IF_NAME(nbr->oi), + zlog_debug("NSM[%s:%s:%s]: %s (%s)", IF_NAME(nbr->oi), inet_ntoa(nbr->router_id), + ospf_get_name(nbr->oi->ospf), lookup_msg(ospf_nsm_state_msg, nbr->state, NULL), ospf_nsm_event_str[event]); @@ -777,9 +784,10 @@ int ospf_nsm_event(struct thread *thread) */ flog_err( EC_OSPF_FSM_INVALID_STATE, - "NSM[%s:%s]: %s (%s): " + "NSM[%s:%s:%s]: %s (%s): " "Warning: action tried to change next_state to %s", IF_NAME(nbr->oi), inet_ntoa(nbr->router_id), + ospf_get_name(nbr->oi->ospf), lookup_msg(ospf_nsm_state_msg, nbr->state, NULL), ospf_nsm_event_str[event], diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c index 6c7531a362..a9067e0119 100644 --- a/ospfd/ospf_packet.c +++ b/ospfd/ospf_packet.c @@ -2381,6 +2381,10 @@ static struct stream *ospf_recv_packet(struct ospf *ospf, int fd, return NULL; } + if (IS_DEBUG_OSPF_PACKET(0, RECV)) + zlog_debug("%s: fd %d(%s) on interface %d(%s)", + __PRETTY_FUNCTION__, fd, ospf_get_name(ospf), + ifindex, *ifp ? (*ifp)->name : "Unknown"); return ibuf; } @@ -2981,8 +2985,15 @@ int ospf_read(struct thread *thread) ospf->vrf_id); if (c) ifp = c->ifp; - if (ifp == NULL) + if (ifp == NULL) { + if (IS_DEBUG_OSPF_PACKET(0, RECV)) + zlog_debug( + "%s: Unable to determine incoming interface from: %s(%s)", + __PRETTY_FUNCTION__, + inet_ntoa(iph->ip_src), + ospf_get_name(ospf)); return 0; + } } /* IP Header dump. */ From edca5860cbee1adb9e35fa074b4e20d4178c44c7 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 6 Nov 2019 20:17:29 -0500 Subject: [PATCH 4/6] ospfd: The ip header dump is crazy long and useless Turning on packet debugs and seeing a header dump that is 11 lines long is useless 2019/11/07 01:07:05.941798 OSPF: ip_v 4 2019/11/07 01:07:05.941806 OSPF: ip_hl 5 2019/11/07 01:07:05.941813 OSPF: ip_tos 192 2019/11/07 01:07:05.941821 OSPF: ip_len 68 2019/11/07 01:07:05.941831 OSPF: ip_id 48576 2019/11/07 01:07:05.941838 OSPF: ip_off 0 2019/11/07 01:07:05.941845 OSPF: ip_ttl 1 2019/11/07 01:07:05.941857 OSPF: ip_p 89 2019/11/07 01:07:05.941865 OSPF: ip_sum 0xcf33 2019/11/07 01:07:05.941873 OSPF: ip_src 200.254.30.14 2019/11/07 01:07:05.941882 OSPF: ip_dst 224.0.0.5 We already have this debugged, it's not going to change and the end developer can stick this back in if needed by hand to debug something that is not working properly. Signed-off-by: Donald Sharp --- ospfd/ospf_dump.c | 17 ----------------- ospfd/ospf_dump.h | 1 - ospfd/ospf_packet.c | 12 ------------ 3 files changed, 30 deletions(-) diff --git a/ospfd/ospf_dump.c b/ospfd/ospf_dump.c index f74d9733ee..dffcb930e4 100644 --- a/ospfd/ospf_dump.c +++ b/ospfd/ospf_dump.c @@ -501,23 +501,6 @@ static void ospf_packet_ls_ack_dump(struct stream *s, uint16_t length) stream_set_getp(s, sp); } -/* Expects header to be in host order */ -void ospf_ip_header_dump(struct ip *iph) -{ - /* IP Header dump. */ - zlog_debug("ip_v %d", iph->ip_v); - zlog_debug("ip_hl %d", iph->ip_hl); - zlog_debug("ip_tos %d", iph->ip_tos); - zlog_debug("ip_len %d", iph->ip_len); - zlog_debug("ip_id %u", (uint32_t)iph->ip_id); - zlog_debug("ip_off %u", (uint32_t)iph->ip_off); - zlog_debug("ip_ttl %d", iph->ip_ttl); - zlog_debug("ip_p %d", iph->ip_p); - zlog_debug("ip_sum 0x%x", (uint32_t)iph->ip_sum); - zlog_debug("ip_src %s", inet_ntoa(iph->ip_src)); - zlog_debug("ip_dst %s", inet_ntoa(iph->ip_dst)); -} - static void ospf_header_dump(struct ospf_header *ospfh) { char buf[9]; diff --git a/ospfd/ospf_dump.h b/ospfd/ospf_dump.h index 09c1811c3c..6b2ebb125a 100644 --- a/ospfd/ospf_dump.h +++ b/ospfd/ospf_dump.h @@ -133,7 +133,6 @@ extern const char *ospf_if_name_string(struct ospf_interface *); extern void ospf_nbr_state_message(struct ospf_neighbor *, char *, size_t); extern const char *ospf_timer_dump(struct thread *, char *, size_t); extern const char *ospf_timeval_dump(struct timeval *, char *, size_t); -extern void ospf_ip_header_dump(struct ip *); extern void ospf_packet_dump(struct stream *); extern void ospf_debug_init(void); diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c index a9067e0119..7415c0316a 100644 --- a/ospfd/ospf_packet.c +++ b/ospfd/ospf_packet.c @@ -614,13 +614,6 @@ static void ospf_write_frags(int fd, struct ospf_packet *op, struct ip *iph, "ospf_write_frags: sent id %d, off %d, len %d to %s\n", iph->ip_id, iph->ip_off, iph->ip_len, inet_ntoa(iph->ip_dst)); - if (IS_DEBUG_OSPF_PACKET(type - 1, DETAIL)) { - zlog_debug( - "-----------------IP Header Dump----------------------"); - ospf_ip_header_dump(iph); - zlog_debug( - "-----------------------------------------------------"); - } } iph->ip_off += offset; @@ -824,7 +817,6 @@ static int ospf_write(struct thread *thread) if (IS_DEBUG_OSPF_PACKET(type - 1, DETAIL)) { zlog_debug( "-----------------------------------------------------"); - ospf_ip_header_dump(&iph); stream_set_getp(op->s, 0); ospf_packet_dump(op->s); } @@ -2996,10 +2988,6 @@ int ospf_read(struct thread *thread) } } - /* IP Header dump. */ - if (IS_DEBUG_OSPF_PACKET(0, RECV)) - ospf_ip_header_dump(iph); - /* Self-originated packet should be discarded silently. */ if (ospf_if_lookup_by_local_addr(ospf, NULL, iph->ip_src)) { if (IS_DEBUG_OSPF_PACKET(0, RECV)) { From 4392cc43379c79b2ac601d4f38a30d8505b5b54d Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 6 Nov 2019 23:04:32 -0500 Subject: [PATCH 5/6] ospfd: Allow packet reads based upon read/write packet counts Read in up to 20(ospf write-multipler X) packets, for handling of data. This improves performance because we allow ospf to have a bit more data to work on in one go for spf calculations instead of 1 packet at a time. Signed-off-by: Donald Sharp --- ospfd/ospf_packet.c | 412 +++++++++++++++++++++++--------------------- 1 file changed, 217 insertions(+), 195 deletions(-) diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c index 7415c0316a..d168a8ef38 100644 --- a/ospfd/ospf_packet.c +++ b/ospfd/ospf_packet.c @@ -2310,10 +2310,12 @@ static struct stream *ospf_recv_packet(struct ospf *ospf, int fd, msgh.msg_control = (caddr_t)buff; msgh.msg_controllen = sizeof(buff); - ret = stream_recvmsg(ibuf, fd, &msgh, 0, OSPF_MAX_PACKET_SIZE + 1); + ret = stream_recvmsg(ibuf, fd, &msgh, MSG_DONTWAIT, + OSPF_MAX_PACKET_SIZE + 1); if (ret < 0) { - flog_warn(EC_OSPF_PACKET, "stream_recvmsg failed: %s", - safe_strerror(errno)); + if (errno != EAGAIN && errno != EWOULDBLOCK) + flog_warn(EC_OSPF_PACKET, "stream_recvmsg failed: %s", + safe_strerror(errno)); return NULL; } if ((unsigned int)ret < sizeof(iph)) /* ret must be > 0 now */ @@ -2947,228 +2949,248 @@ int ospf_read(struct thread *thread) uint16_t length; struct interface *ifp = NULL; struct connected *c; + int32_t count = 0; /* first of all get interface pointer. */ ospf = THREAD_ARG(thread); /* prepare for next packet. */ - ospf->t_read = NULL; thread_add_read(master, ospf_read, ospf, ospf->fd, &ospf->t_read); - stream_reset(ospf->ibuf); - ibuf = ospf_recv_packet(ospf, ospf->fd, &ifp, ospf->ibuf); - if (ibuf == NULL) - return -1; - /* This raw packet is known to be at least as big as its IP header. */ + while (count < ospf->write_oi_count) { + count++; + stream_reset(ospf->ibuf); + ibuf = ospf_recv_packet(ospf, ospf->fd, &ifp, ospf->ibuf); + if (ibuf == NULL) + return -1; + /* + * This raw packet is known to be at least as big as its + * IP header. + * Note that there should not be alignment problems with + * this assignment because this is at the beginning of the + * stream data buffer. + */ + iph = (struct ip *)STREAM_DATA(ibuf); + /* Note that sockopt_iphdrincl_swab_systoh was called in + * ospf_recv_packet. */ - /* Note that there should not be alignment problems with this assignment - because this is at the beginning of the stream data buffer. */ - iph = (struct ip *)STREAM_DATA(ibuf); - /* Note that sockopt_iphdrincl_swab_systoh was called in - * ospf_recv_packet. */ - - if (ifp == NULL) { - /* Handle cases where the platform does not support retrieving - the ifindex, - and also platforms (such as Solaris 8) that claim to support - ifindex - retrieval but do not. */ - c = if_lookup_address((void *)&iph->ip_src, AF_INET, - ospf->vrf_id); - if (c) - ifp = c->ifp; if (ifp == NULL) { - if (IS_DEBUG_OSPF_PACKET(0, RECV)) + /* Handle cases where the platform does not support + * retrieving the ifindex, and also platforms (such + * as Solaris 8) that claim to support ifindex + * retrieval but do not. + */ + c = if_lookup_address((void *)&iph->ip_src, AF_INET, + ospf->vrf_id); + if (c) + ifp = c->ifp; + if (ifp == NULL) { + if (IS_DEBUG_OSPF_PACKET(0, RECV)) + zlog_debug( + "%s: Unable to determine incoming interface from: %s(%s)", + __PRETTY_FUNCTION__, + inet_ntoa(iph->ip_src), + ospf_get_name(ospf)); + continue; + } + } + + /* Self-originated packet should be discarded silently. */ + if (ospf_if_lookup_by_local_addr(ospf, NULL, iph->ip_src)) { + if (IS_DEBUG_OSPF_PACKET(0, RECV)) { zlog_debug( - "%s: Unable to determine incoming interface from: %s(%s)", - __PRETTY_FUNCTION__, - inet_ntoa(iph->ip_src), - ospf_get_name(ospf)); - return 0; + "ospf_read[%s]: Dropping self-originated packet", + inet_ntoa(iph->ip_src)); + } + continue; } - } - /* Self-originated packet should be discarded silently. */ - if (ospf_if_lookup_by_local_addr(ospf, NULL, iph->ip_src)) { - if (IS_DEBUG_OSPF_PACKET(0, RECV)) { - zlog_debug( - "ospf_read[%s]: Dropping self-originated packet", - inet_ntoa(iph->ip_src)); + /* + * Advance from IP header to OSPF header (iph->ip_hl has + * been verified by ospf_recv_packet() to be correct). + */ + stream_forward_getp(ibuf, iph->ip_hl * 4); + + ospfh = (struct ospf_header *)stream_pnt(ibuf); + if (MSG_OK + != ospf_packet_examin(ospfh, + stream_get_endp(ibuf) + - stream_get_getp(ibuf))) + continue; + /* Now it is safe to access all fields of OSPF packet header. */ + + /* associate packet with ospf interface */ + oi = ospf_if_lookup_recv_if(ospf, iph->ip_src, ifp); + + /* ospf_verify_header() relies on a valid "oi" and thus can + * be called only after the passive/backbone/other checks + * below are passed. These checks in turn access the fields + * of unverified "ospfh" structure for their own purposes and + * must remain very accurate in doing this. */ + + /* If incoming interface is passive one, ignore it. */ + if (oi && OSPF_IF_PASSIVE_STATUS(oi) == OSPF_IF_PASSIVE) { + char buf[3][INET_ADDRSTRLEN]; + + if (IS_DEBUG_OSPF_EVENT) + zlog_debug( + "ignoring packet from router %s sent to %s, " + "received on a passive interface, %s", + inet_ntop(AF_INET, &ospfh->router_id, + buf[0], sizeof(buf[0])), + inet_ntop(AF_INET, &iph->ip_dst, buf[1], + sizeof(buf[1])), + inet_ntop(AF_INET, + &oi->address->u.prefix4, + buf[2], sizeof(buf[2]))); + + if (iph->ip_dst.s_addr == htonl(OSPF_ALLSPFROUTERS)) { + /* Try to fix multicast membership. + * Some OS:es may have problems in this area, + * make sure it is removed. + */ + OI_MEMBER_JOINED(oi, MEMBER_ALLROUTERS); + ospf_if_set_multicast(oi); + } + continue; } - return 0; - } - /* Advance from IP header to OSPF header (iph->ip_hl has been verified - by ospf_recv_packet() to be correct). */ - stream_forward_getp(ibuf, iph->ip_hl * 4); - ospfh = (struct ospf_header *)stream_pnt(ibuf); - if (MSG_OK - != ospf_packet_examin( - ospfh, stream_get_endp(ibuf) - stream_get_getp(ibuf))) - return -1; - /* Now it is safe to access all fields of OSPF packet header. */ + /* if no local ospf_interface, + * or header area is backbone but ospf_interface is not + * check for VLINK interface + */ + if ((oi == NULL) + || (OSPF_IS_AREA_ID_BACKBONE(ospfh->area_id) + && !OSPF_IS_AREA_ID_BACKBONE(oi->area->area_id))) { + if ((oi = ospf_associate_packet_vl(ospf, ifp, iph, + ospfh)) + == NULL) { + if (!ospf->instance && IS_DEBUG_OSPF_EVENT) + zlog_debug( + "Packet from [%s] received on link %s" + " but no ospf_interface", + inet_ntoa(iph->ip_src), + ifp->name); + return 0; + } + } - /* associate packet with ospf interface */ - oi = ospf_if_lookup_recv_if(ospf, iph->ip_src, ifp); + /* + * else it must be a local ospf interface, check it was + * received on correct link + */ + else if (oi->ifp != ifp) { + if (IS_DEBUG_OSPF_EVENT) + flog_warn( + EC_OSPF_PACKET, + "Packet from [%s] received on wrong link %s", + inet_ntoa(iph->ip_src), ifp->name); + continue; + } else if (oi->state == ISM_Down) { + char buf[2][INET_ADDRSTRLEN]; - /* ospf_verify_header() relies on a valid "oi" and thus can be called - only - after the passive/backbone/other checks below are passed. These - checks - in turn access the fields of unverified "ospfh" structure for their - own - purposes and must remain very accurate in doing this. */ - - /* If incoming interface is passive one, ignore it. */ - if (oi && OSPF_IF_PASSIVE_STATUS(oi) == OSPF_IF_PASSIVE) { - char buf[3][INET_ADDRSTRLEN]; - - if (IS_DEBUG_OSPF_EVENT) - zlog_debug( - "ignoring packet from router %s sent to %s, " - "received on a passive interface, %s", - inet_ntop(AF_INET, &ospfh->router_id, buf[0], + flog_warn( + EC_OSPF_PACKET, + "Ignoring packet from %s to %s received on interface that is " + "down [%s]; interface flags are %s", + inet_ntop(AF_INET, &iph->ip_src, buf[0], sizeof(buf[0])), inet_ntop(AF_INET, &iph->ip_dst, buf[1], sizeof(buf[1])), - inet_ntop(AF_INET, &oi->address->u.prefix4, - buf[2], sizeof(buf[2]))); - - if (iph->ip_dst.s_addr == htonl(OSPF_ALLSPFROUTERS)) { - /* Try to fix multicast membership. - * Some OS:es may have problems in this area, - * make sure it is removed. - */ - OI_MEMBER_JOINED(oi, MEMBER_ALLROUTERS); - ospf_if_set_multicast(oi); + ifp->name, if_flag_dump(ifp->flags)); + /* Fix multicast memberships? */ + if (iph->ip_dst.s_addr == htonl(OSPF_ALLSPFROUTERS)) + OI_MEMBER_JOINED(oi, MEMBER_ALLROUTERS); + else if (iph->ip_dst.s_addr == htonl(OSPF_ALLDROUTERS)) + OI_MEMBER_JOINED(oi, MEMBER_DROUTERS); + if (oi->multicast_memberships) + ospf_if_set_multicast(oi); + continue; } - return 0; - } + /* + * If the received packet is destined for AllDRouters, the + * packet should be accepted only if the received ospf + * interface state is either DR or Backup -- endo. + * + * I wonder who endo is? + */ + if (iph->ip_dst.s_addr == htonl(OSPF_ALLDROUTERS) + && (oi->state != ISM_DR && oi->state != ISM_Backup)) { + flog_warn( + EC_OSPF_PACKET, + "Dropping packet for AllDRouters from [%s] via [%s] (ISM: %s)", + inet_ntoa(iph->ip_src), IF_NAME(oi), + lookup_msg(ospf_ism_state_msg, oi->state, + NULL)); + /* Try to fix multicast membership. */ + SET_FLAG(oi->multicast_memberships, MEMBER_DROUTERS); + ospf_if_set_multicast(oi); + continue; + } - /* if no local ospf_interface, - * or header area is backbone but ospf_interface is not - * check for VLINK interface - */ - if ((oi == NULL) || (OSPF_IS_AREA_ID_BACKBONE(ospfh->area_id) - && !OSPF_IS_AREA_ID_BACKBONE(oi->area->area_id))) { - if ((oi = ospf_associate_packet_vl(ospf, ifp, iph, ospfh)) - == NULL) { - if (!ospf->instance && IS_DEBUG_OSPF_EVENT) + /* Verify more OSPF header fields. */ + ret = ospf_verify_header(ibuf, oi, iph, ospfh); + if (ret < 0) { + if (IS_DEBUG_OSPF_PACKET(0, RECV)) zlog_debug( - "Packet from [%s] received on link %s" - " but no ospf_interface", - inet_ntoa(iph->ip_src), ifp->name); - return 0; - } - } - - /* else it must be a local ospf interface, check it was received on - * correct link - */ - else if (oi->ifp != ifp) { - if (IS_DEBUG_OSPF_EVENT) - flog_warn(EC_OSPF_PACKET, - "Packet from [%s] received on wrong link %s", - inet_ntoa(iph->ip_src), ifp->name); - return 0; - } else if (oi->state == ISM_Down) { - char buf[2][INET_ADDRSTRLEN]; - flog_warn( - EC_OSPF_PACKET, - "Ignoring packet from %s to %s received on interface that is " - "down [%s]; interface flags are %s", - inet_ntop(AF_INET, &iph->ip_src, buf[0], - sizeof(buf[0])), - inet_ntop(AF_INET, &iph->ip_dst, buf[1], - sizeof(buf[1])), - ifp->name, if_flag_dump(ifp->flags)); - /* Fix multicast memberships? */ - if (iph->ip_dst.s_addr == htonl(OSPF_ALLSPFROUTERS)) - OI_MEMBER_JOINED(oi, MEMBER_ALLROUTERS); - else if (iph->ip_dst.s_addr == htonl(OSPF_ALLDROUTERS)) - OI_MEMBER_JOINED(oi, MEMBER_DROUTERS); - if (oi->multicast_memberships) - ospf_if_set_multicast(oi); - return 0; - } - - /* - * If the received packet is destined for AllDRouters, the packet - * should be accepted only if the received ospf interface state is - * either DR or Backup -- endo. - */ - if (iph->ip_dst.s_addr == htonl(OSPF_ALLDROUTERS) - && (oi->state != ISM_DR && oi->state != ISM_Backup)) { - flog_warn( - EC_OSPF_PACKET, - "Dropping packet for AllDRouters from [%s] via [%s] (ISM: %s)", - inet_ntoa(iph->ip_src), IF_NAME(oi), - lookup_msg(ospf_ism_state_msg, oi->state, NULL)); - /* Try to fix multicast membership. */ - SET_FLAG(oi->multicast_memberships, MEMBER_DROUTERS); - ospf_if_set_multicast(oi); - return 0; - } - - /* Verify more OSPF header fields. */ - ret = ospf_verify_header(ibuf, oi, iph, ospfh); - if (ret < 0) { - if (IS_DEBUG_OSPF_PACKET(0, RECV)) - zlog_debug( - "ospf_read[%s]: Header check failed, " - "dropping.", - inet_ntoa(iph->ip_src)); - return ret; - } - - /* Show debug receiving packet. */ - if (IS_DEBUG_OSPF_PACKET(ospfh->type - 1, RECV)) { - if (IS_DEBUG_OSPF_PACKET(ospfh->type - 1, DETAIL)) { - zlog_debug( - "-----------------------------------------------------"); - ospf_packet_dump(ibuf); + "ospf_read[%s]: Header check failed, " + "dropping.", + inet_ntoa(iph->ip_src)); + continue; } - zlog_debug("%s received from [%s] via [%s]", - lookup_msg(ospf_packet_type_str, ospfh->type, NULL), - inet_ntoa(ospfh->router_id), IF_NAME(oi)); - zlog_debug(" src [%s],", inet_ntoa(iph->ip_src)); - zlog_debug(" dst [%s]", inet_ntoa(iph->ip_dst)); + /* Show debug receiving packet. */ + if (IS_DEBUG_OSPF_PACKET(ospfh->type - 1, RECV)) { + if (IS_DEBUG_OSPF_PACKET(ospfh->type - 1, DETAIL)) { + zlog_debug( + "-----------------------------------------------------"); + ospf_packet_dump(ibuf); + } - if (IS_DEBUG_OSPF_PACKET(ospfh->type - 1, DETAIL)) - zlog_debug( - "-----------------------------------------------------"); - } + zlog_debug("%s received from [%s] via [%s]", + lookup_msg(ospf_packet_type_str, ospfh->type, + NULL), + inet_ntoa(ospfh->router_id), IF_NAME(oi)); + zlog_debug(" src [%s],", inet_ntoa(iph->ip_src)); + zlog_debug(" dst [%s]", inet_ntoa(iph->ip_dst)); - stream_forward_getp(ibuf, OSPF_HEADER_SIZE); + if (IS_DEBUG_OSPF_PACKET(ospfh->type - 1, DETAIL)) + zlog_debug( + "-----------------------------------------------------"); + } - /* Adjust size to message length. */ - length = ntohs(ospfh->length) - OSPF_HEADER_SIZE; + stream_forward_getp(ibuf, OSPF_HEADER_SIZE); - /* Read rest of the packet and call each sort of packet routine. */ - switch (ospfh->type) { - case OSPF_MSG_HELLO: - ospf_hello(iph, ospfh, ibuf, oi, length); - break; - case OSPF_MSG_DB_DESC: - ospf_db_desc(iph, ospfh, ibuf, oi, length); - break; - case OSPF_MSG_LS_REQ: - ospf_ls_req(iph, ospfh, ibuf, oi, length); - break; - case OSPF_MSG_LS_UPD: - ospf_ls_upd(ospf, iph, ospfh, ibuf, oi, length); - break; - case OSPF_MSG_LS_ACK: - ospf_ls_ack(iph, ospfh, ibuf, oi, length); - break; - default: - flog_warn(EC_OSPF_PACKET, - "interface %s: OSPF packet header type %d is illegal", - IF_NAME(oi), ospfh->type); - break; + /* Adjust size to message length. */ + length = ntohs(ospfh->length) - OSPF_HEADER_SIZE; + + /* Read rest of the packet and call each sort of packet routine. + */ + switch (ospfh->type) { + case OSPF_MSG_HELLO: + ospf_hello(iph, ospfh, ibuf, oi, length); + break; + case OSPF_MSG_DB_DESC: + ospf_db_desc(iph, ospfh, ibuf, oi, length); + break; + case OSPF_MSG_LS_REQ: + ospf_ls_req(iph, ospfh, ibuf, oi, length); + break; + case OSPF_MSG_LS_UPD: + ospf_ls_upd(ospf, iph, ospfh, ibuf, oi, length); + break; + case OSPF_MSG_LS_ACK: + ospf_ls_ack(iph, ospfh, ibuf, oi, length); + break; + default: + flog_warn( + EC_OSPF_PACKET, + "interface %s(%s): OSPF packet header type %d is illegal", + IF_NAME(oi), ospf_get_name(ospf), ospfh->type); + break; + } } return 0; From 0263751346afe9e9924caca33e0afdf2d92d552b Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 19 Nov 2019 08:09:56 -0500 Subject: [PATCH 6/6] ospfd: Rework ospf_read_packet into 2 functions The indentation level for ospf_read was starting to be pretty extremene. Rework into 2 functions for improved readability. Signed-off-by: Donald Sharp --- ospfd/ospf_packet.c | 479 +++++++++++++++++++++++--------------------- 1 file changed, 247 insertions(+), 232 deletions(-) diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c index d168a8ef38..8634589b11 100644 --- a/ospfd/ospf_packet.c +++ b/ospfd/ospf_packet.c @@ -2937,19 +2937,258 @@ static int ospf_verify_header(struct stream *ibuf, struct ospf_interface *oi, return 0; } -/* Starting point of packet process function. */ -int ospf_read(struct thread *thread) +enum ospf_read_return_enum { + OSPF_READ_ERROR, + OSPF_READ_CONTINUE, +}; + +static enum ospf_read_return_enum ospf_read_helper(struct ospf *ospf) { int ret; struct stream *ibuf; - struct ospf *ospf; struct ospf_interface *oi; struct ip *iph; struct ospf_header *ospfh; uint16_t length; - struct interface *ifp = NULL; struct connected *c; + struct interface *ifp = NULL; + + stream_reset(ospf->ibuf); + ibuf = ospf_recv_packet(ospf, ospf->fd, &ifp, ospf->ibuf); + if (ibuf == NULL) + return OSPF_READ_ERROR; + + /* + * This raw packet is known to be at least as big as its + * IP header. Note that there should not be alignment problems with + * this assignment because this is at the beginning of the + * stream data buffer. + */ + iph = (struct ip *)STREAM_DATA(ibuf); + /* + * Note that sockopt_iphdrincl_swab_systoh was called in + * ospf_recv_packet. + */ + if (ifp == NULL) { + /* + * Handle cases where the platform does not support + * retrieving the ifindex, and also platforms (such as + * Solaris 8) that claim to support ifindex retrieval but do + * not. + */ + c = if_lookup_address((void *)&iph->ip_src, AF_INET, + ospf->vrf_id); + if (c) + ifp = c->ifp; + if (ifp == NULL) { + if (IS_DEBUG_OSPF_PACKET(0, RECV)) + zlog_debug( + "%s: Unable to determine incoming interface from: %s(%s)", + __PRETTY_FUNCTION__, + inet_ntoa(iph->ip_src), + ospf_get_name(ospf)); + return OSPF_READ_CONTINUE; + } + } + + /* Self-originated packet should be discarded silently. */ + if (ospf_if_lookup_by_local_addr(ospf, NULL, iph->ip_src)) { + if (IS_DEBUG_OSPF_PACKET(0, RECV)) { + zlog_debug( + "ospf_read[%s]: Dropping self-originated packet", + inet_ntoa(iph->ip_src)); + } + return OSPF_READ_CONTINUE; + } + + /* + * Advance from IP header to OSPF header (iph->ip_hl has + * been verified by ospf_recv_packet() to be correct). + */ + stream_forward_getp(ibuf, iph->ip_hl * 4); + + ospfh = (struct ospf_header *)stream_pnt(ibuf); + if (MSG_OK + != ospf_packet_examin(ospfh, stream_get_endp(ibuf) + - stream_get_getp(ibuf))) + return OSPF_READ_CONTINUE; + /* Now it is safe to access all fields of OSPF packet header. */ + + /* associate packet with ospf interface */ + oi = ospf_if_lookup_recv_if(ospf, iph->ip_src, ifp); + + /* + * ospf_verify_header() relies on a valid "oi" and thus can be called + * only after the passive/backbone/other checks below are passed. + * These checks in turn access the fields of unverified "ospfh" + * structure for their own purposes and must remain very accurate + * in doing this. + */ + + /* If incoming interface is passive one, ignore it. */ + if (oi && OSPF_IF_PASSIVE_STATUS(oi) == OSPF_IF_PASSIVE) { + char buf[3][INET_ADDRSTRLEN]; + + if (IS_DEBUG_OSPF_EVENT) + zlog_debug( + "ignoring packet from router %s sent to %s, received on a passive interface, %s", + inet_ntop(AF_INET, &ospfh->router_id, buf[0], + sizeof(buf[0])), + inet_ntop(AF_INET, &iph->ip_dst, buf[1], + sizeof(buf[1])), + inet_ntop(AF_INET, &oi->address->u.prefix4, + buf[2], sizeof(buf[2]))); + + if (iph->ip_dst.s_addr == htonl(OSPF_ALLSPFROUTERS)) { + /* Try to fix multicast membership. + * Some OS:es may have problems in this area, + * make sure it is removed. + */ + OI_MEMBER_JOINED(oi, MEMBER_ALLROUTERS); + ospf_if_set_multicast(oi); + } + return OSPF_READ_CONTINUE; + } + + + /* if no local ospf_interface, + * or header area is backbone but ospf_interface is not + * check for VLINK interface + */ + if ((oi == NULL) + || (OSPF_IS_AREA_ID_BACKBONE(ospfh->area_id) + && !OSPF_IS_AREA_ID_BACKBONE(oi->area->area_id))) { + if ((oi = ospf_associate_packet_vl(ospf, ifp, iph, ospfh)) + == NULL) { + if (!ospf->instance && IS_DEBUG_OSPF_EVENT) + zlog_debug( + "Packet from [%s] received on link %s but no ospf_interface", + inet_ntoa(iph->ip_src), ifp->name); + return OSPF_READ_CONTINUE; + } + } + + /* + * else it must be a local ospf interface, check it was + * received on correct link + */ + else if (oi->ifp != ifp) { + if (IS_DEBUG_OSPF_EVENT) + flog_warn(EC_OSPF_PACKET, + "Packet from [%s] received on wrong link %s", + inet_ntoa(iph->ip_src), ifp->name); + return OSPF_READ_CONTINUE; + } else if (oi->state == ISM_Down) { + char buf[2][INET_ADDRSTRLEN]; + + flog_warn( + EC_OSPF_PACKET, + "Ignoring packet from %s to %s received on interface that is down [%s]; interface flags are %s", + inet_ntop(AF_INET, &iph->ip_src, buf[0], + sizeof(buf[0])), + inet_ntop(AF_INET, &iph->ip_dst, buf[1], + sizeof(buf[1])), + ifp->name, if_flag_dump(ifp->flags)); + /* Fix multicast memberships? */ + if (iph->ip_dst.s_addr == htonl(OSPF_ALLSPFROUTERS)) + OI_MEMBER_JOINED(oi, MEMBER_ALLROUTERS); + else if (iph->ip_dst.s_addr == htonl(OSPF_ALLDROUTERS)) + OI_MEMBER_JOINED(oi, MEMBER_DROUTERS); + if (oi->multicast_memberships) + ospf_if_set_multicast(oi); + return OSPF_READ_CONTINUE; + } + + /* + * If the received packet is destined for AllDRouters, the + * packet should be accepted only if the received ospf + * interface state is either DR or Backup -- endo. + * + * I wonder who endo is? + */ + if (iph->ip_dst.s_addr == htonl(OSPF_ALLDROUTERS) + && (oi->state != ISM_DR && oi->state != ISM_Backup)) { + flog_warn( + EC_OSPF_PACKET, + "Dropping packet for AllDRouters from [%s] via [%s] (ISM: %s)", + inet_ntoa(iph->ip_src), IF_NAME(oi), + lookup_msg(ospf_ism_state_msg, oi->state, NULL)); + /* Try to fix multicast membership. */ + SET_FLAG(oi->multicast_memberships, MEMBER_DROUTERS); + ospf_if_set_multicast(oi); + return OSPF_READ_CONTINUE; + } + + /* Verify more OSPF header fields. */ + ret = ospf_verify_header(ibuf, oi, iph, ospfh); + if (ret < 0) { + if (IS_DEBUG_OSPF_PACKET(0, RECV)) + zlog_debug( + "ospf_read[%s]: Header check failed, " + "dropping.", + inet_ntoa(iph->ip_src)); + return OSPF_READ_CONTINUE; + } + + /* Show debug receiving packet. */ + if (IS_DEBUG_OSPF_PACKET(ospfh->type - 1, RECV)) { + if (IS_DEBUG_OSPF_PACKET(ospfh->type - 1, DETAIL)) { + zlog_debug( + "-----------------------------------------------------"); + ospf_packet_dump(ibuf); + } + + zlog_debug("%s received from [%s] via [%s]", + lookup_msg(ospf_packet_type_str, ospfh->type, NULL), + inet_ntoa(ospfh->router_id), IF_NAME(oi)); + zlog_debug(" src [%s],", inet_ntoa(iph->ip_src)); + zlog_debug(" dst [%s]", inet_ntoa(iph->ip_dst)); + + if (IS_DEBUG_OSPF_PACKET(ospfh->type - 1, DETAIL)) + zlog_debug( + "-----------------------------------------------------"); + } + + stream_forward_getp(ibuf, OSPF_HEADER_SIZE); + + /* Adjust size to message length. */ + length = ntohs(ospfh->length) - OSPF_HEADER_SIZE; + + /* Read rest of the packet and call each sort of packet routine. + */ + switch (ospfh->type) { + case OSPF_MSG_HELLO: + ospf_hello(iph, ospfh, ibuf, oi, length); + break; + case OSPF_MSG_DB_DESC: + ospf_db_desc(iph, ospfh, ibuf, oi, length); + break; + case OSPF_MSG_LS_REQ: + ospf_ls_req(iph, ospfh, ibuf, oi, length); + break; + case OSPF_MSG_LS_UPD: + ospf_ls_upd(ospf, iph, ospfh, ibuf, oi, length); + break; + case OSPF_MSG_LS_ACK: + ospf_ls_ack(iph, ospfh, ibuf, oi, length); + break; + default: + flog_warn( + EC_OSPF_PACKET, + "interface %s(%s): OSPF packet header type %d is illegal", + IF_NAME(oi), ospf_get_name(ospf), ospfh->type); + break; + } + + return OSPF_READ_CONTINUE; +} + +/* Starting point of packet process function. */ +int ospf_read(struct thread *thread) +{ + struct ospf *ospf; int32_t count = 0; + enum ospf_read_return_enum ret; /* first of all get interface pointer. */ ospf = THREAD_ARG(thread); @@ -2959,236 +3198,12 @@ int ospf_read(struct thread *thread) while (count < ospf->write_oi_count) { count++; - stream_reset(ospf->ibuf); - ibuf = ospf_recv_packet(ospf, ospf->fd, &ifp, ospf->ibuf); - if (ibuf == NULL) + ret = ospf_read_helper(ospf); + switch (ret) { + case OSPF_READ_ERROR: return -1; - /* - * This raw packet is known to be at least as big as its - * IP header. - * Note that there should not be alignment problems with - * this assignment because this is at the beginning of the - * stream data buffer. - */ - iph = (struct ip *)STREAM_DATA(ibuf); - /* Note that sockopt_iphdrincl_swab_systoh was called in - * ospf_recv_packet. */ - - if (ifp == NULL) { - /* Handle cases where the platform does not support - * retrieving the ifindex, and also platforms (such - * as Solaris 8) that claim to support ifindex - * retrieval but do not. - */ - c = if_lookup_address((void *)&iph->ip_src, AF_INET, - ospf->vrf_id); - if (c) - ifp = c->ifp; - if (ifp == NULL) { - if (IS_DEBUG_OSPF_PACKET(0, RECV)) - zlog_debug( - "%s: Unable to determine incoming interface from: %s(%s)", - __PRETTY_FUNCTION__, - inet_ntoa(iph->ip_src), - ospf_get_name(ospf)); - continue; - } - } - - /* Self-originated packet should be discarded silently. */ - if (ospf_if_lookup_by_local_addr(ospf, NULL, iph->ip_src)) { - if (IS_DEBUG_OSPF_PACKET(0, RECV)) { - zlog_debug( - "ospf_read[%s]: Dropping self-originated packet", - inet_ntoa(iph->ip_src)); - } - continue; - } - - /* - * Advance from IP header to OSPF header (iph->ip_hl has - * been verified by ospf_recv_packet() to be correct). - */ - stream_forward_getp(ibuf, iph->ip_hl * 4); - - ospfh = (struct ospf_header *)stream_pnt(ibuf); - if (MSG_OK - != ospf_packet_examin(ospfh, - stream_get_endp(ibuf) - - stream_get_getp(ibuf))) - continue; - /* Now it is safe to access all fields of OSPF packet header. */ - - /* associate packet with ospf interface */ - oi = ospf_if_lookup_recv_if(ospf, iph->ip_src, ifp); - - /* ospf_verify_header() relies on a valid "oi" and thus can - * be called only after the passive/backbone/other checks - * below are passed. These checks in turn access the fields - * of unverified "ospfh" structure for their own purposes and - * must remain very accurate in doing this. */ - - /* If incoming interface is passive one, ignore it. */ - if (oi && OSPF_IF_PASSIVE_STATUS(oi) == OSPF_IF_PASSIVE) { - char buf[3][INET_ADDRSTRLEN]; - - if (IS_DEBUG_OSPF_EVENT) - zlog_debug( - "ignoring packet from router %s sent to %s, " - "received on a passive interface, %s", - inet_ntop(AF_INET, &ospfh->router_id, - buf[0], sizeof(buf[0])), - inet_ntop(AF_INET, &iph->ip_dst, buf[1], - sizeof(buf[1])), - inet_ntop(AF_INET, - &oi->address->u.prefix4, - buf[2], sizeof(buf[2]))); - - if (iph->ip_dst.s_addr == htonl(OSPF_ALLSPFROUTERS)) { - /* Try to fix multicast membership. - * Some OS:es may have problems in this area, - * make sure it is removed. - */ - OI_MEMBER_JOINED(oi, MEMBER_ALLROUTERS); - ospf_if_set_multicast(oi); - } - continue; - } - - - /* if no local ospf_interface, - * or header area is backbone but ospf_interface is not - * check for VLINK interface - */ - if ((oi == NULL) - || (OSPF_IS_AREA_ID_BACKBONE(ospfh->area_id) - && !OSPF_IS_AREA_ID_BACKBONE(oi->area->area_id))) { - if ((oi = ospf_associate_packet_vl(ospf, ifp, iph, - ospfh)) - == NULL) { - if (!ospf->instance && IS_DEBUG_OSPF_EVENT) - zlog_debug( - "Packet from [%s] received on link %s" - " but no ospf_interface", - inet_ntoa(iph->ip_src), - ifp->name); - return 0; - } - } - - /* - * else it must be a local ospf interface, check it was - * received on correct link - */ - else if (oi->ifp != ifp) { - if (IS_DEBUG_OSPF_EVENT) - flog_warn( - EC_OSPF_PACKET, - "Packet from [%s] received on wrong link %s", - inet_ntoa(iph->ip_src), ifp->name); - continue; - } else if (oi->state == ISM_Down) { - char buf[2][INET_ADDRSTRLEN]; - - flog_warn( - EC_OSPF_PACKET, - "Ignoring packet from %s to %s received on interface that is " - "down [%s]; interface flags are %s", - inet_ntop(AF_INET, &iph->ip_src, buf[0], - sizeof(buf[0])), - inet_ntop(AF_INET, &iph->ip_dst, buf[1], - sizeof(buf[1])), - ifp->name, if_flag_dump(ifp->flags)); - /* Fix multicast memberships? */ - if (iph->ip_dst.s_addr == htonl(OSPF_ALLSPFROUTERS)) - OI_MEMBER_JOINED(oi, MEMBER_ALLROUTERS); - else if (iph->ip_dst.s_addr == htonl(OSPF_ALLDROUTERS)) - OI_MEMBER_JOINED(oi, MEMBER_DROUTERS); - if (oi->multicast_memberships) - ospf_if_set_multicast(oi); - continue; - } - - /* - * If the received packet is destined for AllDRouters, the - * packet should be accepted only if the received ospf - * interface state is either DR or Backup -- endo. - * - * I wonder who endo is? - */ - if (iph->ip_dst.s_addr == htonl(OSPF_ALLDROUTERS) - && (oi->state != ISM_DR && oi->state != ISM_Backup)) { - flog_warn( - EC_OSPF_PACKET, - "Dropping packet for AllDRouters from [%s] via [%s] (ISM: %s)", - inet_ntoa(iph->ip_src), IF_NAME(oi), - lookup_msg(ospf_ism_state_msg, oi->state, - NULL)); - /* Try to fix multicast membership. */ - SET_FLAG(oi->multicast_memberships, MEMBER_DROUTERS); - ospf_if_set_multicast(oi); - continue; - } - - /* Verify more OSPF header fields. */ - ret = ospf_verify_header(ibuf, oi, iph, ospfh); - if (ret < 0) { - if (IS_DEBUG_OSPF_PACKET(0, RECV)) - zlog_debug( - "ospf_read[%s]: Header check failed, " - "dropping.", - inet_ntoa(iph->ip_src)); - continue; - } - - /* Show debug receiving packet. */ - if (IS_DEBUG_OSPF_PACKET(ospfh->type - 1, RECV)) { - if (IS_DEBUG_OSPF_PACKET(ospfh->type - 1, DETAIL)) { - zlog_debug( - "-----------------------------------------------------"); - ospf_packet_dump(ibuf); - } - - zlog_debug("%s received from [%s] via [%s]", - lookup_msg(ospf_packet_type_str, ospfh->type, - NULL), - inet_ntoa(ospfh->router_id), IF_NAME(oi)); - zlog_debug(" src [%s],", inet_ntoa(iph->ip_src)); - zlog_debug(" dst [%s]", inet_ntoa(iph->ip_dst)); - - if (IS_DEBUG_OSPF_PACKET(ospfh->type - 1, DETAIL)) - zlog_debug( - "-----------------------------------------------------"); - } - - stream_forward_getp(ibuf, OSPF_HEADER_SIZE); - - /* Adjust size to message length. */ - length = ntohs(ospfh->length) - OSPF_HEADER_SIZE; - - /* Read rest of the packet and call each sort of packet routine. - */ - switch (ospfh->type) { - case OSPF_MSG_HELLO: - ospf_hello(iph, ospfh, ibuf, oi, length); break; - case OSPF_MSG_DB_DESC: - ospf_db_desc(iph, ospfh, ibuf, oi, length); - break; - case OSPF_MSG_LS_REQ: - ospf_ls_req(iph, ospfh, ibuf, oi, length); - break; - case OSPF_MSG_LS_UPD: - ospf_ls_upd(ospf, iph, ospfh, ibuf, oi, length); - break; - case OSPF_MSG_LS_ACK: - ospf_ls_ack(iph, ospfh, ibuf, oi, length); - break; - default: - flog_warn( - EC_OSPF_PACKET, - "interface %s(%s): OSPF packet header type %d is illegal", - IF_NAME(oi), ospf_get_name(ospf), ospfh->type); + case OSPF_READ_CONTINUE: break; } }