Wireshark-dev: Re: [Wireshark-dev] Using find_conversation with multiple conversations conducte
sake@xxxxxxxxxx wrote on 27/06/2009 09:52:26 AM:
> Hi Kelvin,
>
> The radius dissectors uses a hash table to store
multiple sessions
> per conversation. You might want to have a look at it (epan/
> dissectors/packet-radius.c). If you need a tracefile to work with,
> showing multiple requests/responses in the same conversation, I
> could create one for you...
>
> Hope this helps,
> Cheers,
> Sake
Thanks for that tip Sake.
I also found the following little note in README.developer
(where I should have looked first I suppose...)
> 2.2.10 The example conversation code using conversation
index field.
>
> Sometimes the conversation isn't enough to define
a unique data storage
> value for the network traffic. For example
if you are storing information
> about requests carried in a conversation, the
request may have an
> identifier that is used to define the request.
In this case the
> conversation and the identifier are required
to find the data storage
> pointer. You can use the conversation data
structure index value to
> uniquely define the conversation.
>
> See packet-afs.c for an example of how to use
the conversation index. In
> this dissector multiple requests are sent in
the same conversation. To store
> information for each request the dissector has
an internal hash table based
> upon the conversation index and values inside
the request packets.
So based on packet-radius.c and packet-afs.c I've
created a patch that seems to solve my problem. I maintain a hashtable
of DNP3.0 data-link conversations that I lookup based on the TCP/UDP conversation
(via find_conversation) and the DNP3.0 source and destination addresses.
I've not actually contributed a patch back to the
Wireshark before (I've mainly worked on internal plugins) so I'm looking
for some help / assistance on this one.
I've supplied a patch based on the latest trunk code
for review / comment - to see if I'm at least on the right track.
Graham Bloice also mentioned raising a bug so I can
attach pcap files etc... for testing - Is this still the right approach
now that I have a proposed patch?
Thanks for your assistance.
Cheers,
Kelvin
The patch follows:
Index: packet-dnp.c
===================================================================
--- packet-dnp.c (revision
28866)
+++ packet-dnp.c (working
copy)
@@ -925,7 +925,52 @@
/* Tables for reassembly of fragments. */
static GHashTable *al_fragment_table = NULL;
static GHashTable *al_reassembled_table = NULL;
+static GHashTable *dll_conversation_table = NULL;
+typedef struct _dll_conversation_key
+{
+ guint32 conversation;
/* TCP / UDP conversation */
+ guint16 src;
/* DNP3.0 Source
Address */
+ guint16 dst;
/* DNP3.0 Destination
Address */
+} dll_conversation_key_t;
+
+/* Conversation structure */
+typedef struct {
+ guint conv_seq_number;
+} dnp3_conv_t;
+
+/* The conversation sequence number */
+static guint seq_number = 0;
+
+/* Conversation key equality function */
+static gint
+dll_conversation_equal(gconstpointer v, gconstpointer
w)
+{
+ const dll_conversation_key_t* v1 = (const
dll_conversation_key_t*)v;
+ const dll_conversation_key_t* v2 = (const
dll_conversation_key_t*)w;
+
+ if (v1->conversation == v2->conversation
&&
+ v1->src == v2->src &&
+ v1->dst == v2->dst)
+ {
+ return 1;
+ }
+
+ return 0;
+}
+
+/* Conversation key hash function */
+static guint
+dll_conversation_hash(gconstpointer v)
+{
+ const dll_conversation_key_t
*key = (const dll_conversation_key_t*)v;
+ guint val;
+
+ val = key->conversation
+ (key->src << 16) + key->dst;
+
+ return val;
+}
+
/* *************************************************************************
*/
/*
Header values for reassembly
*/
/* *************************************************************************
*/
@@ -954,18 +999,6 @@
"DNP 3.0 fragments"
};
-/* Conversation stuff, used for tracking application
message fragments */
-/* the number of entries in the memory chunk array
*/
-#define dnp3_conv_init_count 50
-
-/* Conversation structure */
-typedef struct {
- guint conv_seq_number;
-} dnp3_conv_t;
-
-/* The conversation sequence number */
-static guint seq_number = 0;
-
/* desegmentation of DNP3 over TCP */
static gboolean dnp3_desegment = TRUE;
@@ -2311,6 +2344,7 @@
gboolean update_col_info
= TRUE;
conversation_t *conversation;
dnp3_conv_t *conv_data_ptr;
+ dll_conversation_key_t dll_conversation_key;
@@ -2524,18 +2558,33 @@
conversation
= conversation_new(pinfo->fd->num, &pinfo->src, &pinfo->dst,
pinfo->ptype,
pinfo->srcport,
pinfo->destport, 0);
}
+
+
/*
+
* The TCP/UDP conversation is not sufficient to identify
a conversation
+
* on a multi-drop DNP network. Lookup conversation
data based on TCP/UDP
+
* conversation and the DNP src and dst addresses
+
*/
- conv_data_ptr = (dnp3_conv_t*)conversation_get_proto_data(conversation,
proto_dnp3);
+ dll_conversation_key.conversation
= conversation->index;
+ dll_conversation_key.src
= "">
+ dll_conversation_key.dst
= dl_dst;
- if (conv_data_ptr ==
NULL) {
- /* New data structure
required */
+ conv_data_ptr = (dnp3_conv_t*)g_hash_table_lookup(dll_conversation_table,
&dll_conversation_key);
+
+ if(!pinfo->fd->flags.visited
&& conv_data_ptr == NULL)
+ {
+ dll_conversation_key_t*
new_dll_conversation_key = NULL;
+ new_dll_conversation_key
= se_alloc(sizeof(dll_conversation_key_t));
+ *new_dll_conversation_key
= dll_conversation_key;
+
conv_data_ptr
= se_alloc(sizeof(dnp3_conv_t));
-
+
/*** Increment
static global fragment reassembly id ***/
conv_data_ptr->conv_seq_number
= seq_number++;
- conversation_add_proto_data(conversation,
proto_dnp3, (void *)conv_data_ptr);
- }
+ g_hash_table_insert(dll_conversation_table,
new_dll_conversation_key, conv_data_ptr);
+
}
+
conv_seq_number
= conv_data_ptr->conv_seq_number;
/*
@@ -2647,6 +2696,11 @@
static void
dnp3_init(void)
{
+ if (dll_conversation_table)
+ {
+ g_hash_table_destroy(dll_conversation_table);
+ }
+ dll_conversation_table = g_hash_table_new(dll_conversation_hash,
dll_conversation_equal);
}
static void
