1 files changed, 145 insertions, 102 deletions

diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index b7f97ac612..7e4ac1ddd2 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -1871,31 +1871,6 @@ static void rib_process_after(struct zebra_dplane_ctx *ctx)
 			   dplane_ctx_get_vrf(ctx), dest_str, ctx,
 			   dplane_op2str(op), dplane_res2str(status));
 
-	if (op == DPLANE_OP_ROUTE_DELETE) {
-		/*
-		 * In the delete case, the zebra core datastructs were
-		 * updated (or removed) at the time the delete was issued,
-		 * so we're just notifying the route owner.
-		 */
-		if (status == ZEBRA_DPLANE_REQUEST_SUCCESS) {
-			zsend_route_notify_owner_ctx(ctx, ZAPI_ROUTE_REMOVED);
-
-			if (zvrf)
-				zvrf->removals++;
-		} else {
-			zsend_route_notify_owner_ctx(ctx,
-						     ZAPI_ROUTE_REMOVE_FAIL);
-
-			zlog_warn("%u:%s: Route Deletion failure",
-				  dplane_ctx_get_vrf(ctx),
-				  prefix2str(dest_pfx,
-					     dest_str, sizeof(dest_str)));
-		}
-
-		/* Nothing more to do in delete case */
-		goto done;
-	}
-
 	/*
 	 * Update is a bit of a special case, where we may have both old and new
 	 * routes to post-process.
@@ -1955,59 +1930,92 @@ static void rib_process_after(struct zebra_dplane_ctx *ctx)
 		goto done;
 	}
 
-	if (status == ZEBRA_DPLANE_REQUEST_SUCCESS) {
-		/* Update zebra nexthop FIB flag for each
-		 * nexthop that was installed.
-		 */
-		for (ALL_NEXTHOPS_PTR(dplane_ctx_get_ng(ctx), ctx_nexthop)) {
+	switch (op) {
+	case DPLANE_OP_NONE:
+		break;
+	case DPLANE_OP_ROUTE_INSTALL:
+	case DPLANE_OP_ROUTE_UPDATE:
+		if (status == ZEBRA_DPLANE_REQUEST_SUCCESS) {
+			/* Update zebra nexthop FIB flag for each
+			 * nexthop that was installed.
+			 */
+			for (ALL_NEXTHOPS_PTR(dplane_ctx_get_ng(ctx),
+					      ctx_nexthop)) {
 
-			for (ALL_NEXTHOPS(re->ng, nexthop)) {
-				if (nexthop_same(ctx_nexthop, nexthop))
-					break;
+				for (ALL_NEXTHOPS(re->ng, nexthop)) {
+					if (nexthop_same(ctx_nexthop, nexthop))
+						break;
+				}
+
+				if (nexthop == NULL)
+					continue;
+
+				if (CHECK_FLAG(nexthop->flags,
+					       NEXTHOP_FLAG_RECURSIVE))
+					continue;
+
+				if (CHECK_FLAG(ctx_nexthop->flags,
+					       NEXTHOP_FLAG_FIB))
+					SET_FLAG(nexthop->flags,
+						 NEXTHOP_FLAG_FIB);
+				else
+					UNSET_FLAG(nexthop->flags,
+						   NEXTHOP_FLAG_FIB);
 			}
 
-			if (nexthop == NULL)
-				continue;
+			if (zvrf) {
+				zvrf->installs++;
+				/* Set flag for nexthop tracking processing */
+				zvrf->flags |= ZEBRA_VRF_RIB_SCHEDULED;
+			}
 
-			if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE))
-				continue;
+			/* Redistribute */
+			/*
+			 * TODO -- still calling the redist api using the
+			 * route_entries, and there's a corner-case here:
+			 * if there's no client for the 'new' route, a redist
+			 * deleting the 'old' route will be sent. But if the
+			 * 'old' context info was stale, 'old_re' will be
+			 * NULL here and that delete will not be sent.
+			 */
+			redistribute_update(dest_pfx, src_pfx, re, old_re);
 
-			if (CHECK_FLAG(ctx_nexthop->flags,
-				       NEXTHOP_FLAG_FIB))
-				SET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB);
-			else
-				UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB);
-		}
+			/* Notify route owner */
+			zsend_route_notify_owner(re, dest_pfx,
+						 ZAPI_ROUTE_INSTALLED);
 
