Thanks Roland/Jeff for the responses, much appreciated.

So spent a bit of time debugging this and it looks like we fail sometimes to 
return a valid frag_msg from a call to fragment_add_seq_check() when more_frags 
is set to false.  In the case this happens I am currently failing to see much 
difference in how the id and frag_number are controlled i.e. they are unique 
and in-sequence respectively.  The failing point is that we receive NULL from 
lookup_fd_head() from within fragment_add_seq_common() so I assume 
g_hash_table_lookup_extended() fails in someway.

So perhaps I need to follow your advice Roland and use 
fragment_add_seq_offset() at the beginning of each fragment sequence, though I 
am unclear why this should be needed.  Or I need to follow the packet-mp2t.c 
code which manipulates pinfo->src and pinfo->dst?

Otherwise I have added some code in my payload parse function to make use of 
fragment_get() to infer when the packets payload has already being parsed and 
hence don’t need to re-check the legitimacy of the begin/end/seq-numbers.  I am 
not at the point of testing this since we are failing on the initial file parse 
now.

Regards,

John

From: wireshark-dev-boun...@wireshark.org 
[mailto:wireshark-dev-boun...@wireshark.org] On Behalf Of Roland Knall
Sent: 28 July 2016 15:39
To: Developer support list for Wireshark
Subject: Re: [Wireshark-dev] Question on payload reassembly

Hi

Just a short question, does your sequence counter repeat? If so, this can be an 
issue. Also, for the openSAFETY dissector it only worked properly, after I 
implemented fragment_add_seq_offset, so it will allways count internally 
beginning with 0. You can see that in line 1272 of packet-opensafety.c

regards,
Roland

On Thu, Jul 28, 2016 at 3:38 PM, Jeff Morriss 
<jeff.morriss...@gmail.com<mailto:jeff.morriss...@gmail.com>> wrote:


On Thu, Jul 28, 2016 at 8:35 AM, John Dunlop 
<jdun...@maxlinear.com<mailto:jdun...@maxlinear.com>> wrote:

Hi,



Hope someone can help me with a question of payload reassembly.



First up, I have been trawling the e-mail archives to find an equivalent answer 
and was wondering if there is a better way of searching the e-mail archives 
than opening up each individual month/year?

Personally I use Google with a search string like:
    what I'm interested in site://wireshark.org<http://wireshark.org>


Now my actual question is that I am dissecting  a packet payload which is split 
up into fragments with specific chunks as:

Begin

Middle (no begin/end flagged, so can be multiple)

End



I have a simple state machine that checks these transitions and keeps fragment 
counts so I can then call fragment_add_seq_check() with an appropriate unique 
id and an incrementing (from zero) frag_number.  I know the size of the 
individual fragments and there is a sequence number that increments on each 
packet, though a packet can have multiple fragments for the same or different 
channels .



This appears to ‘initially’ work ok from the various log prints I had added to 
check returns from process_reassembled_data() and the actual reassembled TVB 
size.



The problem I have, and this is probably my fundamental misunderstanding, is 
that it works on the initial pass through the packets but breaks horribly when 
I click on an individual packet as we are mid fragments. I also notice that 
wireshark parses the whole file once and then parses again the visible packets 
in the summary window, this also fails as the 1st packet is parsed again after 
the last which could be in any state of fragmentation.



I suppose I am thinking if we have parsed the payload once for a given 
packet/fragment we should not parse and reassemble again but somehow look-up 
what reassembled payload it belongs to? Using something like fragment_get() ?

Hmm, the reassembly routines should take care of this for you.  See the first 
'if' statement in `fragment_add_seq_check_work()` (in epan/reassemble.c): it 
checks if the current frame has already been dissected and, if so, it skips 
reassembly and just returns what was stored from the first pass.

It sounds like you are but are you *really* sure you're doing all the 
reassembly on the first pass (e.g., the reassembly calls aren't buried under an 
`if(tree)` for example)?
I suppose this won't answer your question but hopefully it might give you a 
direction to look in...

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list 
<wireshark-dev@wireshark.org<mailto:wireshark-dev@wireshark.org>>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             
mailto:wireshark-dev-requ...@wireshark.org<mailto:wireshark-dev-requ...@wireshark.org>?subject=unsubscribe

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@wireshark.org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Reply via email to