[John Thacker <johnthacker@xxxxxxxxx>]

I am implementing fragment reassembly of PPP Multilink (RFC 1990) and also implementing multiclass extension (RFC 2686). (https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12548 https://code.wireshark.org/review/#/c/16044/)
The protocol is unlike other protocols that do reassembly. Most protocols contain a sequence number that corresponds to the reassembled PDU, plus generally fragment numbers that indicate the position of the fragment within the reassembly and a flag that indicates when there are no more fragments expected. (There are some exceptions without fragment numbers; those have to assume that fragments are received in order and use fragment_add_seq_next().)

PPP Multilink only has a single sequence number, plus flags that indicate whether something is the first or last (or both or neither) fragment of a PDU. There's no indicate of what the fragment number is within the sequence, other than "First" or "not first." The ordinary fragment_add_seq* functions would prefer two values, a sequence number, and a fragment number; effectively we have the sum of these two. Thus an in order sequence of packets might look like:

0, First, Last (0, 0)

1, First (1, 0)

2, Last, (1, 1)

3, First (3, 0)
4, (3, 1)

5, Last (3, 2)

The value in parentheses is how these would naturally be treated under the standard (seqnum, fragnum) schema.

If we receive packet 4 or 5 before packet 2 or 3, then it is impossible to tell if packet 4 or 5 are fragments 3 or 4 of a packet beginning at 1, or fragments 1 and 2 of the packet beginning at packet 3. (If we haven't received packet 1 as well, then there are even more possibilities.) The approach I use is to tentatively add them to both, then clean things up as necessary. (The approach could be improved somewhat; I already decrement the sequence number and increment the fragment number and stop if we finish a reassembly. I could also stop as soon as we hit a partially reassembly for which a recemt First fragment (fragnum 0) has been seen.)

Cleaning them up involves two things: using fragment_delete either immediately or later, when that sequence number has cycled around, in order to remove entries in the in-progress reassembly table that are completely wrong. Another necessary method to clean things up is removing extra fragments added to the end. In the example above, if we receive packet 4 and 5 before 2 and 3, but then receive packet 2, then since packet 2 is marked as the Last packet in a reassembly, we remove any tentative packets after 2 (such as 4 and 5) from tentative in-progress reassemblies. That involves some linked list manipulation and use of tvb_free and g_slice_free. That's pretty hairy; at the same time, so is the use of fragment_delete as, even though it's added to reassemble.h, no other dissector currently uses it.

The sequence number itself can be short (12 bit) or long (24 bit), either of which can loop around in a single capture. Because of this, if a sequence number is encountered and there is a packet currently being reassembled whose most recent packet is quite old (set by a configuration value), the old reassembly is discarded with fragment_delete. This use of fragment_delete is unneeded if all packets are present in the capture and reassembly is successful (as that removes it from the in-progress reassembly table), but that may not always be the case.

This all seems to work and I've tested it a lot, but I'm somewhat unsure about it. It's not like any other fragmented protocol (though the aging stuff could be helpful), so I had to use the defragmentation API quite differently. Perhaps these things should be added into reassemble.h and reassemble.c, since they're pretty low level and it's possible that other protocols in the future would need them. The other API calls tend to assume that things are simply in error if, as a result of calls, multiple tails end up getting added, or a tail gets added that is before other data, whereas in this case that's an expected outcome but one that needs to be cleaned up.

A more thorough rewrite of how reassembly works could introduce new API calls that are designed to work for sequence numbers as implemented here, "front end" sequence numbers that increment with each fragment instead of end result PDU sequence numbers. I haven't fully thought of how to do it.

Any thoughts or suggestions?

John Thacker