-		if (zvrf) {
-			zvrf->installs++;
-			/* Set flag for nexthop tracking processing */
-			zvrf->flags |= ZEBRA_VRF_RIB_SCHEDULED;
-		}
+		} else {
+			zsend_route_notify_owner(re, dest_pfx,
+						 ZAPI_ROUTE_FAIL_INSTALL);
 
-		/* Redistribute */
-		/* TODO -- still calling the redist api using the route_entries,
-		 * and there's a corner-case here: if there's no client
-		 * for the 'new' route, a redist deleting the 'old' route
-		 * will be sent. But if the 'old' context info was stale,
-		 * 'old_re' will be NULL here and that delete will not be sent.
+			zlog_warn("%u:%s: Route install failed",
+				  dplane_ctx_get_vrf(ctx),
+				  prefix2str(dest_pfx,
+					     dest_str, sizeof(dest_str)));
+		}
+		break;
+	case DPLANE_OP_ROUTE_DELETE:
+		/*
+		 * In the delete case, the zebra core datastructs were
+		 * updated (or removed) at the time the delete was issued,
+		 * so we're just notifying the route owner.
 		 */
-		redistribute_update(dest_pfx, src_pfx, re, old_re);
-
-		/* Notify route owner */
-		zsend_route_notify_owner(re,
-					 dest_pfx, ZAPI_ROUTE_INSTALLED);
+		if (status == ZEBRA_DPLANE_REQUEST_SUCCESS) {
+			zsend_route_notify_owner_ctx(ctx, ZAPI_ROUTE_REMOVED);
 
-	} else {
-		zsend_route_notify_owner(re, dest_pfx,
-					 ZAPI_ROUTE_FAIL_INSTALL);
+			if (zvrf)
+				zvrf->removals++;
+		} else {
+			zsend_route_notify_owner_ctx(ctx,
+						     ZAPI_ROUTE_REMOVE_FAIL);
 
-		zlog_warn("%u:%s: Route install failed",
-			  dplane_ctx_get_vrf(ctx),
-			  prefix2str(dest_pfx,
-				     dest_str, sizeof(dest_str)));
+			zlog_warn("%u:%s: Route Deletion failure",
+				  dplane_ctx_get_vrf(ctx),
+				  prefix2str(dest_pfx,
+					     dest_str, sizeof(dest_str)));
+		}
+		break;
 	}
-
 done:
 
 	/* Return context to dataplane module */
@@ -2144,43 +2152,66 @@ static wq_item_status meta_queue_process(struct work_queue *dummy, void *data)
 	return mq->size ? WQ_REQUEUE : WQ_SUCCESS;
 }
 
