This is our old Q&A Site. Please post any new questions and answers at ask.wireshark.org.
0
1

Hey Guys,

So, I'm developing a dissector for a custom protocol (let's call it foo for now), part of that protocol is an incrementing sequence number, which we call a heartbeat, it's synonymous with "Packet Number" (this hbt = last_hbt++, very simple to check to see if we missed a packet).

I've been trying to get this working with the conversation interface, but I've run into an issue. I can't work out how to compare two adjacent packets' heartbeats.

Here's what I've got for the conversation code.

struct _foo_conversation_info
{
    guint16  heartbeat;
};

/* Code to actually dissect the packets */
static void
dissect_foo(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
{

/* some variables are omitted for brevity */

conversation_t *htbt_conv;
guint16     last_heartbeat;
struct _foo_conversation_info *proto_data;

/* Some function code is omitted for brevity */

SET_ADDRESS(&null_addr, AT_NONE, 0, NULL);
htbt_conv = find_conversation(pinfo->fd->num, &pinfo->src,
    &null_addr, pinfo->ptype, pinfo->destport, 0x0,
    NO_ADDR2|NO_PORT2);
if(!htbt_conv){
    htbt_conv = conversation_new(pinfo->fd->num, &pinfo->src,
    &null_addr, pinfo->ptype, pinfo->destport, 0x0,
    NO_ADDR2|NO_PORT2);
}

proto_data  = conversation_get_proto_data(htbt_conv, proto_foo);

if(!proto_data){
    proto_data = se_alloc0(sizeof(struct _foo_conversation_info));
    conversation_add_proto_data(htbt_conv, proto_foo,
        proto_data);

    last_heartbeat  = 0;
    proto_data->heartbeat = tvb_get_ntohs(tvb, 1);
    col_set_str(pinfo->cinfo, COL_INFO, "FOO");
}else{

    last_heartbeat  = proto_data->heartbeat;
    proto_data->heartbeat = tvb_get_ntohs(tvb, 1);

    if((proto_data->heartbeat - last_heartbeat) != 1){
        col_set_str(pinfo->cinfo, COL_INFO, "FOO - Skipped Heartbeat!");
    }else{
        col_set_str(pinfo->cinfo, COL_INFO, "FOO");
    }
}

Every packet in the packet list is marked "Skipped Heartbeat!", when it really shouldn't be (I have a packet generator with a switch so I can inject bad packets BTW).

I guess this is because the dissector isn't being passed the packets in order, as I would expect, so my next step was to look at TCP to see how it does it(packet-tcp.c & packet-tcp.h), but I just can't follow it, there are whole function chains which seem to go nowhere, so, how the hell do I step through the conversation chain and test for no sequential packets??

Thanks in advance,

Craig.

asked 20 Sep '12, 06:34

CTNOBLE's gravatar image

CTNOBLE
11236
accept rate: 0%

edited 20 Sep '12, 07:01

SYN-bit's gravatar image

SYN-bit ♦♦
17.1k957245


The problem is that Wireshark will run through the packets once sequentially, but also goes through the packets at random to create the protocol tree to display.

Every frame will be marked "visited" on the first run, so you can use this to flag to do the dissection of each frame. If your dissection depends on the order of frames, use the following macro:

if (!PINFO_FD_VISITED(pinfo)) {
    <your code here>
}

Also make sure that you need to save your dissection results that are sequence dependent in "per packet data" in the conversation table. See "2.5 Per-packet information." in doc/README.developer. For more details :-)

permanent link

answered 20 Sep '12, 07:07

SYN-bit's gravatar image

SYN-bit ♦♦
17.1k957245
accept rate: 20%

Thanks for posting an answer, I do appreciate it, but I need some more info please.

My question my have been a bit vague on exactly what I wanted to know.

I was aware that Wireshark will run through sequentially the first time and then might run through at random on any subsiquent runs.

Which is what the conversation is for, I was under the impression that the "find_conversation()" function would return a different per-packet data structure based on the frame number (first argument to that function), is that true? That said your "visited" macro looks more robust, so I'll go with that.

(20 Sep '12, 07:25) CTNOBLE

not enough chars for full comment

If I was to call find_conversation(pinfo->fd->num--, ...) would that return the information from the previous frame in my stream or are the frame numbers not necessarily continuous? Would using a simple global variable be safe enough?

Basically how do I get data from the previous frame dissection into the current one?

Cheers, Craig.

(20 Sep '12, 07:26) CTNOBLE

find_conversation() will just give you the index to the conversation data based on addresses, ports and frame_number (as there might be more conversations with the same addresses/ports credentials).

A global variable will not do and neither will data inside your conversation data object, as you need to have the history at hand when your dissector is called for a specific frame.

You need to use the per-packet data to store states per packet.

So, please read the before mentioned paragraph of README.developer, which kind of explains this in more detail.

(20 Sep '12, 08:01) SYN-bit ♦♦

Thanks for the clarification, on conversations.

So here's some psudo code for what I think I need to do:

if(packet not seen before){
    per-packet.lastheartbeat   <- conversation.lastheartbeat
    conversation.lastheartbeat <- heartbeat
}else{
    lastheartbeat <- per-packet.lastheartbeat

    if(heartbeat-lastheartbeat != 1)
    {
        do some error stuff
    }
}

I'll keep experimenting and post back here next week with the actual code just in case anyone else wants to see how I ended up solving this.

Cheers, Craig.

(20 Sep '12, 09:44) CTNOBLE

Not exactly, the only time to detect if a heartbeat was lost is in the sequential run, so the check whether a heartbeat was lost should be done in the "packet not seen" block. The result should be stored in the per-packet information.

(20 Sep '12, 10:04) SYN-bit ♦♦

Something like this:

if(packet not seen before){
    if( packet.heartbeat - conversation.lastheartbeat != 1)
        per-packet.skippedheartbeat = TRUE
    } else {
        per-packet.skippedheartbeat = FALSE
    }
    conversation.lastheartbeat = packet.heartbeat
}

if ( tree ) {
    ...
    get-per-packet-information()
    if( per-packet.skippedheartbeat ) {
       show-error-in-tree()
    }
    ...
}
(20 Sep '12, 10:05) SYN-bit ♦♦

Ahhhh, this just showed up when I hit OK for my code below.

I guess not retesting everytime might reduce the processing power needed, but wouldn't this be functionally equivalent code?

Good practice is always worth the effort - I'll change it next week and edit my answer post.

Thanks again!

(20 Sep '12, 10:11) CTNOBLE
showing 5 of 7 show 2 more comments

OK, So I was able to get this working a lot faster than I thought. Here's my code.

    SET_ADDRESS(&null_addr, AT_NONE, 0, NULL);

        pcket_data = p_get_proto_data(pinfo->fd, proto_foo);

        if (!(pinfo->fd->flags.visited)){
            if(!pcket_data){
                pcket_data = se_alloc(sizeof(struct _foo_packet_info));
                p_add_proto_data(pinfo->fd, proto_foo, pcket_data);
            }

            htbt_conv = find_conversation(pinfo->fd->num, &pinfo->src,
                &null_addr, pinfo->ptype, pinfo->destport, 0x0,
                NO_ADDR2|NO_PORT2);

            if(!htbt_conv){
                htbt_conv = conversation_new(pinfo->fd->num, &pinfo->src,
                &null_addr, pinfo->ptype, pinfo->destport, 0x0,
                NO_ADDR2|NO_PORT2);
            }

            proto_data  = conversation_get_proto_data(htbt_conv, proto_foo);

            if(!proto_data){
                proto_data = se_alloc0(sizeof(struct _foo_conversation_info));
                conversation_add_proto_data(htbt_conv, proto_foo,
                    proto_data);

                last_heartbeat  = 0;
            }

            this_heartbeat = tvb_get_ntohs(tvb, 2);
            last_heartbeat = proto_data->heartbeat;
            proto_data->heartbeat = this_heartbeat;
            pcket_data->last_heartbeat = last_heartbeat;
        }

        last_heartbeat  = pcket_data->last_heartbeat;
        this_heartbeat = tvb_get_ntohs(tvb, 2);

        col_clear(pinfo->cinfo, COL_INFO);

        if((this_heartbeat - last_heartbeat) != 1){
            col_set_str(pinfo->cinfo, COL_INFO, "foo Aurora Packet - Skipped Heartbeat!");
        }else{          
            col_set_str(pinfo->cinfo, COL_INFO, "foo Aurora Packet");
        }

If anyone has comments on the style or safety of my code then that would be appreciated, otherwise I'm going to mark this as the answer early next week.

Thankyou SYN-bit for your help and guidance.

permanent link

answered 20 Sep '12, 10:09

CTNOBLE's gravatar image

CTNOBLE
11236
accept rate: 0%

Your answer
toggle preview

Follow this question

By Email:

Once you sign in you will be able to subscribe for any updates here

By RSS:

Answers

Answers and Comments

Markdown Basics

  • *italic* or _italic_
  • **bold** or __bold__
  • link:[text](http://url.com/ "title")
  • image?![alt text](/path/img.jpg "title")
  • numbered list: 1. Foo 2. Bar
  • to add a line break simply add two spaces to where you would like the new line to be.
  • basic HTML tags are also supported

Question tags:

×637
×158
×33

question asked: 20 Sep '12, 06:34

question was seen: 3,575 times

last updated: 20 Sep '12, 10:12

p​o​w​e​r​e​d by O​S​Q​A