Ethereal-dev: Re: [Ethereal-dev] Re: Order of tap listeners (PATCH)
Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.
From: "Lars Ruoff" <Lars.Ruoff@xxxxxxxxxx>
Date: Thu, 31 Mar 2005 10:13:15 +0200
Any other comments on this? Lars ----- Original Message ----- From: "Lars Ruoff" <lars.ruoff@xxxxxxx> To: "Ethereal-Dev" <ethereal-dev@xxxxxxxxxxxx> Sent: Monday, March 28, 2005 5:26 PM Subject: [Ethereal-dev] Re: Order of tap listeners (PATCH) > > 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 > > > > > > > > > > > > > > > > _______________________________________________ > Ethereal-dev mailing list > Ethereal-dev@xxxxxxxxxxxx > http://www.ethereal.com/mailman/listinfo/ethereal-dev
- References:
- [Ethereal-dev] Re: Order of tap listeners (PATCH)
- From: Lars Ruoff
- [Ethereal-dev] Re: Order of tap listeners (PATCH)
- Prev by Date: Re: [Ethereal-dev] Bug in ethereal filtering on DF (Don't Fragment) flag?
- Next by Date: [Ethereal-dev] Updated: Tektronix2pcap-script
- Previous by thread: [Ethereal-dev] Re: Order of tap listeners (PATCH)
- Next by thread: RE: [Ethereal-dev] Re: Order of tap listeners (PATCH)
- Index(es):