Wireshark-dev: Re: [Wireshark-dev] Tapping Behaviour [Was: Export PDU:s]
From: Pascal Quantin <[email protected]>
Date: Sun, 12 May 2013 18:39:47 +0200
2013/5/12 Evan Huus <[email protected]>
On Sun, May 12, 2013 at 8:58 AM, Anders Broman <[email protected]> wrote:
> Pascal Quantin skrev 2013-05-12 11:08:
>
> 2013/5/12 Anders Broman <[email protected]>
>>
>> Pascal Quantin skrev 2013-05-10 15:20:
>>
>> 2013/5/5 Anders Broman <[email protected]>
>>>
>>> Hi,
>>> I have added a basic implementation making it possible to export higher
>>> level PDU:s to file using a USER_DLT.
>>> The basic implementation makes it possible to export SIP traffic to a new
>>> file adding some meta data before the actual SIP message. The idea is that
>>> it should be possible to export the reassembled PDU:s(and mix several
>>> protocols) removing the under laying transport protocol but retaining some
>>> interesting data such as IP addresses and ports.
>>>
>>> The implementation is bare bones to get the demo to work. It would be
>>> nice to get some feedback on useful tags
>>> to add, helper functions to load tags and if some one is willing to work
>>> on the GUI part that'd be nice too.
>>>
>>> Would it be feasible/useful to apply for a link-layer type from tcpdump?
>>>
>>> Any comments welcome.
>>> Regards
>>> Anders
>>
>>
>> Hi Anders,
>>
>> it looks interesting. I started playing a bit with it and fixed a few bugs
>> in r49232. Moreover I added the tags content to a subtree. Feel free to
>> revert it if you do not like the output.
>> I would find it great to have a link layer type allocated. This way the
>> feature could work out of the box without any configuration.
>>
>> Yes
>
>  Then how should we proceed? Can Guy allocate us a DLT or must we send a
> request to someone?
>
> I was hoping Guy would comment :-)
>
>> Any idea on how to handle the export of several protocols ? Should we
>> allow the user to select them in the GUI or should we export all the
>> protocols registering the tap and let the user select afterwards which ones
>> to keep with filters?
>>
>> My idea is to export all the protocols registering to the tap, if you tap
>> with a filter only the filtered
>> protocols should be tapped I think.
>
> Fine with me.
>>
>>
>> By the way, I noticed that if a dissector and sub dissector both support
>> the export functionality, the sub dissector message is dumped twice (once
>> per protocol). Not sure whether this should be considered as a feature or a
>> bug.
>>
>> Do you mean protocol x+y in the first packet and y in the second? I would
>> expect that.
>
> Yes that's what I meant. I think we should tap the packet at the beginning
> of the dissection and not at the end as it is currently done in the SIP
> dissector. I see two advantages:
> - packet x+y will be tapped before packet y and not the opposite (I find it
> awkward to have packet y displayed before packet x+y)
> - a malformed packet will still be tapped
>
> Well as long as the tap, "taps" after reassembly is done I suppose it does
> not matter but if y is on top of x why tap y
> and not only X as that would include Y any way?

Tangent on the tapping behaviour: this is bringing to mind bug 8321
and the regression it cause which was bug 8610. The tap was originally
after dissection which was causing statistics to be wrong if a
malformed packet threw an exception. We moved the tap before
dissection, at which point the taps were missing some information that
they needed which hadn't been dissected yet. We eventually just
wrapped the entire thing in a Try/Catch block and called the tap at
the end again, but that was a fairly ugly solution.

I think what we need is to have taps simply get queued when called by
a dissector, and then have them actually called at the bottom of
packet-frame (or somewhere like that) so that they're always run
regardless of exceptions and with the entirety of what we were able to
dissect. I'm not sure how much work this would be, but I'm hoping it
would actually be a fairly minimal change.

Thoughts?

Evan

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.
If I got you correctly, you are proposing to queue a tap at the beginning of the dissection. Would your temporary area allow to update the tap content before calling it at the bottom of packet-frame.c? I guess this would be necessary depending on the usage of the private structure: the pointer might not be available at the beginning of the dissection.

Regards,
Pascal.