Ronnie Sahlberg wrote:
>
> The reason the tap system works the way it does with the queue and push
> functions are because I
> wanted the taps only to be called when the dissectors were called when the
> packets were first read.
> This was a design mistake.
As guy points out, that functionality is still useful... certain taps
rely on a complete edt/proto_tree structure... but the ones that are
most like the originally envisioned functionally would benefit from
getting called immediately.
Also, I believe one of the concepts with the tap listener system was
to allow tap listeners calls to occur in a lower priority thread than
physical packet capture... Some care might still be needed if this is
still a design goal.
> Since there are valid reasons to have tap listeners to be called whenever a
> dissector is called and not
> only when the pacekt is read the first time, I think the tap system should
> be changed slightly.
>
> Instead of waiting until all dissectors have returned until the tap
> listeners are called
> I think the tap listeners should be called immediately from the dissectors.
> That would remove the need to do the rotating struct trick in dissect_tcp().
> This would also get rid of the list handling in tap.c since the list would
> become obsolete.
Iostat (and friends) are almost a different breed of tap listener. They
don't *really* rely on a specific protocol... they listen to frame just
to guarantee that they get called... but called *after* full
dissection. In fact, I think that they also require that a non-NULL
tree is passed into protocol dissectors. The other taps that rely on
tap_specific_data don't need this (although some future ones might)
Would it be wise to make a different registration call for taps such
as iostat?
Also, I think that tap listeners might benefit from registering
whether or not they need proto_tree to be used... ie. taps that find
extra fields or taps that write to the proto_tree directly.
> It would also allow tap listeners to be called everytime a dissector is
> called. Thus allowing all of
> TCP seq/ack analysis to become a tap listener. Cleaning up packet-tcp.c a
> bit.
Overall, I like that approach and it will benefit most taps. It would
be interesting to get the sequence analysis out of packet-tcp for a
couple of reasons...
1) cleaning up packet-tcp as you said.
2) creating a tap for sequence analysis that might eventually get
generalized for more protocols than just tcp
3) the first tap to write to proto_tree...
3a) Listeners that make one-line packet summaries might benefit from
incorporating flexibility as to where the *user* wants to see
the information... in the proto_tree, in COL_INFO, or printed as
a summary later on.
3b) Adding items to proto_tree can become filterable, even tappable.
Behavior like protocols might stir up even more tap discussion.
> See the current flaw as a design bug.
>
> I think the tap system should be changed so that
> register_tap_listener()
> takes an extra parameter that describes when the _packet() callback should
> be called.
>
> TAP_READ_FIRST would mean the current behaviour
"current behaviour" or the proposed modified form of calling immediately
following a protocol dissector? There still some logic
needed for figuring out which form will be done. Immediately
following a protocol dissector, unfortunately doesn't work for all
taps.
> TAP_READ_NOT_FIRST would mean every time the dissector is invoked EXCEPT the
> one above.
When a listener can potentially modify how a display filter works, it
might be better to have something such as PROTO_TREE_VIEWED...
implying when a filter using that tap's output occurs, or when a
packet is clicked on for viewing, or when tethereal dissects + displays
a packet the first time it is read.
> TAP_READ_ALWAYS would mean that the callback would be called every single
> time that dissector
> is invoked.
>
> I will do this change as soon as the next release is out. I dont want to
> change it soo close to the new release.
Guy Harris wrote:
> Passing the epan_dissect_t might not be useful except for tap listeners
> not tapping off of a dissector, as the protocol tree wouldn't be
> complete; those listeners might be called after the entire dissection is
> complete.
In fact, if a tap listener is called after incomplete dissection and
performs a search by using epan_dissect_t, couldn't that make later
searches for the same item have incorrect results? I believe that edt
caches the results of searches and only performs new searches...
