aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--src/vlib/main.c40
1 files changed, 27 insertions, 13 deletions
diff --git a/src/vlib/main.c b/src/vlib/main.c
index 0e6d66cd933..14f680e61fc 100644
--- a/src/vlib/main.c
+++ b/src/vlib/main.c
@@ -1112,14 +1112,18 @@ dispatch_node (vlib_main_t * vm,
}
static u64
-dispatch_pending_node (vlib_main_t * vm,
- vlib_pending_frame_t * p, u64 last_time_stamp)
+dispatch_pending_node (vlib_main_t * vm, uword pending_frame_index,
+ u64 last_time_stamp)
{
vlib_node_main_t *nm = &vm->node_main;
vlib_frame_t *f;
vlib_next_frame_t *nf, nf_dummy;
vlib_node_runtime_t *n;
u32 restore_frame_index;
+ vlib_pending_frame_t *p;
+
+ /* See comment below about dangling references to nm->pending_frames */
+ p = nm->pending_frames + pending_frame_index;
n = vec_elt_at_index (nm->nodes_by_type[VLIB_NODE_TYPE_INTERNAL],
p->node_runtime_index);
@@ -1169,18 +1173,29 @@ dispatch_pending_node (vlib_main_t * vm,
/* Frame is ready to be used again, so restore it. */
if (restore_frame_index != ~0)
{
- /* we musn't restore a frame that is flagged to be freed. This shouldn't
- happen since frames to be freed post dispatch are those used
- when the to-node frame becomes full i.e. they form a sort of queue of
- frames to a single node. If we get here then the to-node frame and the
- pending frame *were* the same, and so we removed the to-node frame.
- Therefore this frame is no longer part of the queue for that node
- and hence it cannot be it's overspill.
+ /*
+ * We musn't restore a frame that is flagged to be freed. This
+ * shouldn't happen since frames to be freed post dispatch are
+ * those used when the to-node frame becomes full i.e. they form a
+ * sort of queue of frames to a single node. If we get here then
+ * the to-node frame and the pending frame *were* the same, and so
+ * we removed the to-node frame. Therefore this frame is no
+ * longer part of the queue for that node and hence it cannot be
+ * it's overspill.
*/
ASSERT (!(f->flags & VLIB_FRAME_FREE_AFTER_DISPATCH));
- /* p->next_frame_index can change during node dispatch if node
- function decides to change graph hook up. */
+ /*
+ * NB: dispatching node n can result in the creation and scheduling
+ * of new frames, and hence in the reallocation of nm->pending_frames.
+ * Recompute p, or no supper. This was broken for more than 10 years.
+ */
+ p = nm->pending_frames + pending_frame_index;
+
+ /*
+ * p->next_frame_index can change during node dispatch if node
+ * function decides to change graph hook up.
+ */
nf = vec_elt_at_index (nm->next_frames, p->next_frame_index);
nf->flags |= VLIB_FRAME_IS_ALLOCATED;
@@ -1607,8 +1622,7 @@ vlib_main_or_worker_loop (vlib_main_t * vm, int is_main)
Process pending vector until there is nothing left.
All pending vectors will be processed from input -> output. */
for (i = 0; i < _vec_len (nm->pending_frames); i++)
- cpu_time_now = dispatch_pending_node (vm, nm->pending_frames + i,
- cpu_time_now);
+ cpu_time_now = dispatch_pending_node (vm, i, cpu_time_now);
/* Reset pending vector for next iteration. */
_vec_len (nm->pending_frames) = 0;