vlib: reset stop_timer_handle on expired processes

Type: fix

The main loop populates a vector of suspended process nodes to dispatch
by calling TW (tw_timer_expire_timers_vec), which identifies expired
timers and appends the user handle for each one to the vector.

Subsequently, the vector is iterated and the process node corresponding
to each handle is dispatched. The vast majority of the time, the process
node will end up suspending itself again to wait for a new timer or
event.

Given a process node A whose timer has expired, between the point when
the timer expired and the point when A is dispatched and suspends itself
again, its stop_timer_handle contains a stale value.

If another process node B is dispatched before A is dispatched, it may
end up using the timer ID that A formerly used. If another process node
C is dispatched after B and before A and calls
vlib_process_signal_event() to signal A, the timer started by B can be
deleted by vlib_process_signal_event_helper().

After getting the vector of process node IDs for expired timers, reset
the stop_timer_handle on each of those nodes.

Change-Id: I266da438e76e1fc356016da0b9b4941efac1c28a
Signed-off-by: Matthew Smith <mgsmith@netgate.com>
(cherry picked from commit 9aa4ac55b0)

src/plugins/avf/avf_fdir_lib.c

@@ -28,7 +28,7 @@
 static inline int
 fls_u32 (u32 x)
 {
-  return (x == 0) ? 0 : 32 - count_leading_zeros (x);
+  return (x == 0) ? 0 : 64 - count_leading_zeros (x);
 }
 
 static inline int
@@ -257,9 +257,9 @@ avf_fdir_rcfg_set_field (struct avf_fdir_conf *rcfg, int layer,
 	    {
 	      rcfg->input_set |= AVF_INSET_IPV6_DST;
 	      VIRTCHNL_ADD_PROTO_HDR_FIELD_BIT (hdr, IPV6, DST);
-
-	      clib_memcpy (hdr->buffer, ipv6_spec, sizeof (*ipv6_spec));
 	    }
+
+	  clib_memcpy (hdr->buffer, ipv6_spec, sizeof (*ipv6_spec));
 	}
 
       break;

src/plugins/avf/device.c

