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

[ronnie sahlberg <ronniesahlberg@xxxxxxxxx>]

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
> >
> 
>