Wireshark · Ethereal-dev: [Ethereal-dev] Improving filter speed

[didier <dgautheron@xxxxxxxx>]

Hi
Filters are slow because dissectors are called with a non null tree 
parameter.
If you assume that:
1) in a frame with A- B -C -D protocols and a filter C.x you can only 
fully decode  C
2) if a filter field is only a protocol type  ie tcp, afp you don't need to
you can speed it a lot

Attached patch:
proto.c proto.h the logic.

packet-xxx.c dissectors using it.
Comments?

regards
Didier

Index: epan/proto.c
===================================================================
--- epan/proto.c	(revision 13776)
+++ epan/proto.c	(working copy)
@@ -196,6 +196,10 @@
 } gpa_hfinfo_t;
 gpa_hfinfo_t gpa_hfinfo;
 
+#define PROTO_REGISTRAR_GET_NTH(hfindex, hfinfo) \
+	g_assert((guint)hfindex < gpa_hfinfo.len); \
+	hfinfo=gpa_hfinfo.hfi[hfindex];		
+
 /* Balanced tree of abbreviations and IDs */
 static GTree *gpa_name_tree = NULL;
 
@@ -423,10 +427,21 @@
 }
 
 static void
-free_GPtrArray_value(gpointer key _U_, gpointer value, gpointer user_data _U_)
+free_GPtrArray_value(gpointer key, gpointer value, gpointer user_data _U_)
 {
 	GPtrArray   *ptrs = value;
+	gint hfid = (gint)key;
+	header_field_info *hfinfo;
 
+	/* assume interesting_hfids are destroyed in blocks otherwise need to ref count */
+	PROTO_REGISTRAR_GET_NTH(hfid, hfinfo);
+	hfinfo->infilter = 0;
+	if (hfinfo->parent != -1) {
+	    hfinfo->infilter = 0;
+	    PROTO_REGISTRAR_GET_NTH(hfinfo->parent, hfinfo );
+	    hfinfo->infilter = 0;
+	}
+
 	g_ptr_array_free(ptrs, TRUE);
 }
 
@@ -483,10 +498,27 @@
 	PTREE_DATA(tree)->visible = visible;
 }
 
-#define PROTO_REGISTRAR_GET_NTH(hfindex, hfinfo) \
-	g_assert((guint)hfindex < gpa_hfinfo.len); \
-	hfinfo=gpa_hfinfo.hfi[hfindex];		
+/* assume dissector set only its protocol fields
+*/
+gboolean
+proto_tree_is_null(proto_tree *tree, int proto_id)
+{
+    register header_field_info	*hfinfo;
 
+
+    if (!tree)
+        return TRUE;
+
+    if (PTREE_DATA(tree)->visible)
+        return FALSE;
+
+    PROTO_REGISTRAR_GET_NTH(proto_id, hfinfo);
+    if (hfinfo->infilter > 0)
+        return FALSE;
+  
+    return TRUE;
+}
+
 /* Finds a record in the hf_info_records array by id. */
 header_field_info*
 proto_registrar_get_nth(guint hfindex)
@@ -891,10 +923,12 @@
 
 	/* If the proto_tree wants to keep a record of this finfo
 	 * for quick lookup, then record it. */
