From dd5875c487dea7e75b3653485d153c0dab909699 Mon Sep 17 00:00:00 2001 From: Michael Mann Date: Mon, 18 Sep 2017 22:48:11 -0400 Subject: [PATCH] Sequence analysis (flow graph) optimizations for dissectors 1. Remove protocol member from seq_analysis_item_t. It's not used by any GUI, so don't burden dissectors with populating it. 2. Allow any dissector to change colors display by flow graph 3. Provide helper functions that may be common if other dissectors want to create sequence analysis. Change-Id: I04fa3c9f3cf6879ab9a8d7d6f4896b4979d010d7 Reviewed-on: https://code.wireshark.org/review/23613 Reviewed-by: Michael Mann Petri-Dish: Michael Mann Tested-by: Petri Dish Buildbot Reviewed-by: Jakub Zawadzki Reviewed-by: Anders Broman --- debian/libwireshark0.symbols | 3 ++ epan/dissectors/packet-frame.c | 73 ++-------------------------------- epan/dissectors/packet-tcp.c | 15 +++---- epan/sequence_analysis.c | 60 +++++++++++++++++++++++++++- epan/sequence_analysis.h | 25 +++++++++++- sharkd_session.c | 6 --- ui/gtk/lbm_uimflow_dlg.c | 1 - ui/qt/lbm_uimflow_dialog.cpp | 1 - ui/qt/sequence_diagram.cpp | 10 ++--- ui/voip_calls.c | 2 - 10 files changed, 98 insertions(+), 98 deletions(-) diff --git a/debian/libwireshark0.symbols b/debian/libwireshark0.symbols index 220b020e5d..bd0d085c88 100644 --- a/debian/libwireshark0.symbols +++ b/debian/libwireshark0.symbols @@ -1365,6 +1365,7 @@ libwireshark.so.0 libwireshark0 #MINVER# scsi_smc_vals_ext@Base 1.12.0~rc1 scsi_ssc_vals_ext@Base 1.12.0~rc1 sctp_port_to_display@Base 1.99.2 + sequence_analysis_create_sai_with_addresses@Base 2.5.0 sequence_analysis_find_by_name@Base 2.5.0 sequence_analysis_free_nodes@Base 2.5.0 sequence_analysis_get_name@Base 2.5.0 @@ -1378,6 +1379,8 @@ libwireshark.so.0 libwireshark0 #MINVER# sequence_analysis_list_free@Base 2.5.0 sequence_analysis_list_sort@Base 2.5.0 sequence_analysis_table_iterate_tables@Base 2.5.0 + sequence_analysis_use_col_info_as_label_comment@Base 2.5.0 + sequence_analysis_use_color_filter@Base 2.5.0 serv_name_lookup@Base 2.1.0 set_actual_length@Base 1.9.1 set_column_custom_fields@Base 2.1.0 diff --git a/epan/dissectors/packet-frame.c b/epan/dissectors/packet-frame.c index 9b9a0dbaf6..83ace71d37 100644 --- a/epan/dissectors/packet-frame.c +++ b/epan/dissectors/packet-frame.c @@ -170,99 +170,32 @@ frame_seq_analysis_packet( void *ptr, packet_info *pinfo, epan_dissect_t *edt _U seq_analysis_info_t *sainfo = (seq_analysis_info_t *) ptr; if ((sainfo->all_packets) || (pinfo->fd->flags.passed_dfilter == 1)) { - gchar *protocol = NULL; - gchar *colinfo = NULL; - seq_analysis_item_t *sai = NULL; - - if (sainfo->any_addr) { - if (pinfo->net_src.type!=AT_NONE && pinfo->net_dst.type!=AT_NONE) { - sai = g_new0(seq_analysis_item_t, 1); - copy_address(&(sai->src_addr),&(pinfo->net_src)); - copy_address(&(sai->dst_addr),&(pinfo->net_dst)); - } - - } else { - if (pinfo->src.type!=AT_NONE && pinfo->dst.type!=AT_NONE) { - sai = g_new0(seq_analysis_item_t, 1); - copy_address(&(sai->src_addr),&(pinfo->src)); - copy_address(&(sai->dst_addr),&(pinfo->dst)); - } - } + seq_analysis_item_t *sai = sequence_analysis_create_sai_with_addresses(pinfo, sainfo); if (!sai) return FALSE; sai->frame_number = pinfo->num; - if (pinfo->fd->color_filter) { - sai->bg_color = color_t_to_rgb(&pinfo->fd->color_filter->bg_color); - sai->fg_color = color_t_to_rgb(&pinfo->fd->color_filter->fg_color); - sai->has_color_filter = TRUE; - } + sequence_analysis_use_color_filter(pinfo, sai); sai->port_src=pinfo->srcport; sai->port_dst=pinfo->destport; - sai->protocol = g_strdup(port_type_to_str(pinfo->ptype)); - if (pinfo->cinfo) { - col_item_t *col_item; - int i; - - if (pinfo->cinfo->col_first[COL_INFO] >= 0) { - for (i = pinfo->cinfo->col_first[COL_INFO]; i <= pinfo->cinfo->col_last[COL_INFO]; i++) { - col_item = &pinfo->cinfo->columns[i]; - if (col_item->fmt_matx[COL_INFO]) { - colinfo = g_strdup(col_item->col_data); - /* break; ? or g_free(colinfo); before g_strdup() */ - } - } - } - - if (pinfo->cinfo->col_first[COL_PROTOCOL] >= 0) { - for (i = pinfo->cinfo->col_first[COL_PROTOCOL]; i <= pinfo->cinfo->col_last[COL_PROTOCOL]; i++) { - col_item = &pinfo->cinfo->columns[i]; - if (col_item->fmt_matx[COL_PROTOCOL]) { - protocol = g_strdup(col_item->col_data); - /* break; ? or g_free(protocol); before g_strdup() */ - } - } - } - } - - if (colinfo != NULL) { - sai->frame_label = g_strdup(colinfo); - if (protocol != NULL) { - sai->comment = g_strdup_printf("%s: %s", protocol, colinfo); - } else { - sai->comment = g_strdup(colinfo); - } - } else { - /* This will probably never happen...*/ - if (protocol != NULL) { - sai->frame_label = g_strdup(protocol); - sai->comment = g_strdup(protocol); - } - } + sequence_analysis_use_col_info_as_label_comment(pinfo, sai); if (pinfo->ptype == PT_NONE) { icmp_info_t *p_icmp_info; if ((p_icmp_info = (icmp_info_t *) p_get_proto_data(wmem_file_scope(), pinfo, proto_get_id_by_short_name("ICMP"), 0))) { - g_free(sai->protocol); - sai->protocol = g_strdup("ICMP"); sai->port_src = 0; sai->port_dst = p_icmp_info->type * 256 + p_icmp_info->code; } else if ((p_icmp_info = (icmp_info_t *) p_get_proto_data(wmem_file_scope(), pinfo, proto_get_id_by_short_name("ICMPv6"), 0))) { - g_free(sai->protocol); - sai->protocol = g_strdup("ICMPv6"); sai->port_src = 0; sai->port_dst = p_icmp_info->type * 256 + p_icmp_info->code; } } - g_free(protocol); - g_free(colinfo); - sai->line_style = 1; sai->conv_num = 0; sai->display = TRUE; diff --git a/epan/dissectors/packet-tcp.c b/epan/dissectors/packet-tcp.c index 899326f6a6..4c6bded5ff 100644 --- a/epan/dissectors/packet-tcp.c +++ b/epan/dissectors/packet-tcp.c @@ -872,20 +872,15 @@ tcp_seq_analysis_packet( void *ptr, packet_info *pinfo, epan_dissect_t *edt _U_, if ((sainfo->all_packets)||(pinfo->fd->flags.passed_dfilter==1)){ const char* flags; - seq_analysis_item_t *sai; + seq_analysis_item_t *sai = sequence_analysis_create_sai_with_addresses(pinfo, sainfo); + + if (!sai) + return FALSE; - sai = g_new0(seq_analysis_item_t, 1); sai->frame_number = pinfo->num; - if (sainfo->any_addr) { - copy_address(&(sai->src_addr),&(pinfo->net_src)); - copy_address(&(sai->dst_addr),&(pinfo->net_dst)); - } else { - copy_address(&(sai->src_addr),&(pinfo->src)); - copy_address(&(sai->dst_addr),&(pinfo->dst)); - } + sai->port_src=pinfo->srcport; sai->port_dst=pinfo->destport; - sai->protocol=g_strdup(port_type_to_str(pinfo->ptype)); flags = tcp_flags_to_str(NULL, tcph); diff --git a/epan/sequence_analysis.c b/epan/sequence_analysis.c index 7d8f03064f..e829f48746 100644 --- a/epan/sequence_analysis.c +++ b/epan/sequence_analysis.c @@ -28,6 +28,8 @@ #include "addr_resolv.h" #include "proto.h" +#include "color_filters.h" +#include "column-info.h" #include "tap.h" #include "wmem/wmem.h" @@ -105,6 +107,63 @@ void sequence_analysis_table_iterate_tables(wmem_foreach_func func, gpointer use wmem_tree_foreach(registered_seq_analysis, func, user_data); } +seq_analysis_item_t* sequence_analysis_create_sai_with_addresses(packet_info *pinfo, seq_analysis_info_t *sainfo) +{ + seq_analysis_item_t *sai = NULL; + + if (sainfo->any_addr) { + if (pinfo->net_src.type!=AT_NONE && pinfo->net_dst.type!=AT_NONE) { + sai = g_new0(seq_analysis_item_t, 1); + copy_address(&(sai->src_addr),&(pinfo->net_src)); + copy_address(&(sai->dst_addr),&(pinfo->net_dst)); + } + + } else { + if (pinfo->src.type!=AT_NONE && pinfo->dst.type!=AT_NONE) { + sai = g_new0(seq_analysis_item_t, 1); + copy_address(&(sai->src_addr),&(pinfo->src)); + copy_address(&(sai->dst_addr),&(pinfo->dst)); + } + } + + return sai; +} + +void sequence_analysis_use_color_filter(packet_info *pinfo, seq_analysis_item_t *sai) +{ + if (pinfo->fd->color_filter) { + sai->bg_color = color_t_to_rgb(&pinfo->fd->color_filter->bg_color); + sai->fg_color = color_t_to_rgb(&pinfo->fd->color_filter->fg_color); + sai->has_color_filter = TRUE; + } +} + +void sequence_analysis_use_col_info_as_label_comment(packet_info *pinfo, seq_analysis_item_t *sai) +{ + const gchar *protocol = NULL; + const gchar *colinfo = NULL; + + if (pinfo->cinfo) { + colinfo = col_get_text(pinfo->cinfo, COL_INFO); + protocol = col_get_text(pinfo->cinfo, COL_PROTOCOL); + } + + if (colinfo != NULL) { + sai->frame_label = g_strdup(colinfo); + if (protocol != NULL) { + sai->comment = g_strdup_printf("%s: %s", protocol, colinfo); + } else { + sai->comment = g_strdup(colinfo); + } + } else { + /* This will probably never happen...*/ + if (protocol != NULL) { + sai->frame_label = g_strdup(protocol); + sai->comment = g_strdup(protocol); + } + } +} + seq_analysis_info_t * sequence_analysis_info_new(void) { @@ -135,7 +194,6 @@ static void sequence_analysis_item_free(gpointer data) g_free(seq_item->frame_label); g_free(seq_item->time_str); g_free(seq_item->comment); - g_free(seq_item->protocol); free_address(&seq_item->src_addr); free_address(&seq_item->dst_addr); g_free(data); diff --git a/epan/sequence_analysis.h b/epan/sequence_analysis.h index bcebdef3a5..247b3d5fbd 100644 --- a/epan/sequence_analysis.h +++ b/epan/sequence_analysis.h @@ -63,7 +63,6 @@ typedef struct _seq_analysis_item { guint src_node; /**< this is used by graph_analysis.c to identify the node */ guint dst_node; /**< a node is an IP address that will be displayed in columns */ guint16 line_style; /**< the arrow line width in pixels*/ - gchar *protocol; /**< the label of the protocol defined in the IP packet */ } seq_analysis_item_t; /** defines the graph analysis structure */ @@ -128,6 +127,30 @@ WS_DLL_PUBLIC tap_packet_cb sequence_analysis_get_packet_func(register_analysis_ */ WS_DLL_PUBLIC guint sequence_analysis_get_tap_flags(register_analysis_t* analysis); +/** Helper function to create a sequence analysis item with address fields populated + * Allocate a seq_analysis_item_t to return and populate the src_addr and dst_addr + * members based on seq_analysis_info_t any_addr member + * + * @param pinfo packet info + * @param sainfo info determining address type + * @return sequence analysis tap flags + */ +WS_DLL_PUBLIC seq_analysis_item_t* sequence_analysis_create_sai_with_addresses(packet_info *pinfo, seq_analysis_info_t *sainfo); + +/** Helper function to set colors for analysis the same as Wireshark display + * + * @param pinfo packet info + * @param sai item to set color + */ +WS_DLL_PUBLIC void sequence_analysis_use_color_filter(packet_info *pinfo, seq_analysis_item_t *sai); + +/** Helper function to set frame label and comments to use protocol and info column data + * + * @param pinfo packet info + * @param sai item to set label and comments + */ +WS_DLL_PUBLIC void sequence_analysis_use_col_info_as_label_comment(packet_info *pinfo, seq_analysis_item_t *sai); + /** Find a registered sequence analysis "protocol" by name * * @param name Registered sequence analysis to find diff --git a/sharkd_session.c b/sharkd_session.c index 129fa6e5ab..2413fb6405 100644 --- a/sharkd_session.c +++ b/sharkd_session.c @@ -1066,12 +1066,6 @@ sharkd_session_process_tap_flow_cb(void *tapdata) printf(",\"n\":[%u,%u]", sai->src_node, sai->dst_node); printf(",\"pn\":[%u,%u]", sai->port_src, sai->port_dst); - if (sai->protocol) - { - printf(",\"p\":"); - json_puts_string(sai->protocol); - } - if (sai->comment) { printf(",\"c\":"); diff --git a/ui/gtk/lbm_uimflow_dlg.c b/ui/gtk/lbm_uimflow_dlg.c index 0a5f1fbd4c..3c2dfcb32c 100644 --- a/ui/gtk/lbm_uimflow_dlg.c +++ b/ui/gtk/lbm_uimflow_dlg.c @@ -170,7 +170,6 @@ static int lbmc_uim_flow_graph_add_to_graph(packet_info * pinfo, const lbm_uim_s item->frame_number = pinfo->num; item->port_src = pinfo->srcport; item->port_dst = pinfo->destport; - item->protocol = g_strdup(port_type_to_str(pinfo->ptype)); if (stream_info->description == NULL) { item->frame_label = g_strdup_printf("(%" G_GUINT32_FORMAT ")", stream_info->sqn); diff --git a/ui/qt/lbm_uimflow_dialog.cpp b/ui/qt/lbm_uimflow_dialog.cpp index c355afa031..29e5020270 100644 --- a/ui/qt/lbm_uimflow_dialog.cpp +++ b/ui/qt/lbm_uimflow_dialog.cpp @@ -128,7 +128,6 @@ static gboolean lbm_uimflow_add_to_graph(seq_analysis_info_t * seq_info, packet_ item->frame_number = pinfo->num; item->port_src = pinfo->srcport; item->port_dst = pinfo->destport; - item->protocol = g_strdup(port_type_to_str(pinfo->ptype)); if (stream_info->description == NULL) { item->frame_label = g_strdup_printf("(%" G_GUINT32_FORMAT ")", stream_info->sqn); diff --git a/ui/qt/sequence_diagram.cpp b/ui/qt/sequence_diagram.cpp index 98071b18a6..3b20d0b462 100644 --- a/ui/qt/sequence_diagram.cpp +++ b/ui/qt/sequence_diagram.cpp @@ -263,12 +263,10 @@ void SequenceDiagram::draw(QCPPainter *painter) fg_pen.setColor(sel_pal.color(QPalette::HighlightedText)); bg_color = sel_pal.color(QPalette::Highlight); selected_key_ = cur_key; - } else if (strcmp(sainfo_->name, "any") == 0) { - if (sai->has_color_filter) { - fg_pen.setColor(QColor().fromRgb(sai->fg_color)); - bg_color = QColor().fromRgb(sai->bg_color); - } - } else { // SEQ_ANALYSIS_VOIP, SEQ_ANALYSIS_TCP + } else if (sai->has_color_filter) { + fg_pen.setColor(QColor().fromRgb(sai->fg_color)); + bg_color = QColor().fromRgb(sai->bg_color); + } else { fg_pen.setColor(Qt::black); bg_color = ColorUtils::sequenceColor(sai->conv_num); } diff --git a/ui/voip_calls.c b/ui/voip_calls.c index 25629fe0f4..7bf41b4f8f 100644 --- a/ui/voip_calls.c +++ b/ui/voip_calls.c @@ -342,7 +342,6 @@ add_to_graph(voip_calls_tapinfo_t *tapinfo, packet_info *pinfo, epan_dissect_t * gai->port_src=pinfo->srcport; gai->port_dst=pinfo->destport; - gai->protocol = g_strdup(port_type_to_str(pinfo->ptype)); if (frame_label != NULL) gai->frame_label = g_strdup(frame_label); @@ -463,7 +462,6 @@ static void insert_to_graph_t38(voip_calls_tapinfo_t *tapinfo, packet_info *pinf new_gai->port_src=pinfo->srcport; new_gai->port_dst=pinfo->destport; - new_gai->protocol = g_strdup(port_type_to_str(pinfo->ptype)); if (frame_label != NULL) new_gai->frame_label = g_strdup(frame_label); else