Move the definition of the structure constructed for each SPX

transmission (and shared by all retransmissions), and passed to SPX
subdissectors, to "packet-ipx.h", and use the same structure in the SPX
dissector and the NDPS dissector.

Set up conversations and those structures without checking whether we've
seen the packet before or not; just check whether we find the
conversation before creating a new one, and check whether we find a
structure for the packet before creating a new one.  Pass it to the
subdissector regardless of whether we've seen the packet before or not,
and check it in the NDPS dissector regardless of whether we've seen it
before or not.

Don't store a "retransmission" flag in the structure - the initial
transmission and the retransmissions all share a single data structure,
but they don't all have the same value for the "retransmission" flag,
and you can tell whether a packet is a retransmission or not by
comparing its frame number with the frame number from the structure; if
they're different, it's a retransmission.

git-svn-id: http://anonsvn.wireshark.org/wireshark/trunk@7418 f5534014-38df-0310-8fa8-9805f1628bb7

diff --git a/packet-ipx.c b/packet-ipx.c
index 657df6fd6fc28fdc58256c8fdd2bed41c913b518..2d295ae858f90571f52f4adbed1a3f52390f0a5c 100644 (file)
--- a/packet-ipx.c
+++ b/packet-ipx.c
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 2000-2002 by Gilbert Ramirez.
  * Portions Copyright (c) Novell, Inc. 2002-2003
  *
- * $Id: packet-ipx.c,v 1.123 2003/04/08 00:39:27 guy Exp $
+ * $Id: packet-ipx.c,v 1.124 2003/04/08 02:00:53 guy Exp $
  *
  * Ethereal - Network traffic analyzer
  * By Gerald Combs <gerald@ethereal.com>
@@ -354,14 +354,6 @@ typedef struct {
        guint32             spx_src;
 } spx_hash_key;
 
-typedef struct {
-        guint16             spx_seq;
-        guint16             spx_ack;
-        guint16             spx_all;
-        guint32             num;
-        gboolean            retransmission;
-} spx_hash_value;
-
 static GHashTable *spx_hash = NULL;
 static GMemChunk *spx_hash_keys = NULL;
 static GMemChunk *spx_hash_values = NULL;
@@ -406,8 +398,8 @@ spx_init_protocol(void)
                        SPX_PACKET_INIT_COUNT * sizeof(spx_hash_key),
                        G_ALLOC_ONLY);
        spx_hash_values = g_mem_chunk_new("spx_hash_values",
-                       sizeof(spx_hash_value),
-                       SPX_PACKET_INIT_COUNT * sizeof(spx_hash_value),
+                       sizeof(spx_info),
+                       SPX_PACKET_INIT_COUNT * sizeof(spx_info),
                        G_ALLOC_ONLY);
 }
 
@@ -430,11 +422,11 @@ spx_postseq_cleanup(void)
         * needed during random-access processing of the proto_tree.*/
 }
 
-spx_hash_value*
+spx_info*
 spx_hash_insert(conversation_t *conversation, guint32 spx_src)
 {
        spx_hash_key            *key;
-       spx_hash_value          *value;
+       spx_info                *value;
 
        /* Now remember the packet, so we can find it if we later. */
        key = g_mem_chunk_alloc(spx_hash_keys);
@@ -446,7 +438,6 @@ spx_hash_insert(conversation_t *conversation, guint32 spx_src)
        value->spx_ack = 0;
        value->spx_all = 0;
        value->num = 0;
-       value->retransmission = FALSE;
        
        g_hash_table_insert(spx_hash, key, value);
 
@@ -454,7 +445,7 @@ spx_hash_insert(conversation_t *conversation, guint32 spx_src)
 }
 
 /* Returns the spx_rec*, or NULL if not found. */
