From ed480148844259b7e9e30ed92489cdf44085457e Mon Sep 17 00:00:00 2001 From: Acee Lindem Date: Fri, 5 Jul 2024 20:44:30 +0000 Subject: [PATCH] ospfd: Fix several problems with direct acknowledgments and improved delay acks. 1. On P2MP interfaces, direct ack would include the same LSA multiple times multiple packets are processed before the OSPF interfae direct LSA acknowledgment event is processed. Now duplicates LSA in the same event are suppressed. 2. On non-broadcast interfaces, direct acks for multiple neighbors would be unicast to the same neighbor due to the multiple OSPF LS Update packets being process prior to the OSPF interface direct ack event. Now, separate direct acks are unicast to the neighbors requiring them. 3. The interface delayed acknowledgment timer runs would run continously (every second as long as the interace is up). Now, the timer is set when delayed acknowledgments are queued and all queued delayed acknowledges are sent when it fires. 4. For non-broadcast interface delayed acknowledgments, the logic to send to multiple neighbors wasn't working because the list was emptied while building the packet for the first neighbor. Signed-off-by: Acee Lindem --- lib/libospf.h | 1 + ospfd/ospf_flood.c | 26 ++++- ospfd/ospf_flood.h | 23 +++- ospfd/ospf_interface.c | 44 +++++--- ospfd/ospf_interface.h | 17 +-- ospfd/ospf_ism.c | 14 +-- ospfd/ospf_packet.c | 243 ++++++++++++++++++++++++++++++----------- ospfd/ospf_packet.h | 5 +- 8 files changed, 265 insertions(+), 108 deletions(-) diff --git a/lib/libospf.h b/lib/libospf.h index f2dc5d61d9eb..8a208beb3c33 100644 --- a/lib/libospf.h +++ b/lib/libospf.h @@ -61,6 +61,7 @@ extern "C" { #define OSPF_RETRANSMIT_WINDOW_DEFAULT 50 /* milliseconds */ #define OSPF_TRANSMIT_DELAY_DEFAULT 1 #define OSPF_DEFAULT_BANDWIDTH 10000 /* Mbps */ +#define OSPF_ACK_DELAY_DEFAULT 1 #define OSPF_DEFAULT_REF_BANDWIDTH 100000 /* Kbps */ diff --git a/ospfd/ospf_flood.c b/ospfd/ospf_flood.c index e9797ce935a7..2af4ae317098 100644 --- a/ospfd/ospf_flood.c +++ b/ospfd/ospf_flood.c @@ -110,6 +110,9 @@ void ospf_area_update_fr_state(struct ospf_area *area) static void ospf_flood_delayed_lsa_ack(struct ospf_neighbor *inbr, struct ospf_lsa *lsa) { + struct ospf_lsa_list_entry *ls_ack_list_entry; + struct ospf_interface *oi = inbr->oi; + /* LSA is more recent than database copy, but was not flooded back out receiving interface. Delayed acknowledgment sent. If interface is in Backup state @@ -122,12 +125,27 @@ static void ospf_flood_delayed_lsa_ack(struct ospf_neighbor *inbr, worked out previously */ /* Deal with router as BDR */ - if (inbr->oi->state == ISM_Backup && !NBR_IS_DR(inbr)) + if (oi->state == ISM_Backup && !NBR_IS_DR(inbr)) return; - /* Schedule a delayed LSA Ack to be sent */ - listnode_add(inbr->oi->ls_ack, - ospf_lsa_lock(lsa)); /* delayed LSA Ack */ + if (IS_DEBUG_OSPF(lsa, LSA_FLOODING)) + zlog_debug("%s:Add LSA[Type%d:%pI4:%pI4]: seq 0x%x age %u NBR %pI4 (%s) ack queue", + __func__, lsa->data->type, &lsa->data->id, + &lsa->data->adv_router, ntohl(lsa->data->ls_seqnum), + ntohs(lsa->data->ls_age), &inbr->router_id, + IF_NAME(inbr->oi)); + + /* Add the LSA to the interface delayed Ack list. */ + ls_ack_list_entry = XCALLOC(MTYPE_OSPF_LSA_LIST, + sizeof(struct ospf_lsa_list_entry)); + ls_ack_list_entry->lsa = ospf_lsa_lock(lsa); + ospf_lsa_list_add_tail(&oi->ls_ack_delayed, ls_ack_list_entry); + + /* Set LS Ack timer if it is not already scheduled. */ + if (!oi->t_ls_ack_delayed) + OSPF_ISM_TIMER_ON(oi->t_ls_ack_delayed, + ospf_ls_ack_delayed_timer, + oi->v_ls_ack_delayed); } /* Check LSA is related to external info. */ diff --git a/ospfd/ospf_flood.h b/ospfd/ospf_flood.h index d9d953735148..241205297000 100644 --- a/ospfd/ospf_flood.h +++ b/ospfd/ospf_flood.h @@ -7,6 +7,8 @@ #ifndef _ZEBRA_OSPF_FLOOD_H #define _ZEBRA_OSPF_FLOOD_H +#include "typesafe.h" + /* * OSPF Temporal LSA List */ @@ -16,14 +18,25 @@ struct ospf_lsa_list_entry { /* Linkage for LSA List */ struct ospf_lsa_list_item list_linkage; - /* - * Time associated with the list entry. For example, for a neigbhor - * link retransmission list, this is the retransmission time. - */ - struct timeval list_entry_time; + union { + /* + * Time associated with the list entry. For example, for a + * neigbhor link retransmission list, this is the + * retransmission time. + */ + struct timeval list_entry_timeval; + + /* + * Destanation address specific to the LSA list. For example, + * the distination for an associated direct LS acknowledgment. + */ + struct in_addr list_entry_dst_addr; + } u; struct ospf_lsa *lsa; }; +#define list_entry_time u.list_entry_timeval +#define list_entry_dst u.list_entry_dst_addr DECLARE_DLIST(ospf_lsa_list, struct ospf_lsa_list_entry, list_linkage); diff --git a/ospfd/ospf_interface.c b/ospfd/ospf_interface.c index 803c36861dcd..c4210eb70c7c 100644 --- a/ospfd/ospf_interface.c +++ b/ospfd/ospf_interface.c @@ -186,10 +186,12 @@ static void ospf_if_default_variables(struct ospf_interface *oi) oi->crypt_seqnum = 0; - /* This must be short, (less than RxmtInterval) - - RFC 2328 Section 13.5 para 3. Set to 1 second to avoid Acks being - held back for too long - MAG */ - oi->v_ls_ack = 1; + /* + * The OSPF LS ACK Delay timer must be less than the LS Retransmision + * timer. As per RFC 2328 Section 13.5 paragraph 3, Set to 1 second + * to avoid Acks being held back for too long + */ + oi->v_ls_ack_delayed = OSPF_ACK_DELAY_DEFAULT; } /* lookup oi for specified prefix/ifp */ @@ -272,9 +274,9 @@ struct ospf_interface *ospf_if_new(struct ospf *ospf, struct interface *ifp, /* Initialize static neighbor list. */ oi->nbr_nbma = list_new(); - /* Initialize Link State Acknowledgment list. */ - oi->ls_ack = list_new(); - oi->ls_ack_direct.ls_ack = list_new(); + /* Initialize Link State Acknowledgment lists. */ + ospf_lsa_list_init(&oi->ls_ack_delayed); + ospf_lsa_list_init(&oi->ls_ack_direct); /* Set default values. */ ospf_if_default_variables(oi); @@ -306,6 +308,22 @@ struct ospf_interface *ospf_if_new(struct ospf *ospf, struct interface *ifp, return oi; } +/* + * Cleanup Interface Ack List + */ +static void ospf_if_cleanup_ack_list(struct ospf_lsa_list_head *ls_ack_list) +{ + struct ospf_lsa_list_entry *ls_ack_list_entry; + struct ospf_lsa *lsa; + + frr_each_safe (ospf_lsa_list, ls_ack_list, ls_ack_list_entry) { + lsa = ls_ack_list_entry->lsa; + ospf_lsa_list_del(ls_ack_list, ls_ack_list_entry); + XFREE(MTYPE_OSPF_LSA_LIST, ls_ack_list_entry); + ospf_lsa_unlock(&lsa); + } +} + /* Restore an interface to its pre UP state Used from ism_interface_down only */ void ospf_if_cleanup(struct ospf_interface *oi) @@ -314,7 +332,6 @@ void ospf_if_cleanup(struct ospf_interface *oi) struct listnode *node, *nnode; struct ospf_neighbor *nbr; struct ospf_nbr_nbma *nbr_nbma; - struct ospf_lsa *lsa; /* oi->nbrs and oi->nbr_nbma should be deleted on InterfaceDown event */ /* delete all static neighbors attached to this interface */ @@ -338,10 +355,9 @@ void ospf_if_cleanup(struct ospf_interface *oi) OSPF_NSM_EVENT_EXECUTE(nbr, NSM_KillNbr); } - /* Cleanup Link State Acknowlegdment list. */ - for (ALL_LIST_ELEMENTS(oi->ls_ack, node, nnode, lsa)) - ospf_lsa_unlock(&lsa); /* oi->ls_ack */ - list_delete_all_node(oi->ls_ack); + /* Cleanup Link State Delayed Acknowlegdment list. */ + ospf_if_cleanup_ack_list(&oi->ls_ack_delayed); + ospf_if_cleanup_ack_list(&oi->ls_ack_direct); oi->crypt_seqnum = 0; @@ -377,8 +393,8 @@ void ospf_if_free(struct ospf_interface *oi) /* Free any lists that should be freed */ list_delete(&oi->nbr_nbma); - list_delete(&oi->ls_ack); - list_delete(&oi->ls_ack_direct.ls_ack); + ospf_if_cleanup_ack_list(&oi->ls_ack_delayed); + ospf_if_cleanup_ack_list(&oi->ls_ack_direct); if (IS_DEBUG_OSPF_EVENT) zlog_debug("%s: ospf interface %s vrf %s id %u deleted", diff --git a/ospfd/ospf_interface.h b/ospfd/ospf_interface.h index a944847b5d9b..78a4fb9e59f9 100644 --- a/ospfd/ospf_interface.h +++ b/ospfd/ospf_interface.h @@ -13,6 +13,7 @@ #include "keychain.h" #include "ospfd/ospf_packet.h" #include "ospfd/ospf_spf.h" +#include #define IF_OSPF_IF_INFO(I) ((struct ospf_if_info *)((I)->info)) #define IF_DEF_PARAMS(I) (IF_OSPF_IF_INFO (I)->def_params) @@ -265,20 +266,20 @@ struct ospf_interface { struct route_table *ls_upd_queue; - struct list *ls_ack; /* Link State Acknowledgment list. */ - - struct { - struct list *ls_ack; - struct in_addr dst; - } ls_ack_direct; + /* + * List of LSAs for delayed and direct link + * state acknowledgment transmission. + */ + struct ospf_lsa_list_head ls_ack_delayed; + struct ospf_lsa_list_head ls_ack_direct; /* Timer values. */ - uint32_t v_ls_ack; /* Delayed Link State Acknowledgment */ + uint32_t v_ls_ack_delayed; /* Delayed Link State Acknowledgment */ /* Threads. */ struct event *t_hello; /* timer */ struct event *t_wait; /* timer */ - struct event *t_ls_ack; /* timer */ + struct event *t_ls_ack_delayed; /* timer */ struct event *t_ls_ack_direct; /* event */ struct event *t_ls_upd_event; /* event */ struct event *t_opaque_lsa_self; /* Type-9 Opaque-LSAs */ diff --git a/ospfd/ospf_ism.c b/ospfd/ospf_ism.c index 878ab725bd32..377e7a6bcc20 100644 --- a/ospfd/ospf_ism.c +++ b/ospfd/ospf_ism.c @@ -285,7 +285,7 @@ static void ism_timer_set(struct ospf_interface *oi) reset also. */ EVENT_OFF(oi->t_hello); EVENT_OFF(oi->t_wait); - EVENT_OFF(oi->t_ls_ack); + EVENT_OFF(oi->t_ls_ack_delayed); EVENT_OFF(oi->gr.hello_delay.t_grace_send); break; case ISM_Loopback: @@ -293,7 +293,7 @@ static void ism_timer_set(struct ospf_interface *oi) unavailable for regular data traffic. */ EVENT_OFF(oi->t_hello); EVENT_OFF(oi->t_wait); - EVENT_OFF(oi->t_ls_ack); + EVENT_OFF(oi->t_ls_ack_delayed); EVENT_OFF(oi->gr.hello_delay.t_grace_send); break; case ISM_Waiting: @@ -304,7 +304,7 @@ static void ism_timer_set(struct ospf_interface *oi) OSPF_ISM_TIMER_MSEC_ON(oi->t_hello, ospf_hello_timer, 1); OSPF_ISM_TIMER_ON(oi->t_wait, ospf_wait_timer, OSPF_IF_PARAM(oi, v_wait)); - EVENT_OFF(oi->t_ls_ack); + EVENT_OFF(oi->t_ls_ack_delayed); break; case ISM_PointToPoint: /* The interface connects to a physical Point-to-point network @@ -314,8 +314,6 @@ static void ism_timer_set(struct ospf_interface *oi) /* send first hello immediately */ OSPF_ISM_TIMER_MSEC_ON(oi->t_hello, ospf_hello_timer, 1); EVENT_OFF(oi->t_wait); - OSPF_ISM_TIMER_ON(oi->t_ls_ack, ospf_ls_ack_timer, - oi->v_ls_ack); break; case ISM_DROther: /* The network type of the interface is broadcast or NBMA @@ -324,8 +322,6 @@ static void ism_timer_set(struct ospf_interface *oi) Backup Designated Router. */ OSPF_HELLO_TIMER_ON(oi); EVENT_OFF(oi->t_wait); - OSPF_ISM_TIMER_ON(oi->t_ls_ack, ospf_ls_ack_timer, - oi->v_ls_ack); break; case ISM_Backup: /* The network type of the interface is broadcast os NBMA @@ -333,8 +329,6 @@ static void ism_timer_set(struct ospf_interface *oi) and the router is Backup Designated Router. */ OSPF_HELLO_TIMER_ON(oi); EVENT_OFF(oi->t_wait); - OSPF_ISM_TIMER_ON(oi->t_ls_ack, ospf_ls_ack_timer, - oi->v_ls_ack); break; case ISM_DR: /* The network type of the interface is broadcast or NBMA @@ -342,8 +336,6 @@ static void ism_timer_set(struct ospf_interface *oi) and the router is Designated Router. */ OSPF_HELLO_TIMER_ON(oi); EVENT_OFF(oi->t_wait); - OSPF_ISM_TIMER_ON(oi->t_ls_ack, ospf_ls_ack_timer, - oi->v_ls_ack); break; } } diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c index 86f877b6214e..2d15a7ecca33 100644 --- a/ospfd/ospf_packet.c +++ b/ospfd/ospf_packet.c @@ -369,19 +369,16 @@ void ospf_ls_rxmt_timer(struct event *thread) ospf_ls_retransmit_set_timer(nbr); } -void ospf_ls_ack_timer(struct event *thread) +void ospf_ls_ack_delayed_timer(struct event *thread) { struct ospf_interface *oi; oi = EVENT_ARG(thread); - oi->t_ls_ack = NULL; + oi->t_ls_ack_delayed = NULL; /* Send Link State Acknowledgment. */ - if (listcount(oi->ls_ack) > 0) + if (ospf_lsa_list_count(&oi->ls_ack_delayed)) ospf_ls_ack_send_delayed(oi); - - /* Set LS Ack timer. */ - OSPF_ISM_TIMER_ON(oi->t_ls_ack, ospf_ls_ack_timer, oi->v_ls_ack); } #ifdef WANT_OSPF_WRITE_FRAGMENT @@ -1820,7 +1817,7 @@ static void ospf_ls_upd(struct ospf *ospf, struct ip *iph, if (IS_LSA_MAXAGE(lsa) && !current && ospf_check_nbr_status(oi->ospf)) { /* (4a) Response Link State Acknowledgment. */ - ospf_ls_ack_send(nbr, lsa); + ospf_ls_ack_send_direct(nbr, lsa); /* (4b) Discard LSA. */ if (IS_DEBUG_OSPF(lsa, LSA)) { @@ -1845,7 +1842,7 @@ static void ospf_ls_upd(struct ospf *ospf, struct ip *iph, if (IS_LSA_MAXAGE(lsa)) { zlog_info("LSA[%s]: Boomerang effect?", dump_lsa_key(lsa)); - ospf_ls_ack_send(nbr, lsa); + ospf_ls_ack_send_direct(nbr, lsa); ospf_lsa_discard(lsa); if (current != NULL && !IS_LSA_MAXAGE(current)) @@ -1879,7 +1876,7 @@ static void ospf_ls_upd(struct ospf *ospf, struct ip *iph, SET_FLAG(lsa->flags, OSPF_LSA_SELF); - ospf_ls_ack_send(nbr, lsa); + ospf_ls_ack_send_direct(nbr, lsa); if (!ospf->gr_info.restart_in_progress) { ospf_opaque_self_originated_lsa_received( @@ -2018,9 +2015,8 @@ static void ospf_ls_upd(struct ospf *ospf, struct ip *iph, */ if (oi->state == ISM_Backup) if (NBR_IS_DR(nbr)) - listnode_add( - oi->ls_ack, - ospf_lsa_lock(lsa)); + ospf_ls_ack_send_direct(nbr, + lsa); DISCARD_LSA(lsa, 6); } else @@ -2029,7 +2025,7 @@ static void ospf_ls_upd(struct ospf *ospf, struct ip *iph, receiving interface. */ { - ospf_ls_ack_send(nbr, lsa); + ospf_ls_ack_send_direct(nbr, lsa); DISCARD_LSA(lsa, 7); } } @@ -3331,17 +3327,36 @@ static int ospf_make_ls_upd(struct ospf_interface *oi, struct list *update, return length; } -static int ospf_make_ls_ack(struct ospf_interface *oi, struct list *ack, - struct stream *s) +static int ospf_make_ls_ack(struct ospf_interface *oi, + struct ospf_lsa_list_head *ls_ack_list, + bool direct_ack, bool delete_ack, struct stream *s) { - struct listnode *node, *nnode; + struct ospf_lsa_list_entry *ls_ack_list_first; + struct ospf_lsa_list_entry *ls_ack_list_entry; uint16_t length = OSPF_LS_ACK_MIN_SIZE; - unsigned long delta = OSPF_LSA_HEADER_SIZE; struct ospf_lsa *lsa; + struct in_addr first_dst_addr; - for (ALL_LIST_ELEMENTS(ack, node, nnode, lsa)) { + /* + * For direct LS Acks, assure the destination address doesn't + * change between queued acknowledgments. + */ + if (direct_ack) { + ls_ack_list_first = ospf_lsa_list_first(ls_ack_list); + if (ls_ack_list_first) + first_dst_addr.s_addr = + ls_ack_list_first->list_entry_dst.s_addr; + } else + first_dst_addr.s_addr = INADDR_ANY; + + frr_each_safe (ospf_lsa_list, ls_ack_list, ls_ack_list_entry) { + lsa = ls_ack_list_entry->lsa; assert(lsa); + if (direct_ack && (ls_ack_list_entry->list_entry_dst.s_addr != + first_dst_addr.s_addr)) + break; + /* LS Ack packet overflows interface MTU * delta is just number of bytes required for * 1 LS Ack(1 LS Hdr) ospf_packet_max will return @@ -3350,19 +3365,46 @@ static int ospf_make_ls_ack(struct ospf_interface *oi, struct list *ack, * against ospf_packet_max to check if it can fit * another ls header in the same packet. */ - if ((length + delta) > ospf_packet_max(oi)) + if ((length + OSPF_LSA_HEADER_SIZE) > ospf_packet_max(oi)) break; stream_put(s, lsa->data, OSPF_LSA_HEADER_SIZE); length += OSPF_LSA_HEADER_SIZE; - listnode_delete(ack, lsa); - ospf_lsa_unlock(&lsa); /* oi->ls_ack_direct.ls_ack */ + if (delete_ack) { + ospf_lsa_list_del(ls_ack_list, ls_ack_list_entry); + XFREE(MTYPE_OSPF_LSA_LIST, ls_ack_list_entry); + ospf_lsa_unlock(&lsa); + } } return length; } +/* + * On non-braodcast networks, the same LS acks must be sent to multiple + * neighbors and deletion must be deferred until after the LS Ack packet + * is sent to all neighbors. + */ +static void ospf_delete_ls_ack_delayed(struct ospf_interface *oi) +{ + struct ospf_lsa_list_entry *ls_ack_list_entry; + struct ospf_lsa *lsa; + uint16_t length = OSPF_LS_ACK_MIN_SIZE; + + frr_each_safe (ospf_lsa_list, &oi->ls_ack_delayed, ls_ack_list_entry) { + lsa = ls_ack_list_entry->lsa; + assert(lsa); + if ((length + OSPF_LSA_HEADER_SIZE) > ospf_packet_max(oi)) + break; + + length += OSPF_LSA_HEADER_SIZE; + ospf_lsa_list_del(&oi->ls_ack_delayed, ls_ack_list_entry); + XFREE(MTYPE_OSPF_LSA_LIST, ls_ack_list_entry); + ospf_lsa_unlock(&lsa); + } +} + static void ospf_hello_send_sub(struct ospf_interface *oi, in_addr_t addr) { struct ospf_packet *op; @@ -3934,10 +3976,13 @@ void ospf_ls_upd_send(struct ospf_neighbor *nbr, struct list *update, int flag, &oi->t_ls_upd_event); } -static void ospf_ls_ack_send_list(struct ospf_interface *oi, struct list *ack, +static void ospf_ls_ack_send_list(struct ospf_interface *oi, + struct ospf_lsa_list_head *ls_ack_list, + bool direct_ack, bool delete_ack, struct in_addr dst) { struct ospf_packet *op; + struct ospf_lsa_list_entry *ls_ack_list_first; uint16_t length = OSPF_HEADER_SIZE; op = ospf_packet_new(oi->ifp->mtu); @@ -3945,8 +3990,18 @@ static void ospf_ls_ack_send_list(struct ospf_interface *oi, struct list *ack, /* Prepare OSPF common header. */ ospf_make_header(OSPF_MSG_LS_ACK, oi, op->s); + /* Determine the destination address - for direct acks, + * the list entries always include the distination address. + */ + if (direct_ack) { + ls_ack_list_first = ospf_lsa_list_first(ls_ack_list); + op->dst.s_addr = ls_ack_list_first->list_entry_dst.s_addr; + } else + op->dst.s_addr = dst.s_addr; + /* Prepare OSPF Link State Acknowledgment body. */ - length += ospf_make_ls_ack(oi, ack, op->s); + length += ospf_make_ls_ack(oi, ls_ack_list, direct_ack, delete_ack, + op->s); /* Fill OSPF header. */ ospf_fill_header(oi, op->s, length); @@ -3954,14 +4009,6 @@ static void ospf_ls_ack_send_list(struct ospf_interface *oi, struct list *ack, /* Set packet length. */ op->length = length; - /* Decide destination address. */ - if (oi->type == OSPF_IFTYPE_POINTOPOINT || - (oi->type == OSPF_IFTYPE_POINTOMULTIPOINT && - !oi->p2mp_non_broadcast)) - op->dst.s_addr = htonl(OSPF_ALLSPFROUTERS); - else - op->dst.s_addr = dst.s_addr; - /* Add packet to the interface output queue. */ ospf_packet_add(oi, op); @@ -3969,34 +4016,96 @@ static void ospf_ls_ack_send_list(struct ospf_interface *oi, struct list *ack, OSPF_ISM_WRITE_ON(oi->ospf); } -static void ospf_ls_ack_send_event(struct event *thread) +static void ospf_ls_ack_send_direct_event(struct event *thread) { struct ospf_interface *oi = EVENT_ARG(thread); + struct in_addr dst = { INADDR_ANY }; oi->t_ls_ack_direct = NULL; - while (listcount(oi->ls_ack_direct.ls_ack)) - ospf_ls_ack_send_list(oi, oi->ls_ack_direct.ls_ack, - oi->ls_ack_direct.dst); + while (ospf_lsa_list_count(&oi->ls_ack_direct)) + ospf_ls_ack_send_list(oi, &(oi->ls_ack_direct), true, true, dst); } -void ospf_ls_ack_send(struct ospf_neighbor *nbr, struct ospf_lsa *lsa) +void ospf_ls_ack_send_direct(struct ospf_neighbor *nbr, struct ospf_lsa *lsa) { + struct ospf_lsa_list_entry *ls_ack_list_entry; struct ospf_interface *oi = nbr->oi; + if (IS_DEBUG_OSPF(lsa, LSA_FLOODING)) + zlog_debug("%s:Add LSA[Type%d:%pI4:%pI4]: seq 0x%x age %u NBR %pI4 (%s) ack queue", + __func__, lsa->data->type, &lsa->data->id, + &lsa->data->adv_router, ntohl(lsa->data->ls_seqnum), + ntohs(lsa->data->ls_age), &nbr->router_id, + IF_NAME(nbr->oi)); + + /* + * On Point-to-Multipoint broadcast-capabile interfaces, + * where direct acks from are sent to the ALLSPFRouters + * address and one direct ack send event, may include LSAs + * from multiple neighbors, there is a possibility of the same + * LSA being processed more than once in the same send event. + * In this case, the instances subsequent to the first can be + * ignored. + */ + if (oi->type == OSPF_IFTYPE_POINTOMULTIPOINT && !oi->p2mp_non_broadcast) { + struct ospf_lsa_list_entry *ls_ack_list_entry; + struct ospf_lsa *ack_queue_lsa; + + frr_each (ospf_lsa_list, &oi->ls_ack_direct, ls_ack_list_entry) { + ack_queue_lsa = ls_ack_list_entry->lsa; + if ((lsa == ack_queue_lsa) || + ((lsa->data->type == ack_queue_lsa->data->type) && + (lsa->data->id.s_addr == + ack_queue_lsa->data->id.s_addr) && + (lsa->data->adv_router.s_addr == + ack_queue_lsa->data->adv_router.s_addr) && + (lsa->data->ls_seqnum == + ack_queue_lsa->data->ls_seqnum))) { + if (IS_DEBUG_OSPF(lsa, LSA_FLOODING)) + zlog_debug("%s:LSA[Type%d:%pI4:%pI4]: seq 0x%x age %u NBR %pI4 (%s) ack queue duplicate", + __func__, lsa->data->type, + &lsa->data->id, + &lsa->data->adv_router, + ntohl(lsa->data->ls_seqnum), + ntohs(lsa->data->ls_age), + &nbr->router_id, + IF_NAME(nbr->oi)); + return; + } + } + } + if (IS_GRACE_LSA(lsa)) { if (IS_DEBUG_OSPF_GR) zlog_debug("%s, Sending GRACE ACK to Restarter.", __func__); } - if (listcount(oi->ls_ack_direct.ls_ack) == 0) - oi->ls_ack_direct.dst = nbr->address.u.prefix4; + ls_ack_list_entry = XCALLOC(MTYPE_OSPF_LSA_LIST, + sizeof(struct ospf_lsa_list_entry)); - listnode_add(oi->ls_ack_direct.ls_ack, ospf_lsa_lock(lsa)); + /* + * Determine the destination address - Direct LS acknowledgments + * are sent the AllSPFRouters multicast address on Point-to-Point + * and Point-to-Multipoint broadcast-capable interfaces. For all other + * interface types, they are unicast directly to the neighbor. + */ + if (oi->type == OSPF_IFTYPE_POINTOPOINT || + (oi->type == OSPF_IFTYPE_POINTOMULTIPOINT && + !oi->p2mp_non_broadcast)) + ls_ack_list_entry->list_entry_dst.s_addr = + htonl(OSPF_ALLSPFROUTERS); + else + ls_ack_list_entry->list_entry_dst.s_addr = + nbr->address.u.prefix4.s_addr; - event_add_event(master, ospf_ls_ack_send_event, oi, 0, - &oi->t_ls_ack_direct); + ls_ack_list_entry->lsa = ospf_lsa_lock(lsa); + ospf_lsa_list_add_tail(&nbr->oi->ls_ack_direct, ls_ack_list_entry); + + if (oi->t_ls_ack_direct == NULL) + event_add_event(master, ospf_ls_ack_send_direct_event, oi, 0, + &oi->t_ls_ack_direct); } /* Send Link State Acknowledgment delayed. */ @@ -4013,33 +4122,39 @@ void ospf_ls_ack_send_delayed(struct ospf_interface *oi) struct ospf_neighbor *nbr; struct route_node *rn; - for (rn = route_top(oi->nbrs); rn; rn = route_next(rn)) { - nbr = rn->info; + while (ospf_lsa_list_count(&oi->ls_ack_delayed)) { + for (rn = route_top(oi->nbrs); rn; rn = route_next(rn)) { + nbr = rn->info; - if (!nbr) - continue; + if (!nbr) + continue; - if (nbr != oi->nbr_self && nbr->state >= NSM_Exchange) - while (listcount(oi->ls_ack)) - ospf_ls_ack_send_list( - oi, oi->ls_ack, - nbr->address.u.prefix4); + if (nbr != oi->nbr_self && + nbr->state >= NSM_Exchange) + ospf_ls_ack_send_list(oi, + &oi->ls_ack_delayed, + false, false, + nbr->address.u + .prefix4); + } + ospf_delete_ls_ack_delayed(oi); } - return; - } - if (oi->type == OSPF_IFTYPE_VIRTUALLINK) - dst.s_addr = oi->vl_data->peer_addr.s_addr; - else if (oi->state == ISM_DR || oi->state == ISM_Backup) - dst.s_addr = htonl(OSPF_ALLSPFROUTERS); - else if (oi->type == OSPF_IFTYPE_POINTOPOINT) - dst.s_addr = htonl(OSPF_ALLSPFROUTERS); - else if (oi->type == OSPF_IFTYPE_POINTOMULTIPOINT) - dst.s_addr = htonl(OSPF_ALLSPFROUTERS); - else - dst.s_addr = htonl(OSPF_ALLDROUTERS); + } else { + if (oi->type == OSPF_IFTYPE_VIRTUALLINK) + dst.s_addr = oi->vl_data->peer_addr.s_addr; + else if (oi->state == ISM_DR || oi->state == ISM_Backup) + dst.s_addr = htonl(OSPF_ALLSPFROUTERS); + else if (oi->type == OSPF_IFTYPE_POINTOPOINT) + dst.s_addr = htonl(OSPF_ALLSPFROUTERS); + else if (oi->type == OSPF_IFTYPE_POINTOMULTIPOINT) + dst.s_addr = htonl(OSPF_ALLSPFROUTERS); + else + dst.s_addr = htonl(OSPF_ALLDROUTERS); - while (listcount(oi->ls_ack)) - ospf_ls_ack_send_list(oi, oi->ls_ack, dst); + while (ospf_lsa_list_count(&oi->ls_ack_delayed)) + ospf_ls_ack_send_list(oi, &oi->ls_ack_delayed, false, + true, dst); + } } /* diff --git a/ospfd/ospf_packet.h b/ospfd/ospf_packet.h index 2c9dba6c8819..84e4b027e694 100644 --- a/ospfd/ospf_packet.h +++ b/ospfd/ospf_packet.h @@ -135,13 +135,14 @@ extern void ospf_ls_upd_send(struct ospf_neighbor *, struct list *, int, int); extern void ospf_ls_upd_queue_send(struct ospf_interface *oi, struct list *update, struct in_addr addr, int send_lsupd_now); -extern void ospf_ls_ack_send(struct ospf_neighbor *, struct ospf_lsa *); +extern void ospf_ls_ack_send_direct(struct ospf_neighbor *nbr, + struct ospf_lsa *lsa); extern void ospf_ls_ack_send_delayed(struct ospf_interface *); extern void ospf_ls_retransmit(struct ospf_interface *, struct ospf_lsa *); extern void ospf_ls_req_event(struct ospf_neighbor *); extern void ospf_ls_rxmt_timer(struct event *thread); -extern void ospf_ls_ack_timer(struct event *thread); +extern void ospf_ls_ack_delayed_timer(struct event *thread); extern void ospf_poll_timer(struct event *thread); extern void ospf_hello_reply_timer(struct event *thread);