From 5ff8c3895a41b44486fbc17e05e4a188e3c4827e Mon Sep 17 00:00:00 2001 From: Ivan Nardi Date: Thu, 31 May 2018 20:25:08 +0200 Subject: [PATCH] SCCP: fix performance drop in reassembler code Commit 46dc5f75, while fixing sccp reassembler in the generic case, introduced a huge performance drop in some scenarios. The bottleneck is the sccp_reassembly_id_map hash table and, more precisely, the combination of the key layout and the hash function g_int64_hash() The key is defined as: guint64 key = ((guint64)frame << 32) | offset; Since the hash function uses only the lowest 32 bits of the key, all fragments at the same offset are saved in the same bucket If the sccp fragments are always in different packets and at the same offset (because, for example, there are only 1 chunk in every sctp packet) the hash table degenerates in exactly one linked list. Changing the key definition seems to restore the original performance Since there are usually hardly more than ~10/20 sctp chunks in a packet, this change shouldn't significantly affect performance when (all) fragments are in the same frame Change-Id: I2867a72819c2d91e1e0ae2cb97d63b5684d35bcc Reviewed-on: https://code.wireshark.org/review/27944 Petri-Dish: Peter Wu Tested-by: Petri Dish Buildbot Reviewed-by: Peter Wu --- epan/dissectors/packet-sccp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/epan/dissectors/packet-sccp.c b/epan/dissectors/packet-sccp.c index 6b1d64d46b..54e2612d64 100644 --- a/epan/dissectors/packet-sccp.c +++ b/epan/dissectors/packet-sccp.c @@ -844,7 +844,7 @@ sccp_reassembly_get_id_pass1(guint32 frame, guint32 offset, guint32 key, gboolea } /* Save ID for second pass. */ guint64 *frame_offset = wmem_new(wmem_file_scope(), guint64); - *frame_offset = ((guint64)frame << 32) | offset; + *frame_offset = ((guint64)offset << 32) | frame; wmem_map_insert(sccp_reassembly_id_map, frame_offset, GUINT_TO_POINTER(id)); return id; } @@ -852,7 +852,7 @@ sccp_reassembly_get_id_pass1(guint32 frame, guint32 offset, guint32 key, gboolea static guint32 sccp_reassembly_get_id_pass2(guint32 frame, guint32 offset) { - guint64 frame_offset = ((guint64)frame << 32) | offset; + guint64 frame_offset = ((guint64)offset << 32) | frame; return GPOINTER_TO_UINT(wmem_map_lookup(sccp_reassembly_id_map, &frame_offset)); }