-spx_hash_value*
+spx_info*
 spx_hash_lookup(conversation_t *conversation, guint32 spx_src)
 {
        spx_hash_key            key;
@@ -520,7 +511,7 @@ dissect_spx(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
        const char      *spx_msg_string;
        guint16         low_socket, high_socket;
        guint32         src;
-       spx_hash_value  *pkt_value = NULL;
+       spx_info        *pkt_value = NULL;
        conversation_t  *conversation = NULL;
 
        if (check_col(pinfo->cinfo, COL_PROTOCOL))
@@ -555,56 +546,70 @@ dissect_spx(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
                proto_tree_add_item(spx_tree, hf_spx_ack_nr, tvb,  8, 2, FALSE);
                proto_tree_add_item(spx_tree, hf_spx_all_nr, tvb, 10, 2, FALSE);
        }
-       src = tvb_get_ntohs(tvb, 0)+tvb_get_ntohs(tvb, 2)+tvb_get_ntohs(tvb, 4)+tvb_get_ntohs(tvb, 6)+tvb_get_ntohs(tvb, 8);
-       /* SPX is Connection Oriented and Delivery Guaranteed
-        *  We need to flag retransmissions by the SPX protocol
+
+       /*
+        * SPX is Connection Oriented and Delivery Guaranteed.
+        * We need to flag retransmissions by the SPX protocol, so that
+        * subdissectors know whether a packet was retransmitted.
+        *
+        * We start out by creating a conversation for this IPX session.
+        * XXX - should we be using "pinfo->srcport" twice, or
+        * "pinfo->srcport" and "pinfo->destport"?  For that matter, should
+        * we just be using PT_IPX, and get rid of PT_NCP?
         */
-       if (!pinfo->fd->flags.visited) {
-               conversation = find_conversation(&pinfo->src, &pinfo->dst,
+       conversation = find_conversation(&pinfo->src, &pinfo->dst,
+           PT_NCP, pinfo->srcport, pinfo->srcport, 0);
+       if (conversation == NULL) {
+               /*
+                * It's not part of any conversation - create a new one.
+                */
+               conversation = conversation_new(&pinfo->src, &pinfo->dst,
                    PT_NCP, pinfo->srcport, pinfo->srcport, 0);
-    
-               if (conversation == NULL) {
-                       /*
-                        * It's not part of any conversation - create a new
-                        * one.
-                        */
-                       conversation = conversation_new(&pinfo->src,
-                           &pinfo->dst, PT_NCP, pinfo->srcport,
-                           pinfo->srcport, 0);
-                       pkt_value = spx_hash_insert(conversation, src);
-                       pkt_value->spx_seq = tvb_get_ntohs(tvb, 6);
-                       pkt_value->spx_ack = tvb_get_ntohs(tvb, 8);
-                       pkt_value->spx_all = tvb_get_ntohs(tvb, 10);
-                       pkt_value->num = pinfo->fd->num;
-                       pkt_value->retransmission = FALSE;
-               } else {
-                       pkt_value = spx_hash_lookup(conversation, src);
-                       if (pkt_value &&
-                           tvb_reported_length_remaining(tvb, SPX_HEADER_LEN) > 0) {
-                               if (check_col(pinfo->cinfo, COL_INFO)) {
-                                       col_add_fstr(pinfo->cinfo, COL_INFO,
-                                       "[Retransmission] Original Packet %u",
-                                       pkt_value->num);
-                               }
-                               pkt_value->spx_seq = tvb_get_ntohs(tvb, 6);
-                               pkt_value->spx_ack = tvb_get_ntohs(tvb, 8);
-                               pkt_value->spx_all = tvb_get_ntohs(tvb, 10);
-                               pkt_value->retransmission = TRUE;
-                       } else {
-                               pkt_value = spx_hash_insert(conversation, src);
-                               pkt_value->spx_seq = tvb_get_ntohs(tvb, 6);
-                               pkt_value->spx_ack = tvb_get_ntohs(tvb, 8);
-                               pkt_value->spx_all = tvb_get_ntohs(tvb, 10);
-                               pkt_value->num = pinfo->fd->num;
-                               pkt_value->retransmission = FALSE;
-                       }
-               }
+       }
+
+       /*
+        * Now we'll hash the SPX header and use the result of that,
+        * plus the conversation, as a hash key to identify this packet,
+        * and, if it's not already in the SPX packet hash table, enter
+        * it into that hash table so we can detect retransmissions.
+        *
+        * On the first pass, if we don't find it in the hash table, it's
+        * not a retransmission, otherwise it is.  If we don't find it,
+        * we enter it into the hash table, with the frame number.
+        *
+        * On subsequent passes, we should find it in the frame table
+        * whether it's a retransmission or not; it's a retransmission
+        * iff the frame number doesn't match the frame number in the
+        * hash table.
+        */
+       src = tvb_get_ntohs(tvb, 0)+tvb_get_ntohs(tvb, 2)+tvb_get_ntohs(tvb, 4)+tvb_get_ntohs(tvb, 6)+tvb_get_ntohs(tvb, 8);
+       pkt_value = spx_hash_lookup(conversation, src);
+       if (pkt_value == NULL) {
                /*
-                * Store the spx info so that subidissectors know what this
-                * is.
-                * Note: num will equal retransmitted source packet number.
+                * Not found in the hash table.
+                * Enter it into the hash table.
                 */
-               pinfo->private_data = pkt_value;
+               pkt_value = spx_hash_insert(conversation, src);
+               pkt_value->spx_seq = tvb_get_ntohs(tvb, 6);
+               pkt_value->spx_ack = tvb_get_ntohs(tvb, 8);
+               pkt_value->spx_all = tvb_get_ntohs(tvb, 10);
+               pkt_value->num = pinfo->fd->num;
+       }
+
+       /*
+        * It's a retransmission iff the frame number in the hash table
+        * entry (which is the frame number of the original transmission)
+        * isn't the frame number of this frame.
+        *
+        * XXX - put something into the protocol tree as well?
+        * If so, give the frame number, as an FT_FRAMENUM.
+        */
+       if (pkt_value->num != pinfo->fd->num) {
+               if (check_col(pinfo->cinfo, COL_INFO)) {
+                       col_add_fstr(pinfo->cinfo, COL_INFO,
+                           "[Retransmission] Original Packet %u",
+                           pkt_value->num);
+               }
        }
 
        if (tvb_reported_length_remaining(tvb, SPX_HEADER_LEN) > 0) {
@@ -631,6 +636,13 @@ dissect_spx(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
                        high_socket = pinfo->destport;
                }
 
+               /*
+                * Pass the SPX info to subdissectors, so that they know
+                * what this is.
+                * Note: num will equal retransmitted source packet number.
+                */
+               pinfo->private_data = pkt_value;
+
                next_tvb = tvb_new_subset(tvb, SPX_HEADER_LEN, -1, -1);
                if (dissector_try_port(spx_socket_dissector_table, low_socket,
                    next_tvb, pinfo, tree))

diff --git a/packet-ipx.h b/packet-ipx.h
index 99fbe66175c87b72480381c893056cbf944c4356..cabccc32966db1a48e12b822001764ca6a0c1ddc 100644 (file)
--- a/packet-ipx.h
+++ b/packet-ipx.h
@@ -6,7 +6,7 @@
  * Portions Copyright (c) by Gilbert Ramirez 2000-2002
  * Portions Copyright (c) Novell, Inc. 2002-2003
  *
- * $Id: packet-ipx.h,v 1.24 2003/04/08 00:22:26 guy Exp $
+ * $Id: packet-ipx.h,v 1.25 2003/04/08 02:00:53 guy Exp $
  *
  * Ethereal - Network traffic analyzer
  * By Gerald Combs <gerald@ethereal.com>
@@ -151,4 +151,18 @@ extern const value_string server_vals[];
 
 void capture_ipx(packet_counts *);
 
+/*
+ * Structure passed to subdissectors of the SPX dissector.
+ * It contains fields from the SPX header, plus the frame number.
+ * It's kept in a hash table, so if an SPX packet is retransmitted,
+ * there's only one copy, and "num" is the frame number of the first
+ * transmission.
+ */
+typedef struct _spx_info{
+        guint16             spx_seq;
+        guint16             spx_ack;
+        guint16             spx_all;
+        guint32             num;
+} spx_info;
+
 #endif

diff --git a/packet-ndps.c b/packet-ndps.c
index 72ec2ff5280fcff89095a2bdd803b6ffadc16ed7..513b46fedf94ccf1d93501a753455bbcb627b95a 100644 (file)
--- a/packet-ndps.c
+++ b/packet-ndps.c
@@ -3,7 +3,7 @@
  * Greg Morris <gmorris@novell.com>
  * Copyright (c) Novell, Inc. 2002-2003
  *
- * $Id: packet-ndps.c,v 1.11 2003/04/08 00:56:17 guy Exp $
+ * $Id: packet-ndps.c,v 1.12 2003/04/08 02:00:53 guy Exp $
  *
  * Ethereal - Network traffic analyzer
  * By Gerald Combs <gerald@ethereal.com>
@@ -3320,14 +3320,12 @@ ndps_defrag(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
     guint32         id=0;
     int             len=0;
     tvbuff_t        *next_tvb = NULL;
-       fragment_data   *fd_head;
+    fragment_data   *fd_head;
     spx_info        *spx_data;
 
-    if (!pinfo->fd->flags.visited) {
-        spx_data = pinfo->private_data;
-    }
-    if (!ndps_defragment || spx_data->retransmission == TRUE) {
-        if (spx_data->retransmission == TRUE) {
+    spx_data = pinfo->private_data;
+    if (!ndps_defragment || spx_data->num != pinfo->fd->num) {
+        if (spx_data->num != pinfo->fd->num) {
             if (check_col(pinfo->cinfo, COL_INFO))
             {
                 col_add_fstr(pinfo->cinfo, COL_INFO, "[Retransmission] Original Packet %d", spx_data->num);

diff --git a/packet-ndps.h b/packet-ndps.h
index 258872b94fab33ec516b001d5f5defa6ffc497c0..ed4962c4719af08eacafde5c1774f6980541b9cd 100644 (file)
--- a/packet-ndps.h
+++ b/packet-ndps.h
@@ -4,7 +4,7 @@
  *
  * Copyright (c) Novell, Inc. 2002-2003
  *
- * $Id: packet-ndps.h,v 1.3 2003/04/08 00:56:17 guy Exp $
+ * $Id: packet-ndps.h,v 1.4 2003/04/08 02:00:54 guy Exp $
  *
  * Ethereal - Network traffic analyzer
  * By Gerald Combs <gerald@ethereal.com>
@@ -32,11 +32,3 @@
 #define TCP_PORT_ENS                    0x0bc8 /* NDPS Event Notification Service */
 #define TCP_PORT_RMS                    0x0bcb /* NDPS Remote Management Service */
 #define TCP_PORT_NOTIFY_LISTENER        0x0bc9 /* NDPS Notify Listener */
-
-typedef struct _spx_info{
-        guint16             spx_seq;
-        guint16             spx_ack;
-        guint16             spx_all;
-        guint32             num;
-        gboolean            retransmission;
-} spx_info;