Wireshark · Ethereal-dev: [Ethereal-dev] Re: Order of tap listeners (PATCH)

["Lars Ruoff" <lars.ruoff@xxxxxxx>]

Hi Ronnie,
thanks for the proposal, but consider this:

My problem is not with the order of how 2 *different* tap listeners (tap_A 
and tap_B) are called.
But my problem is with the *same* tap listener (tap_A) getting called twice 
for a frame which carries 2 times a packet of a certain kind (i.e. 
packet_A)!
In the actual implementation, the tap is called twice, one time for each 
packet, but in *reverse* order of which the packets were dissected!
This is problematic for taps that manage state info between successive 
packets of same kind (example: rtp_analysis).
My patch fixes this by assuring at least the right order (=order of 
dissection) in this case.

> Is there actually tap listener features you are planning that would
> need a defined order of calling of tap listeners or is it just a
> generic request for 'it-might-be-useful'.
I'm working on an experimental SRP (Spectralink)-dissector that carries 
multiple occurences of RTP packets in a single frame, and i would like to be 
able to use the rtp_analysis tap on these SRP-"embedded" RTP packets.
Currently (without the proposed patch), rtp_analysis gets stuck because it 
sees the packets in wrong order!

But i think this is would be usefull for *any* taps that are likely to get 
called for multiple packets in the same frame.

regards,
Lars Ruoff

> Message: 23
> Date: Mon, 28 Mar 2005 07:28:43 -0400
> From: ronnie sahlberg <ronniesahlberg@xxxxxxxxx>
> Subject: [Ethereal-dev] Re: Order of tap listeners (PATCH)
> To: Ethereal development <ethereal-dev@xxxxxxxxxxxx>
> Message-ID: <c9a3e454050328032842c68a76@xxxxxxxxxxxxxx>
> Content-Type: text/plain; charset=ISO-8859-1
>
> I dont think that would work.
>
> There is no guarantee that a dissector calls the tap framework after
> it has called its subdissectors instead of before calling them.
>
> I think instead you should go for a different approach.
>
> First,  whan an application is registering a tap listener, this is
> likely a very infrequent call so performance would be non-critical
> (happens when user selects something from the menu).
> But when the framework needs to call every listener for queued data
> this would a frequent function (happens after potentially every
> packet)
>
> So we have to make sure the second case is fast while it doesnt matter
> much if the first case is fast or not.
>
>
> proposal :
>
> Code a patch that adds a priority field to the tap listener list.
> Add a comment that when new entries are inserted into the list it is
> important that the ordered-by-priority property of the list is
> maintained  or else undefined behaviour is invoked. (and ethereal
> execv() nethack)
>
> Add a priority parameter as such to the register_tap_listener parameters.
> Change all existing tap listnerers to pass 0 as the priority parameter.
>
> Inside the tap framework, keep using the existing list but instead of
> always adding new tap listeners to the head of the list, change this
> function to add the tap listener to be the first one in the list with
> the given priotiry or higher.
> I.e. change register_tap_listener to always insert new tap listeners
> so that the list is always sorted by priority.
>
> This will make register_tap_listener() slightly slower than before
> since it might have to walk a few entries in the list before it can
> add the entry.
> Since most tap listeners will register with priority==0 this is a
> non-issue since these entries would be at the head of the list anyway
> and no walking the list is required.
>
>
> This also means that tap_push_tapped_queue()
> will still be a simple walk a simple linked list once and thus still
> be fast (which is good since this function is time critical).
>
>
> Then document something like :
> The tap listeners will be called by
> priority in incrementing order. First all priority==0 listeners will
> be called, second all priority 1 listeners etc etc.
> Please note that the order is only defined for listeners of different
> priority. Within a priority level the order in which listeners are
> called is undefined.
>
>
> This would work and still be fast.
> It would be simple.
> The change to the code would be minimal.
> I.e.  three good things in one patch:-)
> This would also allow the possibility of specifying explicit call
> order for tap listeners if so required.
>
>
>
> Is there actually tap listener features you are planning that would
> need a defined order of calling of tap listeners or is it just a
> generic request for 'it-might-be-useful'.
>
>
>
>
> On Sat, 19 Mar 2005 22:14:36 +0100, Lars Ruoff <lars.ruoff@xxxxxxx> wrote:
> >
> > If no one objects, could we apply the following patch to tap.c, which
> > brings
> > the tapping in order with the packet dissection?
> >
> > tap.c:
> > - Replaced fixed length linked list of tap_packet_t with static array.
> > We advance in the array with every call to tap_queue_packet.
> > In tap_push_tapped_queue we iterate over all queued packets in the same
> > order.
> > => The order of which the tap listeners will be called is the same as 
> > the
> > order of which  the corresponding dissectors for the packet have been
> > called. (README.tapping to be updated).
> >
> > Attached is relevant output of 'svn diff'.
> >
> > regards,
> > Lars Ruoff
> >
> >
> > > README.tapping says:
> > > "After each individual packet has been completely dissected and all
> > > dissectors have returned, all the tap listeners that has been flagged
> > > to receive tap data during the dissection of the frame will be called 
> > > in
> > > sequence.
> > > The order of which the tap listeners will be called is not defined."
> > >
> > > Why can't we just define the order as the same order of the sequential
> > calls
> > > to tap_queue_packet, at least for a given tap listener?
> > > That would help a lot with a statefull tap listeners that inspect
> > multiple
> > > packets beeing transported in the same frame.
> > > Currently, i'm trying to tap on RTP packets over a Wifi-Connection 
> > > where
> > > multiple RTP packets are encpasulated in a radio transport protocol. 
> > > But
> > the
> > > RTP tap listener sees the order mangled up and wrongly detects 
> > > sequence
> > > errors.
> > >
> > > regards,
> > > Lars Ruoff
> > >
> >
> >