major refactoring of the ifcp dissector.

remove the port preference setting and replace it with strong heuristics instead
(attempt but fail to set a conversation dissector when the heuristics and the dissection match)

use tcp_dissect_pdus() for reassembly and pdu tracking and get rid of the try-to-step-through-the-pdu-to-find-where-the-next-pdu-starts thing.




svn path=/trunk/; revision=17804

1 files changed, 394 insertions, 275 deletions

diff --git a/epan/dissectors/packet-ifcp.c b/epan/dissectors/packet-ifcp.c
index 2efcf4b5b1..7aff5cffc1 100644
--- a/epan/dissectors/packet-ifcp.c
+++ b/epan/dissectors/packet-ifcp.c
@@ -3,6 +3,7 @@
  * RFC 3821, RFC 3643
  *
  * Copyright 2005   Aboo Valappil     (valappil_aboo@emc.com)
+ *           2006 ronnie sahlberg   major refactoring
  *
  *
  * Significantly based on packet-fcip.c by
@@ -41,6 +42,9 @@
 
 #include <epan/packet.h>
 #include <epan/prefs.h>
+#include <epan/emem.h>
+#include <epan/conversation.h>
+#include "packet-tcp.h"
 
 #define iFCP_ENCAP_HEADER_LEN                    28
 #define iFCP_MIN_HEADER_LEN                      16 /* upto frame len field */ 
@@ -113,7 +117,6 @@ static int hf_ifcp_protocol    = -1;
 static int hf_ifcp_protocol_c  = -1;
 static int hf_ifcp_version     = -1;
 static int hf_ifcp_version_c   = -1;
-static int hf_ifcp_encap_flags = -1;
 static int hf_ifcp_encap_flags_c=-1;
 static int hf_ifcp_framelen    = -1;
 static int hf_ifcp_framelen_c  = -1;
@@ -125,327 +128,439 @@ static int hf_ifcp_sof_c       = -1;
 static int hf_ifcp_eof         = -1;
 static int hf_ifcp_eof_c       = -1;
 static int hf_ifcp_ls_command_acc = -1;
+static int hf_ifcp_flags     = -1;
 static int hf_ifcp_flags_ses = -1;
 static int hf_ifcp_flags_trp = -1;
 static int hf_ifcp_flags_spc = -1;
+static int hf_ifcp_common_flags    = -1;
+static int hf_ifcp_common_flags_crcv = -1;
+
+static int ett_ifcp              = -1;
+static int ett_ifcp_sof          = -1;
+static int ett_ifcp_eof          = -1;
+static int ett_ifcp_flags        = -1;
+static int ett_ifcp_common_flags = -1;
+static int ett_ifcp_protocol     = -1;
+static int ett_ifcp_version      = -1;
+static int ett_ifcp_frame_len    = -1;
 
-static int ett_ifcp            = -1;
-
-static guint ifcp_port         = 3420;
 static gboolean ifcp_desegment    = TRUE;
 
-static dissector_handle_t data_handle;
-static dissector_handle_t fc_handle;
+static dissector_handle_t ifcp_handle=NULL;
+static dissector_handle_t data_handle=NULL;
+static dissector_handle_t fc_handle=NULL;
+
 
-/* This routine attempts to locate the position of the next header in the
- * provided segment
+/* This function checks the first 16 bytes of the "header" that it looks sane
+ * and returns TRUE if this looks like iFCP and FALSE if it doesnt.
  */
