ospfd: Fix several problems with direct and delayed acknowledgments

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 */
 

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. */

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);
 

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",

ospfd/ospf_interface.h

@@ -13,6 +13,7 @@
 #include "keychain.h"
 #include "ospfd/ospf_packet.h"
 #include "ospfd/ospf_spf.h"
+#include <ospfd/ospf_flood.h>
 
 #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 */

ospfd/ospf_ism.c

@@ -285,15 +285,15 @@ 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:
 		/* In this state, the interface may be looped back and will be
 		   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,26 +322,20 @@ 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
 		   network,
 		   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
 		   network,
 		   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;
 	}
 }