-/* Look into the RN and queue it into one or more priority queues,
- * increasing the size for each data push done.
+
+/*
+ * Look into the RN and queue it into the highest priority queue
+ * at this point in time for processing.
+ *
+ * We will enqueue a route node only once per invocation.
+ *
+ * There are two possibilities here that should be kept in mind.
+ * If the original invocation has not been pulled off for processing
+ * yet, A subsuquent invocation can have a route entry with a better
+ * meta queue index value and we can have a situation where
+ * we might have the same node enqueued 2 times.  Not necessarily
+ * an optimal situation but it should be ok.
+ *
+ * The other possibility is that the original invocation has not
+ * been pulled off for processing yet, A subsusquent invocation
+ * doesn't have a route_entry with a better meta-queue and the
+ * original metaqueue index value will win and we'll end up with
+ * the route node enqueued once.
  */
 static void rib_meta_queue_add(struct meta_queue *mq, struct route_node *rn)
 {
-	struct route_entry *re;
+	struct route_entry *re = NULL, *curr_re = NULL;
+	uint8_t qindex = MQ_SIZE, curr_qindex = MQ_SIZE;
+	struct zebra_vrf *zvrf;
 
-	RNODE_FOREACH_RE (rn, re) {
-		uint8_t qindex = route_info[re->type].meta_q_map;
-		struct zebra_vrf *zvrf;
+	RNODE_FOREACH_RE (rn, curr_re) {
+		curr_qindex = route_info[curr_re->type].meta_q_map;
 
-		/* Invariant: at this point we always have rn->info set. */
-		if (CHECK_FLAG(rib_dest_from_rnode(rn)->flags,
-			       RIB_ROUTE_QUEUED(qindex))) {
-			if (IS_ZEBRA_DEBUG_RIB_DETAILED)
-				rnode_debug(
-					rn, re->vrf_id,
-					"rn %p is already queued in sub-queue %u",
-					(void *)rn, qindex);
-			continue;
+		if (curr_qindex <= qindex) {
+			re = curr_re;
+			qindex = curr_qindex;
 		}
+	}
 
-		SET_FLAG(rib_dest_from_rnode(rn)->flags,
-			 RIB_ROUTE_QUEUED(qindex));
-		listnode_add(mq->subq[qindex], rn);
-		route_lock_node(rn);
-		mq->size++;
+	if (!re)
+		return;
 
+	/* Invariant: at this point we always have rn->info set. */
+	if (CHECK_FLAG(rib_dest_from_rnode(rn)->flags,
+		       RIB_ROUTE_QUEUED(qindex))) {
 		if (IS_ZEBRA_DEBUG_RIB_DETAILED)
 			rnode_debug(rn, re->vrf_id,
-				    "queued rn %p into sub-queue %u",
+				    "rn %p is already queued in sub-queue %u",
 				    (void *)rn, qindex);
-
-		zvrf = zebra_vrf_lookup_by_id(re->vrf_id);
-		if (zvrf)
-			zvrf->flags |= ZEBRA_VRF_RIB_SCHEDULED;
+		return;
 	}
+
+	SET_FLAG(rib_dest_from_rnode(rn)->flags, RIB_ROUTE_QUEUED(qindex));
+	listnode_add(mq->subq[qindex], rn);
+	route_lock_node(rn);
+	mq->size++;
+
+	if (IS_ZEBRA_DEBUG_RIB_DETAILED)
+		rnode_debug(rn, re->vrf_id, "queued rn %p into sub-queue %u",
+			    (void *)rn, qindex);
+
+	zvrf = zebra_vrf_lookup_by_id(re->vrf_id);
+	if (zvrf)
+		zvrf->flags |= ZEBRA_VRF_RIB_SCHEDULED;
 }
 
 /* Add route_node to work queue and schedule processing */
@@ -3165,21 +3196,34 @@ void rib_close_table(struct route_table *table)
 static int rib_process_dplane_results(struct thread *thread)
 {
 	struct zebra_dplane_ctx *ctx;
+	struct dplane_ctx_q ctxlist;
+
+	/* Dequeue a list of completed updates with one lock/unlock cycle */
 
 	do {
+		TAILQ_INIT(&ctxlist);
+
 		/* Take lock controlling queue of results */
 		pthread_mutex_lock(&dplane_mutex);
 		{
 			/* Dequeue context block */
-			ctx = dplane_ctx_dequeue(&rib_dplane_q);
+			dplane_ctx_list_append(&ctxlist, &rib_dplane_q);
 		}
 		pthread_mutex_unlock(&dplane_mutex);
 
-		if (ctx)
-			rib_process_after(ctx);
-		else
+		/* Dequeue context block */
+		ctx = dplane_ctx_dequeue(&ctxlist);
+
+		/* If we've emptied the results queue, we're done */
+		if (ctx == NULL)
 			break;
 
+		while (ctx) {
+			rib_process_after(ctx);
+
+			ctx = dplane_ctx_dequeue(&ctxlist);
+		}
+
 	} while (1);
 
 	/* Check for nexthop tracking processing after finishing with results */
@@ -3193,17 +3237,17 @@ static int rib_process_dplane_results(struct thread *thread)
  * the dataplane pthread. We enqueue the results here for processing by
  * the main thread later.
  */
-static int rib_dplane_results(struct zebra_dplane_ctx *ctx)
+static int rib_dplane_results(struct dplane_ctx_q *ctxlist)
 {
 	/* Take lock controlling queue of results */
 	pthread_mutex_lock(&dplane_mutex);
 	{
-		/* Enqueue context block */
-		dplane_ctx_enqueue_tail(&rib_dplane_q, ctx);
+		/* Enqueue context blocks */
+		dplane_ctx_list_append(&rib_dplane_q, ctxlist);
 	}
 	pthread_mutex_unlock(&dplane_mutex);
 
-	/* Ensure event is signalled to zebra main thread */
+	/* Ensure event is signalled to zebra main pthread */
 	thread_add_event(zebrad.master, rib_process_dplane_results, NULL, 0,
 			 &t_dplane);
 
@@ -3218,8 +3262,7 @@ void rib_init(void)
 	/* Init dataplane, and register for results */
 	pthread_mutex_init(&dplane_mutex, NULL);
 	TAILQ_INIT(&rib_dplane_q);
-	zebra_dplane_init();
-	dplane_results_register(rib_dplane_results);
+	zebra_dplane_init(rib_dplane_results);
 }
 
 /*