Wireshark · Wireshark-dev: [Wireshark-dev] packet reassembly

[Martin Kaiser <lists@xxxxxxxxx>]

Dear all,

I'm trying to implement reassembly of DVB-CI link layer packages.
Finally, I got to a point where the code is working but I'm not sure if
I understand it well enough. Therefore, I'd like your comments before I
submit a patch.

The DVB-CI link layer (lpdu) is fairly simple:

   1 byte "transport connection id"
   1 byte more/last indicator
   <fragment of a transport layer packet>

There's no such thing as a sequence number, the lpdus should arrive in
order (DVB-CI devices aren't able to re-order).

Please see the code below for a rough outline.
I tried to avoid saving any state in static variables.

Is it ok to call fragment_add_seq_next() and process_reassembled_data()
even if the packet is not fragmented? In my tests, I seem to receive the
complete payload_tvb for a non-fragmented packet.

Is it ok to use a constant sequence number for all packets? It seems ok
if the packets arrive in order (to avoid it, I would probably need
static variables and increment the sequence number at the beginning of
each fragmented transport packet - which can't be detected reliably).

Is it necessary to set pinfo->fragmented and save/restore its original
value? Where is the temporary setting used?

Thanks in advance for your feedback.


   Martin


------------------------
static void
dissect_dvbci_lpdu(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
        guint8 direction)
{
/* ... */
    tvbuff_t *payload_tvb = NULL;
    fragment_data *frag_msg = NULL;
    gboolean save_fragmented;


/* ... create link_tree ... */

/* ... get all data and do some checks ... */

    save_fragmented = pinfo->fragmented;
    frag_msg = fragment_add_seq_next(tvb, 2, pinfo,
            4, /* sequence id, TODO: can this be a constant value if
                  packets must arrive in order? */
            msg_fragment_table,
            msg_reassembled_table,
            tvb_reported_length_remaining(tvb, 2),
            more_last == ML_MORE ? 1 : 0);

    payload_tvb = process_reassembled_data(tvb, 2, pinfo,
            "Reassembled Message", frag_msg, &msg_frag_items,
            NULL, link_tree);
    if (payload_tvb) {
        pinfo->fragmented = TRUE;
    }
    else {
        pinfo->fragmented = FALSE;
        if (more_last == ML_MORE)
            col_append_fstr(pinfo->cinfo, COL_INFO, " (Message fragment)");
        else {
            payload_tvb = tvb_new_subset(tvb, 2, -1, -1);
       }
    }
    if (payload_tvb)
        dissect_dvbci_tpdu(payload_tvb, pinfo, tree, direction, tcid);
    pinfo->fragmented = save_fragmented;