Flowprobe: Fix flow start time and hash computation

Upon hash collision, the flow start time was not reset.
The hash computation techniques (crc32 or xxhash) also both
had bugs which are now fixed.

Change-Id: I94d72997f34018d1699324264f7dded2a5cbd776
Signed-off-by: Pierre Pfister <ppfister@cisco.com>

diff --git a/src/plugins/flowprobe/flowprobe.h b/src/plugins/flowprobe/flowprobe.h
index 760e924..02ee053 100644 (file)
--- a/src/plugins/flowprobe/flowprobe.h
+++ b/src/plugins/flowprobe/flowprobe.h
@@ -72,30 +72,22 @@ typedef struct
   u16 *next_record_offset_per_worker;
 } flowprobe_protocol_context_t;
 
-#define FLOWPROBE_KEY_IN_U32 22
 /* *INDENT-OFF* */
-typedef CLIB_PACKED (union
-{
-  struct {
-    u32 rx_sw_if_index;
-    u32 tx_sw_if_index;
-    u8 src_mac[6];
-    u8 dst_mac[6];
-    u16 ethertype;
-    ip46_address_t src_address;
-    ip46_address_t dst_address;
-    u8 protocol;
-    u16 src_port;
-    u16 dst_port;
-    flowprobe_variant_t which;
-  };
-  u32 as_u32[FLOWPROBE_KEY_IN_U32];
-}) flowprobe_key_t;
+typedef struct __attribute__ ((aligned (8))) {
+  u32 rx_sw_if_index;
+  u32 tx_sw_if_index;
+  u8 src_mac[6];
+  u8 dst_mac[6];
+  u16 ethertype;
+  ip46_address_t src_address;
+  ip46_address_t dst_address;
+  u8 protocol;
+  u16 src_port;
+  u16 dst_port;
+  flowprobe_variant_t which;
+} flowprobe_key_t;
 /* *INDENT-ON* */
 
-STATIC_ASSERT (sizeof (flowprobe_key_t) == FLOWPROBE_KEY_IN_U32 *
-              sizeof (u32), "flowprobe_key_t padding is wrong");
-
 typedef struct
 {
   u32 sec;

diff --git a/src/plugins/flowprobe/node.c b/src/plugins/flowprobe/node.c
index c4610a7..2f7d002 100644 (file)
--- a/src/plugins/flowprobe/node.c
+++ b/src/plugins/flowprobe/node.c
@@ -289,10 +289,13 @@ flowprobe_hash (flowprobe_key_t * k)
   u32 h = 0;
 
 #ifdef clib_crc32c_uses_intrinsics
-  h = clib_crc32c ((u8 *) k->as_u32, FLOWPROBE_KEY_IN_U32);
+  h = clib_crc32c ((u8 *) k, sizeof (*k));
 #else
-  u64 tmp =
-    k->as_u32[0] ^ k->as_u32[1] ^ k->as_u32[2] ^ k->as_u32[3] ^ k->as_u32[4];
+  int i;
+  u64 tmp = 0;
+  for (i = 0; i < sizeof (*k) / 8; i++)
+    tmp ^= ((u64 *) k)[i];
+
   h = clib_xxhash (tmp);
 #endif
 
@@ -370,7 +373,7 @@ add_to_flow_record_state (vlib_main_t * vm, vlib_node_runtime_t * node,
   ethernet_header_t *eth = vlib_buffer_get_current (b);
   u16 ethertype = clib_net_to_host_u16 (eth->type);
   /* *INDENT-OFF* */
-  flowprobe_key_t k = { {0} };
+  flowprobe_key_t k = {};
   /* *INDENT-ON* */
   ip4_header_t *ip4 = 0;
   ip6_header_t *ip6 = 0;
@@ -472,6 +475,7 @@ add_to_flow_record_state (vlib_main_t * vm, vlib_node_runtime_t * node,
          if (e->packetcount)
            flowprobe_export_entry (vm, e);
          e->key = k;
+         e->flow_start = timestamp;
          vlib_node_increment_counter (vm, node->node_index,
                                       FLOWPROBE_ERROR_COLLISION, 1);
        }