@@ -1763,7 +1763,7 @@ avf_create_if (vlib_main_t * vm, avf_create_if_args_t * args)
   /* set hw interface caps */
   vnet_hw_if_set_caps (vnm, ad->hw_if_index,
 		       VNET_HW_IF_CAP_INT_MODE | VNET_HW_IF_CAP_MAC_FILTER |
-			 VNET_HW_IF_CAP_L4_TX_CKSUM | VNET_HW_IF_CAP_TCP_GSO);
+			 VNET_HW_IF_CAP_TX_CKSUM | VNET_HW_IF_CAP_TCP_GSO);
 
   for (i = 0; i < ad->n_rx_queues; i++)
     {

src/plugins/avf/flow.c

@@ -506,6 +506,7 @@ pattern_end:
   if (f->actions & VNET_FLOW_ACTION_RSS)
     {
       avf_actions[action_count].conf = &act_rss;
+      is_fdir = false;
 
       if ((act_rss.func = avf_flow_convert_rss_func (f->rss_fun)) ==
 	  AVF_ETH_HASH_FUNCTION_MAX)
@@ -522,8 +523,6 @@ pattern_end:
 	  is_fdir = true;
 	}
 
-      is_fdir = false;
-
       if (fate == true)
 	{
 	  rv = VNET_FLOW_ERROR_INTERNAL;

src/plugins/avf/output.c

@@ -123,8 +123,6 @@ avf_tx_prepare_cksum (vlib_buffer_t * b, u8 is_tso)
 	  sum = ~clib_ip_csum ((u8 *) &psh, sizeof (psh));
 	}
 
-  /* ip_csum does a byte swap for some reason... */
-  sum = clib_net_to_host_u16 (sum);
   if (is_tcp)
     tcp->checksum = sum;
   else

src/plugins/dpdk/device/flow.c

@@ -319,7 +319,8 @@ dpdk_flow_add (dpdk_device_t * xd, vnet_flow_t * f, dpdk_flow_entry_t * fe)
 
       if ((ip6_ptr->src_addr.mask.as_u64[0] == 0) &&
 	  (ip6_ptr->src_addr.mask.as_u64[1] == 0) &&
-	  (!ip6_ptr->protocol.mask))
+	  (ip6_ptr->dst_addr.mask.as_u64[0] == 0) &&
+	  (ip6_ptr->dst_addr.mask.as_u64[1] == 0) && (!ip6_ptr->protocol.mask))
 	{
 	  item->spec = NULL;
 	  item->mask = NULL;

src/plugins/nat/nat44-ed/nat44_ed_api.c

@@ -1202,7 +1202,7 @@ vl_api_nat44_ed_add_del_vrf_table_t_handler (
   vl_api_nat44_ed_add_del_vrf_table_reply_t *rmp;
   int rv = nat44_ed_add_del_vrf_table (clib_net_to_host_u32 (mp->table_vrf_id),
 				       mp->is_add);
-  REPLY_MACRO (VL_API_NAT44_ED_ADD_DEL_VRF_TABLE);
+  REPLY_MACRO (VL_API_NAT44_ED_ADD_DEL_VRF_TABLE_REPLY);
 }
 
 static void
@@ -1214,7 +1214,7 @@ vl_api_nat44_ed_add_del_vrf_route_t_handler (
   int rv =
     nat44_ed_add_del_vrf_route (clib_net_to_host_u32 (mp->table_vrf_id),
 				clib_net_to_host_u32 (mp->vrf_id), mp->is_add);
-  REPLY_MACRO (VL_API_NAT44_ED_ADD_DEL_VRF_ROUTE);
+  REPLY_MACRO (VL_API_NAT44_ED_ADD_DEL_VRF_ROUTE_REPLY);
 }
 
 static void

src/vcl/vppcom.c

@@ -3034,6 +3034,7 @@ vcl_epoll_wait_handle_mq_event (vcl_worker_t * wrk, session_event_t * e,
   u64 session_evt_data = ~0;
   vcl_session_t *s;
   u8 add_event = 0;
+  svm_fifo_t *txf;
 
   switch (e->event_type)
     {
@@ -3099,9 +3100,11 @@ vcl_epoll_wait_handle_mq_event (vcl_worker_t * wrk, session_event_t * e,
       if (!(EPOLLOUT & session_events))
 	break;
       /* We didn't have a fifo when the event was added */
-      svm_fifo_add_want_deq_ntf (
-	(vcl_session_is_ct (s) ? s->ct_tx_fifo : s->tx_fifo),
-	SVM_FIFO_WANT_DEQ_NOTIF_IF_FULL);
+      txf = vcl_session_is_ct (s) ? s->ct_tx_fifo : s->tx_fifo;
+      if (txf)
+	{
+	  svm_fifo_add_want_deq_ntf (txf, SVM_FIFO_WANT_DEQ_NOTIF_IF_FULL);
+	}
       add_event = 1;
       events[*num_ev].events = EPOLLOUT;
       session_evt_data = s->vep.ev.data.u64;

src/vlib/main.c

@@ -1615,10 +1615,8 @@ vlib_main_or_worker_loop (vlib_main_t * vm, int is_main)
 	  if (PREDICT_FALSE (vm->elog_trace_graph_dispatch))
 	    ed = ELOG_DATA (&vlib_global_main.elog_main, es);
 
-	  nm->data_from_advancing_timing_wheel =
-	    TW (tw_timer_expire_timers_vec)
-	    ((TWT (tw_timer_wheel) *) nm->timing_wheel, vlib_time_now (vm),
-	     nm->data_from_advancing_timing_wheel);
+	  TW (tw_timer_expire_timers)
+	  ((TWT (tw_timer_wheel) *) nm->timing_wheel, vlib_time_now (vm));
 
 	  ASSERT (nm->data_from_advancing_timing_wheel != 0);
 
@@ -1649,6 +1647,7 @@ vlib_main_or_worker_loop (vlib_main_t * vm, int is_main)
 			vlib_get_node (vm, te->process_node_index);
 		      vlib_process_t *p =
 			vec_elt (nm->processes, n->runtime_index);
+		      p->stop_timer_handle = ~0;
 		      void *data;
 		      data =
 			vlib_process_signal_event_helper (nm, n, p,
@@ -1847,6 +1846,23 @@ vl_api_get_elog_trace_api_messages (void)
   return 0;
 }
 
+static void
+process_expired_timer_cb (u32 *expired_timer_handles)
+{
+  vlib_main_t *vm = vlib_get_main ();
+  vlib_node_main_t *nm = &vm->node_main;
+  u32 *handle;
+
+  vec_foreach (handle, expired_timer_handles)
+    {
+      u32 pi = vlib_timing_wheel_data_get_index (*handle);
+      vlib_process_t *p = vec_elt (nm->processes, pi);
+
+      p->stop_timer_handle = ~0;
+    }
+  vec_append (nm->data_from_advancing_timing_wheel, expired_timer_handles);
+}
+
 /* Main function. */
 int
 vlib_main (vlib_main_t * volatile vm, unformat_input_t * input)
@@ -1952,10 +1968,10 @@ vlib_main (vlib_main_t * volatile vm, unformat_input_t * input)
   vec_set_len (nm->data_from_advancing_timing_wheel, 0);
 
   /* Create the process timing wheel */
-  TW (tw_timer_wheel_init) ((TWT (tw_timer_wheel) *) nm->timing_wheel,
-			    0 /* no callback */ ,
-			    10e-6 /* timer period 10us */ ,
-			    ~0 /* max expirations per call */ );
+  TW (tw_timer_wheel_init)
+  ((TWT (tw_timer_wheel) *) nm->timing_wheel,
+   process_expired_timer_cb /* callback */, 10e-6 /* timer period 10us */,
+   ~0 /* max expirations per call */);
 
   vec_validate (vm->pending_rpc_requests, 0);
   vec_set_len (vm->pending_rpc_requests, 0);

src/vlib/node_funcs.h

@@ -998,8 +998,11 @@ vlib_process_signal_event_helper (vlib_node_main_t * nm,
       p->flags = p_flags | VLIB_PROCESS_RESUME_PENDING;
       vec_add1 (nm->data_from_advancing_timing_wheel, x);
       if (delete_from_wheel)
-	TW (tw_timer_stop) ((TWT (tw_timer_wheel) *) nm->timing_wheel,
-			    p->stop_timer_handle);
+	{
+	  TW (tw_timer_stop)
+	  ((TWT (tw_timer_wheel) *) nm->timing_wheel, p->stop_timer_handle);
+	  p->stop_timer_handle = ~0;
+	}
     }
 
   return data_to_be_written_by_caller;