Merge pull request #4907 from donaldsharp/ospf_write_q

ospfd: Do not turn on write thread unless we have something in it

ospfd/ospf_interface.c

@@ -225,12 +225,14 @@ struct ospf_interface *ospf_if_new(struct ospf *ospf, struct interface *ifp,
 {
 	struct ospf_interface *oi;
 
-	if ((oi = ospf_if_table_lookup(ifp, p)) == NULL) {
+	oi = ospf_if_table_lookup(ifp, p);
-		oi = XCALLOC(MTYPE_OSPF_IF, sizeof(struct ospf_interface));
+	if (oi)
-		memset(oi, 0, sizeof(struct ospf_interface));
-	} else
 		return oi;
 
+	oi = XCALLOC(MTYPE_OSPF_IF, sizeof(struct ospf_interface));
+
+	oi->obuf = ospf_fifo_new();
+
 	/* Set zebra interface pointer. */
 	oi->ifp = ifp;
 	oi->address = p;
@@ -264,8 +266,6 @@ struct ospf_interface *ospf_if_new(struct ospf *ospf, struct interface *ifp,
 
 	oi->ospf = ospf;
 
-	ospf_if_stream_set(oi);
-
 	QOBJ_REG(oi, ospf_interface);
 
 	if (IS_DEBUG_OSPF_EVENT)
@@ -325,8 +325,7 @@ void ospf_if_free(struct ospf_interface *oi)
 {
 	ospf_if_down(oi);
 
-	if (oi->obuf)
+	ospf_fifo_free(oi->obuf);
-		ospf_fifo_free(oi->obuf);
 
 	assert(oi->state == ISM_Down);
 
@@ -490,29 +489,20 @@ static void ospf_if_reset_stats(struct ospf_interface *oi)
 	oi->ls_ack_in = oi->ls_ack_out = 0;
 }
 
-void ospf_if_stream_set(struct ospf_interface *oi)
-{
-	/* set output fifo queue. */
-	if (oi->obuf == NULL)
-		oi->obuf = ospf_fifo_new();
-}
-
 void ospf_if_stream_unset(struct ospf_interface *oi)
 {
 	struct ospf *ospf = oi->ospf;
 
-	if (oi->obuf) {
+	/* flush the interface packet queue */
-		/* flush the interface packet queue */
+	ospf_fifo_flush(oi->obuf);
-		ospf_fifo_flush(oi->obuf);
+	/*reset protocol stats */
-		/*reset protocol stats */
+	ospf_if_reset_stats(oi);
-		ospf_if_reset_stats(oi);
 
-		if (oi->on_write_q) {
+	if (oi->on_write_q) {
-			listnode_delete(ospf->oi_write_q, oi);
+		listnode_delete(ospf->oi_write_q, oi);
-			if (list_isempty(ospf->oi_write_q))
+		if (list_isempty(ospf->oi_write_q))
-				OSPF_TIMER_OFF(ospf->t_write);
+			OSPF_TIMER_OFF(ospf->t_write);
-			oi->on_write_q = 0;
+		oi->on_write_q = 0;
-		}
 	}
 }
 
@@ -903,8 +893,6 @@ struct ospf_interface *ospf_vl_new(struct ospf *ospf,
 
 	ospf_area_add_if(voi->area, voi);
 
-	ospf_if_stream_set(voi);
-
 	if (IS_DEBUG_OSPF_EVENT)
 		zlog_debug("ospf_vl_new(): Stop");
 	return voi;

ospfd/ospf_interface.h

@@ -285,7 +285,6 @@ extern void ospf_if_update_params(struct interface *, struct in_addr);
 
 extern int ospf_if_new_hook(struct interface *);
 extern void ospf_if_init(void);
-extern void ospf_if_stream_set(struct ospf_interface *);
 extern void ospf_if_stream_unset(struct ospf_interface *);
 extern void ospf_if_reset_variables(struct ospf_interface *);
 extern int ospf_if_is_enable(struct ospf_interface *);

ospfd/ospf_ism.h

@@ -53,8 +53,9 @@
 			listnode_add((O)->oi_write_q, oi);                     \
 			oi->on_write_q = 1;                                    \
 		}                                                              \
-		thread_add_write(master, ospf_write, (O), (O)->fd,             \
+		if (!list_isempty((O)->oi_write_q))                            \
-				 &(O)->t_write);                               \
+			thread_add_write(master, ospf_write, (O), (O)->fd,     \
+					 &(O)->t_write);                       \
 	} while (0)
 
 /* Macro for OSPF ISM timer turn on. */

ospfd/ospf_packet.c

@@ -132,7 +132,7 @@ static int ospf_auth_type(struct ospf_interface *oi)
 	return auth_type;
 }
 
-struct ospf_packet *ospf_packet_new(size_t size)
+static struct ospf_packet *ospf_packet_new(size_t size)
 {
 	struct ospf_packet *new;
 
@@ -231,22 +231,8 @@ void ospf_fifo_free(struct ospf_fifo *fifo)
 	XFREE(MTYPE_OSPF_FIFO, fifo);
 }
 
-void ospf_packet_add(struct ospf_interface *oi, struct ospf_packet *op)
+static void ospf_packet_add(struct ospf_interface *oi, struct ospf_packet *op)
 {
-	if (!oi->obuf) {
-		flog_err(
-			EC_OSPF_PKT_PROCESS,
-			"ospf_packet_add(interface %s in state %d [%s], packet type %s, "
-			"destination %s) called with NULL obuf, ignoring "
-			"(please report this bug)!\n",
-			IF_NAME(oi), oi->state,
-			lookup_msg(ospf_ism_state_msg, oi->state, NULL),
-			lookup_msg(ospf_packet_type_str,
-				   stream_getc_from(op->s, 1), NULL),
-			inet_ntoa(op->dst));
-		return;
-	}
-
 	/* Add packet to end of queue. */
 	ospf_fifo_push(oi->obuf, op);
 
@@ -257,20 +243,6 @@ void ospf_packet_add(struct ospf_interface *oi, struct ospf_packet *op)
 static void ospf_packet_add_top(struct ospf_interface *oi,
 				struct ospf_packet *op)
 {
-	if (!oi->obuf) {
-		flog_err(
-			EC_OSPF_PKT_PROCESS,
-			"ospf_packet_add(interface %s in state %d [%s], packet type %s, "
-			"destination %s) called with NULL obuf, ignoring "
-			"(please report this bug)!\n",
-			IF_NAME(oi), oi->state,
-			lookup_msg(ospf_ism_state_msg, oi->state, NULL),
-			lookup_msg(ospf_packet_type_str,
-				   stream_getc_from(op->s, 1), NULL),
-			inet_ntoa(op->dst));
-		return;
-	}
-
 	/* Add packet to head of queue. */
 	ospf_fifo_push_head(oi->obuf, op);
 
@@ -278,7 +250,7 @@ static void ospf_packet_add_top(struct ospf_interface *oi,
 	/* ospf_fifo_debug (oi->obuf); */
 }
 
-void ospf_packet_delete(struct ospf_interface *oi)
+static void ospf_packet_delete(struct ospf_interface *oi)
 {
 	struct ospf_packet *op;
 
@@ -288,7 +260,7 @@ void ospf_packet_delete(struct ospf_interface *oi)
 		ospf_packet_free(op);
 }
 
-struct ospf_packet *ospf_packet_dup(struct ospf_packet *op)
+static struct ospf_packet *ospf_packet_dup(struct ospf_packet *op)
 {
 	struct ospf_packet *new;
 
@@ -698,12 +670,9 @@ static int ospf_write(struct thread *thread)
 		return -1;
 	}
 
-	ospf->t_write = NULL;
-
 	node = listhead(ospf->oi_write_q);
 	assert(node);
 	oi = listgetdata(node);
-	assert(oi);
 
 #ifdef WANT_OSPF_WRITE_FRAGMENT
 	/* seed ipid static with low order bits of time */
@@ -906,9 +875,7 @@ static int ospf_write(struct thread *thread)
 		/* Setup to service from the head of the queue again */
 		if (!list_isempty(ospf->oi_write_q)) {
 			node = listhead(ospf->oi_write_q);
-			assert(node);
 			oi = listgetdata(node);
-			assert(oi);
 		}
 	}
 
@@ -4062,6 +4029,23 @@ static void ospf_ls_upd_queue_send(struct ospf_interface *oi,
 			oi->on_write_q = 1;
 		}
 		ospf_write(&os_packet_thd);
+		/*
+		 * We are fake calling ospf_write with a fake
+		 * thread.  Imagine that we have oi_a already
+		 * enqueued and we have turned on the write
+		 * thread(t_write).
+		 * Now this function calls this for oi_b
+		 * so the on_write_q has oi_a and oi_b on
+		 * it, ospf_write runs and clears the packets
+		 * for both oi_a and oi_b.  Removing them from
+		 * the on_write_q.  After this thread of execution
+		 * finishes we will execute the t_write thread
+		 * with nothing in the on_write_q causing an
+		 * assert.  So just make sure that the t_write
+		 * is actually turned off.
+		 */
+		if (list_isempty(oi->ospf->oi_write_q))
+			OSPF_TIMER_OFF(oi->ospf->t_write);
 	} else {
 		/* Hook thread to write packet. */
 		OSPF_ISM_WRITE_ON(oi->ospf);

ospfd/ospf_packet.h

@@ -131,8 +131,6 @@ struct ospf_ls_update {
 #define IS_SET_DD_ALL(X)        ((X) & OSPF_DD_FLAG_ALL)
 
 /* Prototypes. */
-extern void ospf_output_forward(struct stream *, int);
-extern struct ospf_packet *ospf_packet_new(size_t);
 extern void ospf_packet_free(struct ospf_packet *);
 extern struct ospf_fifo *ospf_fifo_new(void);
 extern void ospf_fifo_push(struct ospf_fifo *, struct ospf_packet *);
@@ -140,10 +138,6 @@ extern struct ospf_packet *ospf_fifo_pop(struct ospf_fifo *);
 extern struct ospf_packet *ospf_fifo_head(struct ospf_fifo *);
 extern void ospf_fifo_flush(struct ospf_fifo *);
 extern void ospf_fifo_free(struct ospf_fifo *);
-extern void ospf_packet_add(struct ospf_interface *, struct ospf_packet *);
-extern void ospf_packet_delete(struct ospf_interface *);
-extern struct stream *ospf_stream_dup(struct stream *);
-extern struct ospf_packet *ospf_packet_dup(struct ospf_packet *);
 
 extern int ospf_read(struct thread *);
 extern void ospf_hello_send(struct ospf_interface *);