Wireshark-dev: Re: [Wireshark-dev] [PATCH] Adding RTSE reassembly
From: "Graeme Lunt" <[email protected]>
Date: Sat, 23 Jun 2007 17:22:33 +0200
Stig,

> > I like this, but I think we may be better using col_set_fence() to  
> > achieve
> > this consistently.
> 
> This will be a bit strange when the MAJOR SYNC POINT is in its own  
> frame. I suppose we get  Protocol "S4406,"  and  Info "Military ... | ".

Yes. Just a space separator would be better. 8^)

> >> 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?).
> 
> This will not work for messages where the last fragment has  
> "checkpoint size" bytes, which happens in about 1 of 3072 packages  
> when using 3k size. Of course, it will work in most cases :)
> 
> I also have some captures from Microsoft Exchange 2003 (from a  
> customer) which does not follow the 1k limits (the RTSE data segment  
> is 3051, not 3072 as it should be).  This messages will not be  
> reassembled correctly with your approach.

Well that scuppers my approach then. Think I need to go and read RTSE in
more depth this time.
I just didn't think there needed to be all that support in SES and PRES to
support RTSE.

> In your patch, if you have more than one RTSE fragment in one frame  
> (I have seen this in MS captures) you will have two (or more)  
> reassembled messages which is a bit strange.

That is something that would have had to have been fixed if the approach was
viable!
 
> Attached a patch to fragment.c with fragment_end_seq_next() to end  
> the fragmented data without adding an empty data fragment.  
> This will  
> still show a reassembled entry when receiving a message smaller than  
> the checkpoint size, but without the 0 bytes fragment.
>
> Also attached a new patch for SES, PRES and RTSE which uses  
> dissect_ber_octet_string() and fragment_end_seq_next().

Thanks - I'll have a look.

Graeme