-	hash = PTREE_DATA(tree)->interesting_hfids;
-	ptrs = g_hash_table_lookup(hash, GINT_TO_POINTER(hfindex));
-	if (ptrs) {
-		g_ptr_array_add(ptrs, new_fi);
+	if (new_fi->hfinfo->infilter) {
+	    hash = PTREE_DATA(tree)->interesting_hfids;
+	    ptrs = g_hash_table_lookup(hash, GINT_TO_POINTER(hfindex));
+	    if (ptrs) {
+	    	g_ptr_array_add(ptrs, new_fi);
+	    }
 	}
 
 	return pi;
@@ -1988,10 +2022,12 @@
 
 	/* If the proto_tree wants to keep a record of this finfo
 	 * for quick lookup, then record it. */
-	hash = PTREE_DATA(tree)->interesting_hfids;
-	ptrs = g_hash_table_lookup(hash, GINT_TO_POINTER(hfindex));
-	if (ptrs) {
-		g_ptr_array_add(ptrs, fi);
+	if (fi->hfinfo->infilter) {
+	    hash = PTREE_DATA(tree)->interesting_hfids;
+	    ptrs = g_hash_table_lookup(hash, GINT_TO_POINTER(hfindex));
+	    if (ptrs) {
+	    	g_ptr_array_add(ptrs, fi);
+	    }
 	}
 
 	/* Does the caller want to know the fi pointer? */
@@ -2269,8 +2305,23 @@
 void
 proto_tree_prime_hfid(proto_tree *tree, gint hfid)
 {
+	header_field_info *hfinfo;
+
 	g_hash_table_insert(PTREE_DATA(tree)->interesting_hfids,
 		GINT_TO_POINTER(hfid), g_ptr_array_new());
+
+	PROTO_REGISTRAR_GET_NTH(hfid, hfinfo);
+	if (hfinfo->parent != -1) {
+	    hfinfo->infilter = 1;
+	    PROTO_REGISTRAR_GET_NTH(hfinfo->parent, hfinfo);
+	    hfinfo->infilter = 1;
+	}
+	else if (!hfinfo->infilter) {
+	    /* use -1 for parent, aka protocol field 
+	     * if -1 trees aren't built
+	    */
+	    hfinfo->infilter = -1;
+	}
 }
 
 
@@ -2400,6 +2451,7 @@
 	hfinfo->strings = NULL;
 	hfinfo->bitmask = 0;
 	hfinfo->bitshift = 0;
+	hfinfo->infilter = 0;
 	hfinfo->blurb = "";
 	hfinfo->parent = -1; /* this field differentiates protos and fields */
 
Index: epan/proto.h
===================================================================
--- epan/proto.h	(revision 13776)
+++ epan/proto.h	(working copy)
@@ -146,6 +146,7 @@
 	/* ------- set by proto routines (prefilled by HFILL macro, see below) ------ */
 	int				id;		/**< Field ID */
 	int				parent;		/**< parent protocol tree */
+	int                             infilter;
 	int				bitshift;	/**< bits to shift (FT_BOOLEAN only) */
 	header_field_info		*same_name_next; /**< Link to next hfinfo with same abbrev*/
 	header_field_info		*same_name_prev; /**< Link to previous hfinfo with same abbrev*/
@@ -156,7 +157,7 @@
  * _header_field_info. If new fields are added or removed, it should
  * be changed as necessary.
  */
-#define HFILL 0, 0, 0, NULL, NULL
+#define HFILL 0, 0, 0, 0, NULL, NULL
 
 /** Used when registering many fields at once, using proto_register_field_array() */
 typedef struct hf_register_info {
@@ -257,8 +258,8 @@
 /** Frees memory used by proto routines. Called at program shutdown */
 extern void proto_cleanup(void);
 
+extern gboolean proto_tree_is_null(proto_tree *tree, int proto_id);
 
-
 /** Create a subtree under an existing item.
  @param ti the parent item of the new subtree
  @param idx one of the ett_ array elements registered with proto_register_subtree_array()
Index: epan/dissectors/packet-ip.c
===================================================================
--- epan/dissectors/packet-ip.c	(revision 13776)
+++ epan/dissectors/packet-ip.c	(working copy)
@@ -832,6 +832,10 @@
   static int eip_current=0;
   e_ip *iph;
   const guchar		*src_addr, *dst_addr;
+  proto_tree *save_tree = tree;
+  
+  if (proto_tree_is_null(tree, proto_ip))
+      tree = NULL;
 
   eip_current++;
   if(eip_current==4){
@@ -847,8 +851,11 @@
   iph->ip_v_hl = tvb_get_guint8(tvb, offset);
   hlen = lo_nibble(iph->ip_v_hl) * 4;	/* IP header length, in bytes */
 
-  if (tree) {
-    ti = proto_tree_add_item(tree, proto_ip, tvb, offset, hlen, FALSE);
+  if (save_tree) {
+    ti = proto_tree_add_item(save_tree, proto_ip, tvb, offset, hlen, FALSE);
+  }
+
+   if (tree) {
     ip_tree = proto_item_add_subtree(ti, ett_ip);
 
     proto_tree_add_uint(ip_tree, hf_ip_version, tvb, offset, 1,
@@ -1084,7 +1091,7 @@
     }
 
     call_dissector(data_handle, tvb_new_subset(tvb, offset, -1, -1), pinfo,
-                   tree);
+                   save_tree);
     pinfo->fragmented = save_fragmented;
     goto end_of_ip;
   }
@@ -1096,13 +1103,13 @@
      even be labelled as an IP frame; ideally, if a frame being dissected
      throws an exception, it'll be labelled as a mangled frame of the
      type in question. */
-  if (!dissector_try_port(ip_dissector_table, nxt, next_tvb, pinfo, tree)) {
+  if (!dissector_try_port(ip_dissector_table, nxt, next_tvb, pinfo, save_tree)) {
     /* Unknown protocol */
     if (update_col_info) {
       if (check_col(pinfo->cinfo, COL_INFO))
         col_add_fstr(pinfo->cinfo, COL_INFO, "%s (0x%02x)", ipprotostr(iph->ip_p), iph->ip_p);
     }
-    call_dissector(data_handle,next_tvb, pinfo, tree);
+    call_dissector(data_handle,next_tvb, pinfo, save_tree);
   }
   pinfo->fragmented = save_fragmented;
 
Index: epan/dissectors/packet-afp.c
===================================================================
--- epan/dissectors/packet-afp.c	(revision 13776)
+++ epan/dissectors/packet-afp.c	(working copy)
@@ -3304,9 +3304,12 @@
 	afp_request_key request_key, *new_request_key;
 	afp_request_val *request_val;
 	guint8	afp_command;
-
 	int     len =  tvb_reported_length_remaining(tvb,0);
 	gint col_info = check_col(pinfo->cinfo, COL_INFO);
+	proto_tree *save_tree = tree;
+	
+	if (proto_tree_is_null(tree, proto_afp))
+      		tree = NULL;
 
 	if (check_col(pinfo->cinfo, COL_PROTOCOL))
 		col_set_str(pinfo->cinfo, COL_PROTOCOL, "AFP");
@@ -3359,10 +3362,11 @@
 		}
 	}
 
-	if (tree)
+	if (save_tree)
 	{
-		ti = proto_tree_add_item(tree, proto_afp, tvb, offset, -1,FALSE);
-		afp_tree = proto_item_add_subtree(ti, ett_afp);
+		ti = proto_tree_add_item(save_tree, proto_afp, tvb, offset, -1,FALSE);
+		if (tree)
+		    afp_tree = proto_item_add_subtree(ti, ett_afp);
 	}
 	if (!aspinfo->reply)  {
 	        
Index: epan/dissectors/packet-dsi.c
===================================================================
--- epan/dissectors/packet-dsi.c	(revision 13776)
+++ epan/dissectors/packet-dsi.c	(working copy)
@@ -474,8 +474,11 @@
 	guint32		dsi_reserved;
 	struct		aspinfo aspinfo;
 	gint            col_info;
-	
+	proto_tree *save_tree = tree;
 
+	if (proto_tree_is_null(tree, proto_dsi))
+		tree = NULL;
+
 	if (check_col(pinfo->cinfo, COL_PROTOCOL))
 		col_set_str(pinfo->cinfo, COL_PROTOCOL, "DSI");
 	col_info = check_col(pinfo->cinfo, COL_INFO);
@@ -498,9 +501,11 @@
 			dsi_requestid);
 	}
 
-
+	if (save_tree) {
+		ti = proto_tree_add_item(save_tree, proto_dsi, tvb, 0, -1, FALSE);
+	}
+ 
 	if (tree) {
-		ti = proto_tree_add_item(tree, proto_dsi, tvb, 0, -1, FALSE);
 		dsi_tree = proto_item_add_subtree(ti, ett_dsi);
 
 		proto_tree_add_uint(dsi_tree, hf_dsi_flags, tvb,
@@ -559,7 +564,7 @@
 	  		proto_item_set_len(dsi_tree, DSI_BLOCKSIZ);
 
 			new_tvb = tvb_new_subset(tvb, DSI_BLOCKSIZ,-1,len);
-			call_dissector(afp_handle, new_tvb, pinfo, tree);
+			call_dissector(afp_handle, new_tvb, pinfo, save_tree);
 		}
 		break;
   	default:
Index: epan/dissectors/packet-eth.c
===================================================================
--- epan/dissectors/packet-eth.c	(revision 13776)
+++ epan/dissectors/packet-eth.c	(working copy)
@@ -170,6 +170,8 @@
   const char		*src_addr, *dst_addr;
   static eth_hdr 	ehdrs[4];
   static int		ehdr_num=0;
+  proto_tree *save_tree = tree;
+  
 
   ehdr_num++;
   if(ehdr_num>=4){
@@ -177,6 +179,8 @@
   }
   ehdr=&ehdrs[ehdr_num];
 
+  if (proto_tree_is_null(tree, proto_eth))
+      tree = NULL;
 
   if (check_col(pinfo->cinfo, COL_PROTOCOL))
     col_set_str(pinfo->cinfo, COL_PROTOCOL, "Ethernet");
@@ -198,7 +202,7 @@
    * a first look before we assume that it's actually an
    * Ethernet packet.
    */
-  if (dissector_try_heuristic(heur_subdissector_list, tvb, pinfo, tree))
+  if (dissector_try_heuristic(heur_subdissector_list, tvb, pinfo, save_tree))
     goto end_of_eth;
 
   if (ehdr->type <= IEEE_802_3_MAX_LEN) {
@@ -212,7 +216,7 @@
 		tvb_get_guint8(tvb, 2) == 0x0C &&
 		tvb_get_guint8(tvb, 3) == 0x00 &&
 		tvb_get_guint8(tvb, 4) == 0x00 ) {
-      dissect_isl(tvb, pinfo, tree, fcs_len);
+      dissect_isl(tvb, pinfo, save_tree, fcs_len);
       goto end_of_eth;
     }
   }
@@ -256,11 +260,12 @@
       col_add_fstr(pinfo->cinfo, COL_INFO, "IEEE 802.3 Ethernet %s",
 		(is_802_2 ? "" : "Raw "));
     }
