[Robin <wireshark@xxxxxxxxxx>]

Hey everyone,
I'm currently working on the SCTP fragmented message reassembly and so I 
require a few changes to the functions in reassemble.c to avoid multiple 
functions doing the same thing with one line of code difference. SCTP 
fragments are sequence number based so the changes only affect the 
add_fragment_seq* and their following functions.

In the following I'll brief on my intended changes so that you can 
notice me if anything would affect any other protocol.


A new function

----------------------

fragment_data *
fragment_add_seq_bsn(tvbuff_t *tvb, int offset, packet_info *pinfo, 
guint32 id, GHashTable *fragment_table, GHashTable *reassembled_table, 
guint32 frag_number, guint32 frag_data_len, gboolean more_frags, guint 
start_bsn)

----------------------

with the new parameter start_bsn to set the first bsn arbitrarily is 
added. This value is passed to fragment_add_seq_check_work which also 
got the new parameter. Every other function calling 
fragment_add_seq_check_work uses 0 for the new parameter to act like 
before. When fragment_add_seq_check_work calls fragment_add_seq_work it 
passes the parameter again. In fragment_add_seq_work there is a check if 
all fragments have been collected:

----------------------

/* check if we have received the entire fragment
* this is easy since the list is sorted and the head is faked.
*/
max = start_bsn;
for(fd_i=fd_head->next;fd_i;fd_i=fd_i->next) {
	if ( fd_i->offset==max ){
		max++;
	}
}
/* max will now be datalen+1 if all fragments have been seen */

----------------------

The variable max is set to start_bsn instead of 0 as before. This should 
be all what it needs.

The next change is a bit more critical. Also in fragment_add_seq_work, 
the fd_head->datalen value will be set different in this part:

----------------------

if (!more_frags) {
	/*
	* This is the tail fragment in the sequence.
	*/
	if (fd_head->datalen) {
		/* ok we have already seen other tails for this packet
		* it might be a duplicate.
		*/
		if (fd_head->datalen != fd->offset ){
			/* Oops, this tail indicates a different packet
			* len than the previous ones. Somethings wrong
			*/
			fd->flags      |= FD_MULTIPLETAILS;
			fd_head->flags |= FD_MULTIPLETAILS;
		}
	} else {
		/* this was the first tail fragment, now we know the
		* sequence number of that fragment (which is NOT
		* the length of the packet!)
		*/
		max = fd->offset;

		/* is any higher offset already there?
		* this is important for SCTP to reassemble
		* packets not arriving in order
		*/
		for (fd_i=fd_head->next;fd_i;fd_i=fd_i->next) {
			if (fd_i->offset > max) max = fd_i->offset;
		}
			
		fd_head->datalen = max;
	}
}

----------------------

Formerly fd_head->datalen was just set to fd->offset, assuming the last 
fragment appearing is the last in the sequence. SCTP fragments are not 
inevitably in order, so if the last part of the message does not arrive 
as the last part of the sequence the highest offset is already there. 
fd_head->datalen has to be set to the highest available value to pass 
the test if all fragments are already there, that is the code I posted 
in the first part of this mail.

There are lots of tests if the new fragment is correct, in order, the 
last, whatever and in every case it is assumed that the last fragment 
has to carry the highest sequence number unless the FD_DEFRAGMENTED bit 
has been set and overlapping maybe found. My additions only set the 
datalen value to the last offset if it's really the highest, but 
regarding to the assumptions made elsewhere in other protocols that's 
always the case.

Please let me know if any of my changes would affect any other protocol 
so I can use another parameter or anything else to keep the behavior of 
other reassemblies.

Regards,
Robin