-static guint
-get_next_ifcp_header_offset (tvbuff_t *tvb, packet_info *pinfo, gint offset)
+static gboolean
+ifcp_header_test(tvbuff_t *tvb, int offset)
 {
-    gint bytes_remaining = tvb_length_remaining (tvb, offset);
-    gint frame_len;
-    guint16 flen, flen1;
-
-    /*
-     * As per the iFCP standard, the following tests must PASS:
-     * 1)  Frame Length field validation -- 15 < Frame Length < 545;
-     * 2)  Comparison of Frame Length field to its ones complement; and
-     * 3)  A valid EOF is found in the word preceding the start of the next
-     *     iFCP header as indicated by the Frame Length field, to be tested
-     *     as follows:
-     *     1)  Bits 24-31 and 16-23 contain identical legal EOF values (the
-     *         list of legal EOF values is in the FC Frame Encapsulation
-     *         [21]); and
-     *     2)  Bits 8-15 and 0-7 contain the ones complement of the EOF
-     *         value found in bits 24-31.
-     *
-     * As per the iFCP standard, in addition, at least 3 of the following set
-     * of tests must be performed to identify that we've located the start of
-     * an iFCP frame. 
-     * a)  Protocol# ones complement field (1 test);
-     * b)  Version ones complement field (1 test);
-     * c)  Replication of encapsulation word 0 in word 1 (1 test);
-     * d)  Reserved field and its ones complement (2 tests);
-     * e)  Flags field and its ones complement (2 tests);
-     *    f)  CRC field is equal to zero (1 test); (DONT DO THIS TEST!)
-     * g)  SOF fields and ones complement fields (4 tests);
-     * h)  Format and values of FC header (1 test);
-     * i)  CRC of FC Frame (2 tests);
-     * j)  FC Frame Encapsulation header information in the next iFCP Frame
-     *     (1 test).
-     *
-     * At least 3 of the 16 tests listed above SHALL be performed. Failure
-     * of any of the above tests actually performed SHALL indicate an
-     * encapsulation error and the FC Frame SHALL NOT be forwarded on to
-     * the FC Entity.
-     */
-
-NXT_BYTE: while (bytes_remaining) {
-        if (bytes_remaining < iFCP_ENCAP_HEADER_LEN) {
-            if(ifcp_desegment && pinfo->can_desegment) {
-                /*
-                 * This frame doesn't have all of the data for
-                 * this message, but we can do reassembly on it.
-                 *
-                 * Tell the TCP dissector where the data for this
-                 * message starts in the data it handed us, and
-                 * how many more bytes we need, and return.
-                 */
-                pinfo->desegment_offset = offset;
-                pinfo->desegment_len = iFCP_ENCAP_HEADER_LEN;
-                return -2;
-            }
+	guint16 flen, flen1;
+
+	/* we can only do this test if we have 16 bytes or more */
+	if(tvb_length_remaining(tvb, offset)<iFCP_MIN_HEADER_LEN){
+		return FALSE;
+	}
+
+	/*
+	* As per the iFCP standard, the following tests must PASS:
+	* 1)  Frame Length field validation -- 15 < Frame Length < 545;
+	* 2)  Comparison of Frame Length field to its ones complement; and
+	* 3)  A valid EOF is found in the word preceding the start of the next
+	*     iFCP header as indicated by the Frame Length field, to be tested
+	*     as follows:
+	*     1)  Bits 24-31 and 16-23 contain identical legal EOF values (the
+	*         list of legal EOF values is in the FC Frame Encapsulation
+	*         [21]); and
+	*     2)  Bits 8-15 and 0-7 contain the ones complement of the EOF
+	*         value found in bits 24-31.
+	*
+	* As per the iFCP standard, in addition, at least 3 of the following
+	* set of tests must be performed to identify that we've located the
+	* start of an iFCP frame. 
+	* a)  Protocol# ones complement field (1 test);
+	* b)  Version ones complement field (1 test);
+	* c)  Replication of encapsulation word 0 in word 1 (1 test);
+	* d)  Reserved field and its ones complement (2 tests);
+	* e)  Flags field and its ones complement (2 tests);
+	*    f)  CRC field is equal to zero (1 test); (DONT DO THIS TEST!)
+	* g)  SOF fields and ones complement fields (4 tests);
+	* h)  Format and values of FC header (1 test);
+	* i)  CRC of FC Frame (2 tests);
+	* j)  FC Frame Encapsulation header information in the next iFCP Frame
+	*     (1 test).
+	*
+	* At least 3 of the 16 tests listed above SHALL be performed. Failure
+	* of any of the above tests actually performed SHALL indicate an
+	* encapsulation error and the FC Frame SHALL NOT be forwarded on to
+	* the FC Entity.
+	*/
+
+
+	/*
+	 * Tests a, b and c
+	 */
+	if(tvb_memeql(tvb, offset, ifcp_header_8_bytes, 8) != 0){
+		return FALSE;
         }
 
-        /* I check that we have a valid header before checking for the frame
-         * length and the other initial tests.
-         */
-
-        /*
-         * Tests a, b and c
-         */
-        if (tvb_memeql(tvb, offset, ifcp_header_8_bytes, 8) != 0) {
-            offset++;
-            bytes_remaining--;
-            goto NXT_BYTE;
-        }
+	/* check the frame length */
+	flen=tvb_get_ntohs(tvb, offset+12)&0x03FF;
+	if((flen < 15) || (flen > 545)){
+		return FALSE;
+	}
 
-        flen = (tvb_get_ntohs (tvb, offset+12)) & 0x03FF;
-        frame_len = (tvb_get_ntohs (tvb, offset+12) & 0x03FF)*4;
-        
-        if ((flen < 15) || (flen > 545)) {
-            /* Frame length check failed. Skip byte and try again */
-            offset++;
-            bytes_remaining--;
-            goto NXT_BYTE;
-        }
+	/* check the complement of the frame length */
+	flen1=tvb_get_ntohs(tvb, offset+14)&0x03FF;
+	if(flen!=((~flen1)&0x03FF)){
+		return FALSE;
+	}
 
-        flen1 = (tvb_get_ntohs (tvb, offset+14)) & 0x03FF;
-        
-        if ((flen & 0x03FF) != ((~flen1)&0x03FF)) {
-            /* frame_len and its one's complement are not the same */
-            offset++;
-            bytes_remaining--;
-            goto NXT_BYTE;
-        }
 
-        if (bytes_remaining >= (frame_len)) {
-            if (tvb_bytes_exist (tvb, offset+frame_len, 8)) {
-                /* The start of the next header matches what we wish to see */ 
-                if (tvb_memeql (tvb, offset+frame_len, ifcp_header_8_bytes,
-                                8) == 0) {
-                    return (offset);
-                }
-                else {
-                    offset++;
-                    bytes_remaining--;
-                    goto NXT_BYTE;
-                }
-            }
-            else {
-                return (offset);
-            }
-        }
-        else {
-            if(ifcp_desegment && pinfo->can_desegment) {
-                /*
-                 * This frame doesn't have all of the data for
-                 * this message, but we can do reassembly on it.
-                 *
-                 * Tell the TCP dissector where the data for this
-                 * message starts in the data it handed us, and
-                 * how many more bytes we need, and return.
-                 */
-                pinfo->desegment_offset = offset;
-                pinfo->desegment_len = frame_len - bytes_remaining;
-                return -2;
-            }
-            else {
-                return (offset);
-            }
-        }
-    }
+	/* this should be good enough for our heuristics */
+	return TRUE;
+}
+
+
+#define IFCP_FLAGS_SES		0x04
+#define IFCP_FLAGS_TRP		0x02
+#define IFCP_FLAGS_SPC		0x01
+
+static const true_false_string ifcp_flags_ses_tfs = {
+	"This is a SESSION CONTROL FRAME",
+	"This is a normal frame"
+};
+
+static const true_false_string ifcp_flags_trp_tfs = {
+	"Address TRANSPARENT Mode Enabled",
+	"Address TRANSLATION Mode Enabled"
+};
 
-    return (-1);                /* Unable to find iFCP header */
+static const true_false_string ifcp_flags_spc_tfs = {
+	"This frame requires SPECIAL PROCESSING",
+	"This is a normal frame"
+};
+
+static int
+dissect_ifcpflags(tvbuff_t *tvb, int offset, proto_tree *parent_tree)
+{
+	proto_item *item=NULL;
+	proto_tree *tree=NULL;
+	guint8 flags;
+
+	if(parent_tree){
+		item=proto_tree_add_item(parent_tree, hf_ifcp_flags, tvb, offset, 1, 0);
+		tree=proto_item_add_subtree (item, ett_ifcp_flags);
+	}
+
+	flags=tvb_get_guint8(tvb, offset);
+
+	/* SES */
+	proto_tree_add_boolean(tree, hf_ifcp_flags_ses, tvb, offset, 1, flags);
+	if(flags&IFCP_FLAGS_SES){
+		proto_item_append_text(item, "  SES");
+	}
+	flags&=(~IFCP_FLAGS_SES);
+
+	/* TRP */
+	proto_tree_add_boolean(tree, hf_ifcp_flags_trp, tvb, offset, 1, flags);
+	if(flags&IFCP_FLAGS_TRP){
+		proto_item_append_text(item, "  TRP");
+	}
+	flags&=(~IFCP_FLAGS_TRP);
+
+	/* SPC */
+	proto_tree_add_boolean(tree, hf_ifcp_flags_spc, tvb, offset, 1, flags);
+	if(flags&IFCP_FLAGS_SPC){
+		proto_item_append_text(item, "  SPC");
+	}
+	flags&=(~IFCP_FLAGS_SPC);
+
+
+	offset++;
+	return offset;
 }
 
+
+#define IFCP_COMMON_FLAGS_CRCV		0x04
+
+static const true_false_string ifcp_common_flags_crcv_tfs = {
+	"CRC is VALID",
+	"Crc is NOT valid"
+};
+
 static void
-dissect_fcencap_header (tvbuff_t *tvb, proto_tree *tree, gint offset)
+dissect_commonflags(tvbuff_t *tvb, int offset, proto_tree *parent_tree)
 {
-    guint8 protocol = tvb_get_guint8 (tvb, offset);
-    
-    if (tree) {
-        proto_tree_add_uint (tree, hf_ifcp_protocol, tvb, offset, 1, protocol);
-        proto_tree_add_item (tree, hf_ifcp_version, tvb, offset+1, 1, 0);
-        proto_tree_add_item (tree, hf_ifcp_protocol_c, tvb, offset+2, 1, 0);
-        proto_tree_add_item (tree, hf_ifcp_version_c, tvb, offset+3, 1, 0);
-
-        if (protocol == FCENCAP_PROTO_iFCP) {
-            proto_tree_add_item (tree, hf_ifcp_ls_command_acc, tvb, offset+8, 1, 0);
-            proto_tree_add_item (tree, hf_ifcp_flags_ses, tvb, offset+9, 1, 0);
-            proto_tree_add_item (tree, hf_ifcp_flags_trp, tvb, offset+9, 1, 0);
-            proto_tree_add_item (tree, hf_ifcp_flags_spc, tvb, offset+9, 1, 0);
-            proto_tree_add_item (tree, hf_ifcp_sof, tvb, offset+10, 1, 0);
-            proto_tree_add_item (tree, hf_ifcp_eof, tvb, offset+11, 1, 0);
-        }
-
-        /* XXX - break out CRCV flag. */
-        proto_tree_add_item (tree, hf_ifcp_encap_flags, tvb, offset+12, 1, 0);
-        proto_tree_add_item (tree, hf_ifcp_framelen, tvb, offset+12, 2, 0);
-        proto_tree_add_item (tree, hf_ifcp_encap_flags_c, tvb, offset+14, 1, 0);
-        proto_tree_add_item (tree, hf_ifcp_framelen_c, tvb, offset+14, 2, 0);
-        proto_tree_add_item (tree, hf_ifcp_tsec, tvb, offset+16, 4, 0);
-        proto_tree_add_item (tree, hf_ifcp_tusec, tvb, offset+20, 4, 0);
-        /* XXX - check CRC if CRCV is set? */
-        proto_tree_add_item (tree, hf_ifcp_encap_crc, tvb, offset+24, 4, 0);
-    }
+	proto_item *item=NULL;
+	proto_tree *tree=NULL;
+	guint8 flags;
+
+	if(parent_tree){
+		item=proto_tree_add_item(parent_tree, hf_ifcp_common_flags, tvb, offset, 1, 0);
+		tree=proto_item_add_subtree (item, ett_ifcp_common_flags);
+	}
+
+	flags=tvb_get_guint8(tvb, offset);
+
+	/* CRCV */
+	proto_tree_add_boolean(tree, hf_ifcp_common_flags_crcv, tvb, offset, 1, flags);
+	if(flags&IFCP_COMMON_FLAGS_CRCV){
+		proto_item_append_text(item, "  CRCV");
+	}
+	flags&=(~IFCP_COMMON_FLAGS_CRCV);
 }
 
-static gboolean
-dissect_ifcp (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
-              gboolean check_port)
+static void
+dissect_ifcp_pdu(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree)
 {
-    gint offset = 0,
-         frame_len = 0;
-    gint bytes_remaining = tvb_length_remaining (tvb, offset);
-    guint8 sof = 0, eof = 0;
-   /* Set up structures needed to add the protocol subtree and manage it */
-    proto_item *ti;
-    proto_tree *ifcp_tree = NULL;
-    tvbuff_t *next_tvb;
-    
-    if (bytes_remaining < iFCP_ENCAP_HEADER_LEN) {
-        return FALSE;
-    }
-
-    if (check_port &&
-        ((pinfo->srcport != ifcp_port) && (pinfo->destport != ifcp_port))) {
-        return FALSE;
-    }
-
-    while (bytes_remaining > iFCP_ENCAP_HEADER_LEN) {
-        if ((offset = get_next_ifcp_header_offset (tvb, pinfo, offset)) == -1) { 
-            return FALSE;
-        }
-        else if (offset == -2) {
-            /* We need more data to desegment */
-            return (TRUE);
-        }
-
-        if (check_col(pinfo->cinfo, COL_PROTOCOL)) 
-            col_set_str(pinfo->cinfo, COL_PROTOCOL, "iFCP");
-
-        frame_len = (tvb_get_ntohs (tvb, offset+12) & 0x03FF)*4;
-
-        if (bytes_remaining < frame_len) {
-            if(ifcp_desegment && pinfo->can_desegment) {
-                /*
-                 * This frame doesn't have all of the data for
-                 * this message, but we can do reassembly on it.
-                 *
-                 * Tell the TCP dissector where the data for this
-                 * message starts in the data it handed us, and
-                 * how many more bytes we need, and return.
-                 */
-                pinfo->desegment_offset = offset;
-                pinfo->desegment_len = frame_len - bytes_remaining;
-                return (TRUE);
-            }
-        }
-        
-        if (tree) {
+	gint offset = 0, frame_len = 0;
+	guint8 sof = 0, eof = 0;
+	proto_item *ti;
+	proto_tree *tree = NULL;
+	tvbuff_t *next_tvb;
+	guint8 protocol;
+	proto_tree *protocol_tree=NULL;    
+	proto_tree *version_tree=NULL;    
+	proto_tree *frame_len_tree=NULL;    
+	proto_tree *sof_tree=NULL;    
+	proto_tree *eof_tree=NULL;    
+
+	/* verify we have a full header  (do we need to do this? */
+	if(tvb_length(tvb)<iFCP_ENCAP_HEADER_LEN){
+		return;
+	}
+
+	if(check_col(pinfo->cinfo, COL_PROTOCOL)){
+		col_set_str(pinfo->cinfo, COL_PROTOCOL, "iFCP");
+	}
+
+	frame_len = (tvb_get_ntohs (tvb, offset+12) & 0x03FF)*4;
+
+
+        if (parent_tree) {
             if (tvb_bytes_exist (tvb, offset, frame_len-4)) {
                 sof = tvb_get_guint8 (tvb, offset+iFCP_ENCAP_HEADER_LEN);
                 eof = tvb_get_guint8 (tvb, offset+frame_len - 4);
 
-                ti = proto_tree_add_protocol_format (tree, proto_ifcp, tvb, offset,
+                ti = proto_tree_add_protocol_format (parent_tree, proto_ifcp, tvb, offset,
                                                      iFCP_ENCAP_HEADER_LEN,
                                                      "iFCP (%s/%s)",
                                                      val_to_str (sof, ifcp_sof_vals,
                                                                  "0x%x"),
                                                      val_to_str (eof, ifcp_eof_vals,
                                                                  "0x%x"));
-            }
-            else {
+            } else {
                 sof = tvb_get_guint8 (tvb, offset+iFCP_ENCAP_HEADER_LEN);
                 
-                ti = proto_tree_add_protocol_format (tree, proto_ifcp, tvb, offset,
+                ti = proto_tree_add_protocol_format (parent_tree, proto_ifcp, tvb, offset,
                                                      iFCP_ENCAP_HEADER_LEN,
                                                      "iFCP (%s/%s)",
                                                      val_to_str (sof, ifcp_sof_vals,
                                                                  "0x%x"),
                                                      "NA");
             }
-            ifcp_tree = proto_item_add_subtree (ti, ett_ifcp);
-            /* Dissect the Common FC Encap header */
-            dissect_fcencap_header (tvb, ifcp_tree, offset);
+            tree = proto_item_add_subtree (ti, ett_ifcp);
+        }
 
 
-            /* print SOF */
-            proto_tree_add_item (ifcp_tree, hf_ifcp_sof, tvb, offset+iFCP_ENCAP_HEADER_LEN, 1, 0);
-            proto_tree_add_item (ifcp_tree, hf_ifcp_sof_c, tvb, offset+iFCP_ENCAP_HEADER_LEN+2, 1, 0);
 
-           /* print EOF */
-                
-            if (tvb_bytes_exist (tvb, offset, 4)) {
-                    proto_tree_add_item (ifcp_tree, hf_ifcp_eof, tvb, offset+frame_len-4, 1, 0);
-                    proto_tree_add_item (ifcp_tree, hf_ifcp_eof_c, tvb, offset+frame_len-2, 1, 0);
-            }
+	/* The Common FC Encap header */
+	/* protocol */
+	protocol = tvb_get_guint8 (tvb, offset);
+	ti=proto_tree_add_item(tree, hf_ifcp_protocol, tvb, offset, 1, 0);
+	if(ti){
+		protocol_tree=proto_item_add_subtree(ti, ett_ifcp_protocol);
+	}
+	offset++;
+
+	/* version */
+	ti=proto_tree_add_item(tree, hf_ifcp_version, tvb, offset, 1, 0);
+	if(ti){
+		version_tree=proto_item_add_subtree(ti, ett_ifcp_version);
+	}
+	offset++;
+
+	/* protocol complement */
+	proto_tree_add_item(protocol_tree, hf_ifcp_protocol_c, tvb, offset, 1, 0);
+	offset++;
+
+	/* version complement */
+	proto_tree_add_item(version_tree, hf_ifcp_version_c, tvb, offset, 1, 0);
+	offset++;
+
+	/* 4 reserved bytes */
+	offset+=4;
+
+	/* iFCP specific fields */
+	if(protocol==FCENCAP_PROTO_iFCP){
+		/* LS_COMMAND_ACC */
+		proto_tree_add_item(tree, hf_ifcp_ls_command_acc, tvb, offset, 1, 0);
+		offset++;
+
+		/* iFCP Flags */
+		offset=dissect_ifcpflags(tvb, offset, tree);
+
+		/* SOF */
+		ti=proto_tree_add_item(tree, hf_ifcp_sof, tvb, offset, 1, 0);
+		if(ti){
+			sof_tree=proto_item_add_subtree(ti, ett_ifcp_sof);
+		}
+		offset++;
+
+		/* EOF */
+		ti=proto_tree_add_item(tree, hf_ifcp_eof, tvb, offset, 1, 0);
+		if(ti){
+			eof_tree=proto_item_add_subtree(ti, ett_ifcp_eof);
+		}
+		offset++;
+	} else {
+		offset+=4;
+		sof_tree=tree; /* better than nothing */
+		eof_tree=tree;
+	}
+
+	/* Common Flags */
+	dissect_commonflags(tvb, offset, tree);
+
+	/* frame len */
+	ti=proto_tree_add_item(tree, hf_ifcp_framelen, tvb, offset, 2, 0);
+	if(ti){
+		frame_len_tree=proto_item_add_subtree(ti, ett_ifcp_frame_len);
+	}
+	offset+=2;
+
+	/* complement of flags and frame len */
+	proto_tree_add_item(frame_len_tree, hf_ifcp_encap_flags_c, tvb, offset, 1, 0);
+	proto_tree_add_item(frame_len_tree, hf_ifcp_framelen_c, tvb, offset, 2, 0);
+	offset+=2;
+
+	/* timestamp seconds */
+	proto_tree_add_item(tree, hf_ifcp_tsec, tvb, offset, 4, 0);
+	offset+=4;
+
+	/* timestamp fractions */
+	proto_tree_add_item(tree, hf_ifcp_tusec, tvb, offset, 4, 0);
+	offset+=4;
+
+	/* crc */
+	proto_tree_add_item(tree, hf_ifcp_encap_crc, tvb, offset, 4, 0);
+	offset+=4;
+
+
+	/* FC SOF/-SOF */
+	proto_tree_add_item(sof_tree, hf_ifcp_sof, tvb, offset, 1, 0);
+	offset++;
+	proto_tree_add_item(sof_tree, hf_ifcp_sof, tvb, offset, 1, 0);
+	offset++;
+	proto_tree_add_item(sof_tree, hf_ifcp_sof_c, tvb, offset, 1, 0);
+	offset++;
+	proto_tree_add_item(sof_tree, hf_ifcp_sof_c, tvb, offset, 1, 0);
+	offset++;
+
+	/* FC EOF/-EOF */
+	if(tvb_bytes_exist(tvb, frame_len-4, 4)) {
+		proto_tree_add_item(eof_tree, hf_ifcp_eof, tvb, frame_len-4, 1, 0);
+		proto_tree_add_item(eof_tree, hf_ifcp_eof, tvb, frame_len-3, 1, 0);
+		proto_tree_add_item(eof_tree, hf_ifcp_eof_c, tvb, frame_len-2, 1, 0);
+		proto_tree_add_item(eof_tree, hf_ifcp_eof_c, tvb, frame_len-1, 1, 0);
         }
 
 
-        /* Call the FC Dissector if this is carrying an FC frame */
-        /* Set the SOF/EOF flags in the packet_info header */
-        pinfo->sof_eof = 0;
-
-        if (sof) {
-                if ((sof == iFCP_SOFi3) || (sof == iFCP_SOFi2) || (sof == iFCP_SOFi4)) {
-                    pinfo->sof_eof = PINFO_SOF_FIRST_FRAME;
-                }
-                else if (sof == iFCP_SOFf) {
-                    pinfo->sof_eof = PINFO_SOF_SOFF;
-                }
-
-                if (eof != iFCP_EOFn) {
-                    pinfo->sof_eof |= PINFO_EOF_LAST_FRAME;
-                }
-                else if (eof != iFCP_EOFt) {
-                    pinfo->sof_eof |= PINFO_EOF_INVALID;
-                }
-        }
+	/* Call the FC Dissector if this is carrying an FC frame */
+	/* Set the SOF/EOF flags in the packet_info header */
+	pinfo->sof_eof = 0;
+
+	switch(sof){
+	case iFCP_SOFi3:
+	case iFCP_SOFi2:
+	case iFCP_SOFi4:
+		pinfo->sof_eof = PINFO_SOF_FIRST_FRAME;
+		break;
+	case iFCP_SOFf:
+		pinfo->sof_eof = PINFO_SOF_SOFF;
+		break;
+	default:
+		if(sof){
+			if (eof != iFCP_EOFn) {
+				pinfo->sof_eof |= PINFO_EOF_LAST_FRAME;
+                	} else if (eof != iFCP_EOFt) {
+				pinfo->sof_eof |= PINFO_EOF_INVALID;
+			}
+		}
+	}
             
-        next_tvb = tvb_new_subset (tvb, offset+iFCP_ENCAP_HEADER_LEN+4, frame_len-iFCP_ENCAP_HEADER_LEN-8, frame_len-iFCP_ENCAP_HEADER_LEN-8);
+	next_tvb=tvb_new_subset(tvb, offset, frame_len-offset-4, frame_len-offset-4);
 
-        if (fc_handle) {
-                call_dissector (fc_handle, next_tvb, pinfo, tree);
-        }
-        else if (data_handle) {
-                call_dissector (data_handle, next_tvb, pinfo, tree);
-        }
+	if(fc_handle){
+		call_dissector(fc_handle, next_tvb, pinfo, parent_tree);
+	} else if(data_handle){
+		call_dissector(data_handle, next_tvb, pinfo, parent_tree);
+	}
 
-        offset += frame_len;
-        bytes_remaining -= frame_len;
-    }
+	return;
+}
+
+static guint
+get_ifcp_pdu_len(tvbuff_t *tvb, int offset)
+{
+	guint pdu_len;
+ 
+	if(!ifcp_header_test(tvb, offset)){
+		return 0;
+	}
+
+	pdu_len=tvb_get_ntohs(tvb, offset+12)*4;
+	return pdu_len;
+}
 
-    return (TRUE);
+static void
+dissect_ifcp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree)
+{
+	tcp_dissect_pdus(tvb, pinfo, parent_tree, ifcp_desegment, iFCP_MIN_HEADER_LEN, get_ifcp_pdu_len, dissect_ifcp_pdu);
 }
 
+
 /* This is called for those sessions where we have explicitely said
-   this to be iFCP using "Decode As..."
-   In this case we will not check the port number for sanity and just
-   do as the user said.
-*/
+ * this to be iFCP using "Decode As..."
+ * In this case we will not check the port number for sanity and just
+ * do as the user said.
+ */
 static void
 dissect_ifcp_handle(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
-    dissect_ifcp (tvb, pinfo, tree, FALSE);
+	dissect_ifcp(tvb, pinfo, tree);
 }
 
 static gboolean
-dissect_ifcp_heur (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
+dissect_ifcp_heur(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
-    return (dissect_ifcp (tvb, pinfo, tree, TRUE));
+	if(!ifcp_header_test(tvb, 0)){
+		return FALSE;
+	}
+
+	dissect_ifcp(tvb, pinfo, tree);
+
+	/* our heuristics are so strong that if the heuristics above passed
+	 * and the dissection of the pdu did not cause any exceptions
+	 * then we can set this as our conversation dissector
+	 */
+	if(ifcp_handle){
+		conversation_t* ifcp_conv;
+
+		ifcp_conv=find_conversation(pinfo->fd->num, &pinfo->src, &pinfo->dst, pinfo->ptype, pinfo->srcport, pinfo->destport, 0);
+		if(!ifcp_conv){
+			ifcp_conv=conversation_new(pinfo->fd->num, &pinfo->src,
+				&pinfo->dst, pinfo->ptype, pinfo->srcport,
+				pinfo->destport, 0);
+		}
+		/* XXX why does this not work? it doesnt result in dissect_ifcp_handle being called    look into later*/
+		conversation_set_dissector(ifcp_conv, ifcp_handle);
+	}
+
+	return TRUE;
 }
 
 void
@@ -466,8 +581,6 @@ proto_register_ifcp (void)
         { &hf_ifcp_version_c,
           {"Version (1's Complement)", "fcencap.versionc", FT_UINT8, BASE_DEC,
            NULL, 0, "", HFILL}},
-        { &hf_ifcp_encap_flags,
-          {"iFCP Encapsulation Flags", "ifcp.encap_flags", FT_UINT8, BASE_HEX, NULL, 0xFC, "", HFILL}},
         { &hf_ifcp_encap_flags_c,
           {"iFCP Encapsulation Flags (1's Complement)", "ifcp.encap_flagsc", FT_UINT8, BASE_HEX,
            NULL, 0xFC, "", HFILL}},
@@ -500,16 +613,29 @@ proto_register_ifcp (void)
         { &hf_ifcp_ls_command_acc,
           {"Ls Command Acc", "ifcp.ls_command_acc", FT_UINT8, BASE_HEX, NULL, 0,
            "", HFILL}},
+        { &hf_ifcp_common_flags, 
+	  {"Flags", "ifcp.common_flags", FT_UINT8, BASE_HEX , NULL, 0xfc, "", HFILL }},
+        { &hf_ifcp_common_flags_crcv, 
+	  {"CRCV", "ifcp.common_flags.crcv", FT_BOOLEAN, 8, TFS(&ifcp_common_flags_crcv_tfs), IFCP_COMMON_FLAGS_CRCV, "Is the CRC field valid?", HFILL }},
+        { &hf_ifcp_flags, 
+	  {"iFCP Flags", "ifcp.flags", FT_UINT8, BASE_HEX , NULL, 0, "", HFILL }},
         { &hf_ifcp_flags_ses, 
-	  {"SES", "ifcp.flags.ses", FT_BOOLEAN, 8 , NULL, 0x04, "Is this a Session control frame", HFILL }},
+	  {"SES", "ifcp.flags.ses", FT_BOOLEAN, 8, TFS(&ifcp_flags_ses_tfs), IFCP_FLAGS_SES, "Is this a Session control frame", HFILL }},
         { &hf_ifcp_flags_trp, 
-	  {"TRP", "ifcp.flags.trp", FT_BOOLEAN, 8 , NULL, 0x02, "Is address transparent mode enabled", HFILL }},
+	  {"TRP", "ifcp.flags.trp", FT_BOOLEAN, 8, TFS(&ifcp_flags_trp_tfs), IFCP_FLAGS_TRP, "Is address transparent mode enabled", HFILL }},
         { &hf_ifcp_flags_spc, 
-	  {"SPC", "ifcp.flags.spc", FT_BOOLEAN, 8 , NULL, 0x01, "Is frame part of link service", HFILL }},
+	  {"SPC", "ifcp.flags.spc", FT_BOOLEAN, 8, TFS(&ifcp_flags_spc_tfs), IFCP_FLAGS_SPC, "Is frame part of link service", HFILL }},
     };
 
     static gint *ett[] = {
         &ett_ifcp,
+        &ett_ifcp_sof,
+        &ett_ifcp_eof,
+        &ett_ifcp_protocol,
+        &ett_ifcp_version,
+	&ett_ifcp_frame_len,
+        &ett_ifcp_flags,
+        &ett_ifcp_common_flags,
     };
     
     module_t *ifcp_module;
@@ -529,12 +655,7 @@ proto_register_ifcp (void)
                                    "Whether the iFCP dissector should reassemble messages spanning multiple TCP segments."
                                    " To use this option, you must also enable \"Allow subdissectors to reassemble TCP streams\" in the TCP protocol settings.",
                                    &ifcp_desegment);
-    prefs_register_uint_preference(ifcp_module,
-                                   "target_port",
-                                   "Target port",
-                                   "Port number used for iFCP",
-                                   10,
-                                   &ifcp_port);
+    prefs_register_obsolete_preference(ifcp_module, "target_port");
 }
 
 
@@ -550,8 +671,6 @@ proto_register_ifcp (void)
 void
 proto_reg_handoff_ifcp (void)
 {
-    dissector_handle_t ifcp_handle;
-
     heur_dissector_add("tcp", dissect_ifcp_heur, proto_ifcp);
 
     ifcp_handle = create_dissector_handle(dissect_ifcp_handle, proto_ifcp);