ping: fixing wrong value when there are worker threads

- the echo_reply_node is now notifying the cli process on the main thread/vlib_main
- the timestamp for the icmp reply is now acquired in the echo_reply_node and not in the cli process to avoid an off by 10ms error (see 【vpp-dev】delay is error in ping with multi worker thread)

Change-Id: I21d37002b0376b4f2ccab08d8f04c2f2944b9b39
Signed-off-by: Mohammed Hawari <mhawari@cisco.com>

diff --git a/src/vnet/ip/ping.c b/src/vnet/ip/ping.c
index c664772..eee2575 100755 (executable)
--- a/src/vnet/ip/ping.c
+++ b/src/vnet/ip/ping.c
@@ -18,6 +18,8 @@
 #include <vnet/fib/ip6_fib.h>
 #include <vnet/fib/ip4_fib.h>
 #include <vnet/fib/fib_entry.h>
+#include <vlib/vlib.h>
+#include <emmintrin.h>
 
 /**
  * @file
@@ -53,8 +55,7 @@ format_icmp_echo_trace (u8 * s, va_list * va)
  */
 
 static int
-signal_ip46_icmp_reply_event (vlib_main_t * vm,
-                             u8 event_type, vlib_buffer_t * b0)
+signal_ip46_icmp_reply_event (u8 event_type, vlib_buffer_t * b0)
 {
   ping_main_t *pm = &ping_main;
   u16 net_icmp_id = 0;
@@ -84,13 +85,19 @@ signal_ip46_icmp_reply_event (vlib_main_t * vm,
     return 0;
 
   ping_run_t *pr = vec_elt_at_index (pm->ping_runs, p[0]);
+  vlib_main_t *vm = vlib_mains[pr->cli_thread_index];
   if (vlib_buffer_alloc (vm, &bi0_copy, 1) == 1)
     {
-      void *dst = vlib_buffer_get_current (vlib_get_buffer (vm, bi0_copy));
+      void *dst = vlib_buffer_get_current (vlib_get_buffer (vm,
+                                                           bi0_copy));
       clib_memcpy (dst, vlib_buffer_get_current (b0), b0->current_length);
     }
   /* If buffer_alloc failed, bi0_copy == 0 - just signaling an event. */
-
+  f64 nowts = vlib_time_now (vm);
+  /* Pass the timestamp to the cli_process thanks to the vnet_buffer unused metadata field */
+  clib_memcpy (vnet_buffer
+              (vlib_get_buffer
+               (vm, bi0_copy))->unused, &nowts, sizeof (nowts));
   vlib_process_signal_event (vm, pr->cli_process_id, event_type, bi0_copy);
   return 1;
 }
@@ -116,7 +123,7 @@ ip6_icmp_echo_reply_node_fn (vlib_main_t * vm,
       bi0 = from[0];
       b0 = vlib_get_buffer (vm, bi0);
 
-      next0 = signal_ip46_icmp_reply_event (vm, PING_RESPONSE_IP6, b0) ?
+      next0 = signal_ip46_icmp_reply_event (PING_RESPONSE_IP6, b0) ?
        ICMP6_ECHO_REPLY_NEXT_DROP : ICMP6_ECHO_REPLY_NEXT_PUNT;
 
       if (PREDICT_FALSE (b0->flags & VLIB_BUFFER_IS_TRACED))
@@ -174,7 +181,7 @@ ip4_icmp_echo_reply_node_fn (vlib_main_t * vm,
       bi0 = from[0];
       b0 = vlib_get_buffer (vm, bi0);
 
-      next0 = signal_ip46_icmp_reply_event (vm, PING_RESPONSE_IP4, b0) ?
+      next0 = signal_ip46_icmp_reply_event (PING_RESPONSE_IP4, b0) ?
        ICMP4_ECHO_REPLY_NEXT_DROP : ICMP4_ECHO_REPLY_NEXT_PUNT;
 
       if (PREDICT_FALSE (b0->flags & VLIB_BUFFER_IS_TRACED))
@@ -497,8 +504,9 @@ print_ip6_icmp_reply (vlib_main_t * vm, u32 bi0)
 {
   vlib_buffer_t *b0 = vlib_get_buffer (vm, bi0);
   icmp6_echo_request_header_t *h0 = vlib_buffer_get_current (b0);
-  f64 rtt = vlib_time_now (vm) - h0->icmp_echo.time_sent;
-
+  f64 rtt = 0;
+  clib_memcpy (&rtt, vnet_buffer (b0)->unused, sizeof (rtt));
+  rtt -= h0->icmp_echo.time_sent;
   vlib_cli_output (vm,
                   "%d bytes from %U: icmp_seq=%d ttl=%d time=%.4f ms",
                   clib_host_to_net_u16 (h0->ip6.payload_length),
@@ -513,7 +521,9 @@ print_ip4_icmp_reply (vlib_main_t * vm, u32 bi0)
 {
   vlib_buffer_t *b0 = vlib_get_buffer (vm, bi0);
   icmp4_echo_request_header_t *h0 = vlib_buffer_get_current (b0);
-  f64 rtt = vlib_time_now (vm) - h0->icmp_echo.time_sent;
+  f64 rtt = 0;
+  clib_memcpy (&rtt, vnet_buffer (b0)->unused, sizeof (rtt));
+  rtt -= h0->icmp_echo.time_sent;
   u32 rcvd_icmp_len =
     clib_host_to_net_u16 (h0->ip4.length) -
     (4 * (0xF & h0->ip4.ip_version_and_header_length));
@@ -565,6 +575,7 @@ run_ping_ip46_address (vlib_main_t * vm, u32 table_id, ip4_address_t * pa4,
   pool_get (pm->ping_runs, pr);
   ping_run_index = pr - pm->ping_runs;
   pr->cli_process_id = curr_proc;
+  pr->cli_thread_index = vlib_get_thread_index ();
   pr->icmp_id = icmp_id;
   hash_set (pm->ping_run_by_icmp_id, icmp_id, ping_run_index);
   for (i = 1; i <= ping_repeat; i++)

diff --git a/src/vnet/ip/ping.h b/src/vnet/ip/ping.h
index 0af9d58..b1b71f6 100644 (file)
--- a/src/vnet/ip/ping.h
+++ b/src/vnet/ip/ping.h
@@ -43,6 +43,7 @@ typedef struct ping_run_t
   u16 icmp_id;
   u16 curr_seq;
   uword cli_process_id;
+  uword cli_thread_index;
 } ping_run_t;
 
 typedef struct ping_main_t