Wireshark-dev: Re: [Wireshark-dev] [PATCH] Adding RTSE reassembly
From: "Graeme Lunt" <graeme.lunt@xxxxxxxxx>
Date: Sat, 23 Jun 2007 11:54:14 +0200
Stig,

> This patch adds RTSE reassembly.  The reassembly is done when
> receiving a SES MAJOR SYNC POINT, as this indicates the end of the
> COTP DT Data stream.  Previous the RTSE dissector was called when
> receiving a COTP DT Data fragment with the "last data unit" bit set,
> but this does not work with messages fragmented in RTSE.  Reassembly
> can be turned off in the preferences.

Thanks for this - a useful addition.
 
> The RTSE reassembly is a bit magic because of the OCTET_STRING
> encapsulation (the data should have been encapsulated in a constructed
> OCTET_STRING after reassembly, which is not possible in wireshark
> (?)), but this is fixed with calling dissect_rtse_EXTERNAL() instead
> of dissect_rtse_RTSE_apdus() for such constructions.

I *think* this can be made a bit more generic and just call
dissect_ber_octet_string 
to extract the data - this will cope better when subsequent fragments use
constructed octet strings.

> Another feature with this patch is that the info column shows info
> from the RTSE content instead of SES "MAJOR SYNC POINT (MAP) SPDU" :)
>  (p772-transfer-success.pcap)

I like this, but I think we may be better using col_set_fence() to achieve
this consistently.
(See attached patch - let me know what you think.)

> One disadvantage is that the last RTSE fragment always is 0 bytes (no
> data).  Any idea how (and if) this can be fixed?

I think we can do this by comparing the RTSE fragment size to the negotiated
checkpoint size (though there may be a better way?).
If a fragment is exactly the checkpoint size (e.g. 3072 in your example
capture) we can assume there
are more fragments. If it is less than this, we know we have the last
fragment. This way we can 
reassemble on the last RTSE fragment and not have to wait for the session
major checkpoint (and add the 0 byte fragment to force the reassembly). 
This approach both tidies up the 0 bytes appearing, keeps the dissection
consistent when the RTSE operation is smaller than the checkpoint size and
gets rid of some of the magic.

Attached is a proposed alternative patch that adopts this approach for you
to review.
Note it is simpler as it does not need any changes in the session and
presentation dissectors. 
It works with the example capture you sent, however I may have missed
something you need. 

What do you think?

Graeme

Attachment: rtse-reass.patch.gz
Description: GNU Zip compressed data

Attachment: s4406-fence.patch.gz
Description: GNU Zip compressed data