-    if (tree) {
-      ti = proto_tree_add_protocol_format(tree, proto_eth, tvb, 0, ETH_HEADER_SIZE,
+    if (save_tree) {
+      ti = proto_tree_add_protocol_format(save_tree, proto_eth, tvb, 0, ETH_HEADER_SIZE,
 		"IEEE 802.3 Ethernet %s", (is_802_2 ? "" : "Raw "));
 
-      fh_tree = proto_item_add_subtree(ti, ett_ieee8023);
+      if (tree) 
+          fh_tree = proto_item_add_subtree(ti, ett_ieee8023);
     }
 
     proto_tree_add_ether(fh_tree, hf_eth_dst, tvb, 0, 6, dst_addr);
@@ -270,25 +275,25 @@
     proto_tree_add_ether_hidden(fh_tree, hf_eth_addr, tvb, 0, 6, dst_addr);
     proto_tree_add_ether_hidden(fh_tree, hf_eth_addr, tvb, 6, 6, src_addr);
 
-    dissect_802_3(ehdr->type, is_802_2, tvb, ETH_HEADER_SIZE, pinfo, tree, fh_tree,
+    dissect_802_3(ehdr->type, is_802_2, tvb, ETH_HEADER_SIZE, pinfo, save_tree, fh_tree,
 		  hf_eth_len, hf_eth_trailer, fcs_len);
   } else {
     if (eth_interpret_as_fw1_monitor) {
 	if ((dst_addr[0] == 'i') || (dst_addr[0] == 'I') ||
 	    (dst_addr[0] == 'o') || (dst_addr[0] == 'O')) {
-	  call_dissector(fw1_handle, tvb, pinfo, tree);
+	  call_dissector(fw1_handle, tvb, pinfo, save_tree);
 	  goto end_of_eth;
 	}
     }
 
     if (check_col(pinfo->cinfo, COL_INFO))
       col_set_str(pinfo->cinfo, COL_INFO, "Ethernet II");
-    if (tree) {
-      ti = proto_tree_add_protocol_format(tree, proto_eth, tvb, 0, ETH_HEADER_SIZE,
+    if (save_tree) {
+      ti = proto_tree_add_protocol_format(save_tree, proto_eth, tvb, 0, ETH_HEADER_SIZE,
 		"Ethernet II, Src: %s, Dst: %s",
 		ether_to_str(src_addr), ether_to_str(dst_addr));
-
-      fh_tree = proto_item_add_subtree(ti, ett_ether2);
+      if (tree)
+          fh_tree = proto_item_add_subtree(ti, ett_ether2);
     }
 
     proto_tree_add_ether(fh_tree, hf_eth_dst, tvb, 0, 6, dst_addr);
@@ -297,7 +302,7 @@
     proto_tree_add_ether_hidden(fh_tree, hf_eth_addr, tvb, 0, 6, dst_addr);
     proto_tree_add_ether_hidden(fh_tree, hf_eth_addr, tvb, 6, 6, src_addr);
 
-    ethertype(ehdr->type, tvb, ETH_HEADER_SIZE, pinfo, tree, fh_tree, hf_eth_type,
+    ethertype(ehdr->type, tvb, ETH_HEADER_SIZE, pinfo, save_tree, fh_tree, hf_eth_type,
           hf_eth_trailer, fcs_len);
   }
 
Index: epan/dissectors/packet-frame.c
===================================================================
--- epan/dissectors/packet-frame.c	(revision 13776)
+++ epan/dissectors/packet-frame.c	(working copy)
@@ -76,10 +76,14 @@
 dissect_frame(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
 	proto_tree	*fh_tree;
-	proto_item	*volatile ti = NULL;
+	proto_item	*ti = NULL;
 	nstime_t	ts;
 	int		cap_len, pkt_len;
+	proto_tree *save_tree = tree;
 
+  	if (proto_tree_is_null(tree, proto_frame))
+      		tree = NULL;
+
 	pinfo->current_proto = "Frame";
 
 	if (pinfo->pseudo_header != NULL) {
@@ -122,14 +126,15 @@
 	}
 
 	/* Put in frame header information. */
-	if (tree) {
+	if (save_tree) {
 
 	  cap_len = tvb_length(tvb);
 	  pkt_len = tvb_reported_length(tvb);
 
-	  ti = proto_tree_add_protocol_format(tree, proto_frame, tvb, 0, -1,
+	  ti = proto_tree_add_protocol_format(save_tree, proto_frame, tvb, 0, -1,
 	    "Frame %u (%u bytes on wire, %u bytes captured)", pinfo->fd->num, pkt_len, cap_len);
-
+	}
+	if (tree) {
 	  fh_tree = proto_item_add_subtree(ti, ett_frame);
 
 	  proto_tree_add_boolean_hidden(fh_tree, hf_frame_marked, tvb, 0, 0,pinfo->fd->flags.marked);
@@ -187,18 +192,18 @@
 
 	TRY {
 		if (!dissector_try_port(wtap_encap_dissector_table, pinfo->fd->lnk_t,
-					tvb, pinfo, tree)) {
+					tvb, pinfo, save_tree)) {
 
 			if (check_col(pinfo->cinfo, COL_PROTOCOL))
 				col_set_str(pinfo->cinfo, COL_PROTOCOL, "UNKNOWN");
 			if (check_col(pinfo->cinfo, COL_INFO))
 				col_add_fstr(pinfo->cinfo, COL_INFO, "WTAP_ENCAP = %u",
 				    pinfo->fd->lnk_t);
-			call_dissector(data_handle,tvb, pinfo, tree);
+			call_dissector(data_handle,tvb, pinfo, save_tree);
 		}
 	}
 	CATCH_ALL {
-		show_exception(tvb, pinfo, tree, EXCEPT_CODE, GET_MESSAGE);
+		show_exception(tvb, pinfo, save_tree, EXCEPT_CODE, GET_MESSAGE);
 	}
 	ENDTRY;
 
@@ -210,7 +215,7 @@
 
 	tap_queue_packet(frame_tap, pinfo, NULL);
 
-	if (mate_handle) call_dissector(mate_handle,tvb, pinfo, tree);
+	if (mate_handle) call_dissector(mate_handle,tvb, pinfo, save_tree);
 
 }