Wireshark · Wireshark-dev: Re: [Wireshark-dev] Tapping Behaviour [Was: Export PDU:s]

[Pascal Quantin <pascal.quantin@xxxxxxxxx>]

2013/5/12 Evan Huus <eapache@xxxxxxxxx>

On Sun, May 12, 2013 at 12:34 PM, Anders Broman <a.broman@xxxxxxxxxxxx> wrote:
> If the tap is extracting information from the packet and a dissector bug
> occurs before that
> information is extracted that information will always be missing from the
> tap so having
> "correct" information from a tap is impossible if there are dissector bugs -
> right?

Correct, but the tap should still get what information there is for
that packet so that it can calculate statistics for the protocol (this
was bug 8321 - the tap wasn't even getting called for malformed
packets).

On Sun, May 12, 2013 at 12:39 PM, Pascal Quantin
<pascal.quantin@xxxxxxxxx> wrote:
> Hi Evan,
>
> I'm not sure I understood your proposal properly.
> The issue with bug 8321 was that the tap was never called (due to the
> exception) and the issue with bug 8610 was that context information was
> missing when the tap was called.
> When looking at the comments in tap.h, it appears that tap_queue_packet() is
> designed to be called once that dissection of the current packet is finished
> (so that packet_info and the private data structures are completely filled).
> As we saw with bug 8321, it does not work if an exception occurs in the
> middle of the dissection.

So this is very interesting - I took a closer look at the tap
internals and it's already doing what I was suggesting, but that means
I no longer understand bug 8610 at all.

Here's my understanding of what happens: a dissector calls
tap_queue_packet with pointers to pinfo and some private data
structure. All this does is add the packet (and pointers) to a list -
the tap callback isn't actually called yet. Once the entire packet has
been dissected (and any appropriate exceptions caught),
tap_push_tapped_queue is called and takes care of actually calling all
the callbacks.

Bug 8321 still makes sense - an exception was being thrown before the
packet was ever queued with tap_queue_packet. However, I'm no longer
following bug 8610. The packet queue only has pointers to the pinfo
and other structure, so it shouldn't matter when the tap gets queued -
more dissection of the same packet should only update the structures
being pointed to, so the taps should automatically see all the
necessary data.

I don't understand why moving the tap_queue_packet earlier caused data
to be missing, since that data should have been available anyways by
my reading. The pointers passed to the tap callback didn't change, and
the actual point when the callback was called didn't change.

What am I missing?

I had another look at the code and the DTMF info that was missing in bug 8610 comes from the rtpevent tap and not from the rtp tap. The rtp listener needs the data from the rtpevent listener. That's why the order does matter.

Regards,
Pascal.

Thanks,

Evan