Wireshark-dev: Re: [Wireshark-dev] causes for losing COL_PROTOCOL or COL_INFO data

From: Jaap Keuter <jaap.keuter@xxxxxxxxx>
Date: Mon, 18 Sep 2017 22:25:09 +0200
Hi John,

Rule of thumb: don’t use ‘if (tree)...’ constructs.
They have little if any use, don’t really save processing time (all wireshark functions are capable of handling tree==NULL),  and cause more trouble than it’s worth (as you’re in right now).

Some of the finer details are that Wireshark runs the dissection engine, and therefore your dissector, several times for various purposes, once sequentially, then at random.
This is either with or without the tree parameter set. Dissectors must function so that they dissect the buffer the same either with or without tree parameter.
Data may be preserved of several stages, and that is why dissection needs to be done in all situations.

Hope it helps reworking your code.
Jaap



> On 18 Sep 2017, at 17:52, John Dill <John.Dill@xxxxxxxxxxxxxxxxx> wrote:
> 
>> Message: 1
>> Date: Sat, 16 Sep 2017 13:38:31 +0100
>> From: Peter Wu <peter@xxxxxxxxxxxxx>
>> To: John Dill <John.Dill@xxxxxxxxxxxxxxxxx>,
>>       "wireshark-dev@xxxxxxxxxxxxx" <wireshark-dev@xxxxxxxxxxxxx>
>> Subject: Re: [Wireshark-dev] causes for losing COL_PROTOCOL or
>>       COL_INFO data
>> Message-ID: <288553DC-6272-4581-A5E5-15B933BE7178@xxxxxxxxxxxxx>
>> Content-Type: text/plain; charset=utf-8
>> 
>> Hi John,
>> 
>> Are your col_* functions guarded/affected by a check like if(tree) or do they depend on pinfo->fd.visited? Are the affected frames >triggering reassembly or exceptions?
> 
> Part of it is.  I have a udp heuristic dissector that has a section that's kind of like this.  I have to be careful with actual content, so here is pseudo-code skeletal structure.
> 
> gboolean
> dissect_protocol_udp_heur(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data)
> {
>  ...
> 
>  data_class = tvb_get_guint8(tvb, offset);
> 
>  if (data_class in list of recognized values)
>  {
>    col_add_str(pinfo->cinfo, COL_INFO, val_to_str(data_class, data_class_enum_vals, "Unknown"); /*this always works*/
>    ...
> 
>    if (tree)
>    {
>      if (data_class == XXX) {
>        dissect_xxx(tvb, pinfo, offset, tree);
>      }
>      if (data_class == YYY) {
>        ...
>      }
>      ...
>    }
>    ...
>  }
> }
> 
> And the COL_INFO that gets lost is in the dissect_xxx function, which has several col_append functions used.  It gets lost after using a filter expression and I never get it back after that.
> 
> I've noticed this 'if (tree)' issue in a couple of the searches I've done attributed to this problem, but I don't quite understand the impact it has on dissection and populating the COL_INFO and friends column.  Any explanation or pointer to how this interaction works would be greatly appreciated.
> 
> Best regards,
> John Dill
> 
> P.S. Thanks to Michael for the follow up too.
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives:    https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>             mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe