Ethereal-dev: Re: [Ethereal-dev] [Patch] tethereal -z tcp,close

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

From: "Ronnie Sahlberg" <ronnie_sahlberg@xxxxxxxxxxxxxx>
Date: Wed, 27 Nov 2002 16:20:34 +1100
----- Original Message -----
From: "Jason House"
Sent: Wednesday, November 27, 2002 10:54 AM
Subject: Re: [Ethereal-dev] [Patch] tethereal -z tcp,close


>
>
> Guy Harris wrote:
> >
> > On Tue, Nov 26, 2002 at 05:44:05PM -0500, Jason House wrote:
> > > There is another reason why it might not have made its way into the
> > > CVS... It did not use the normal mechanism for a tap to function...
> > > The idea is that a protocol should share the tap-specific data rather
> > > than have the data extracted from the protocol tree (either manually
> > > or via the filtering mechanism)
> >
> > I wouldn't describe the shared data as "the normal mechanism".  There's
> > a tradeoff - if you share tap-specific data, the tap stuff will probably
> > run faster, as it has to do less work to extract the data; *however*,
> > you can only do such a tap if the dissector you're tapping into is
> > providing the information the tap wants.  If you extract the data from
> > the protocol tree, it might involve more work, but you can construct
> > taps that do things the protocol dissector writer didn't anticipate, and
> > don't have to hack up any protocol dissectors to do it.

I think that different tap points should not be filled with "random" fields
just
to accomodate specific tap listeners.
But it would make sense to put some "common" fields in there.

I think, for example for a "tcp" tap, it may be appropriate to have this
return a pointer to a structure holding the tcp header (minus options fields
maybe).
This may also require the tcp dissector to be recoded slightly to actually
use
this tcp_header_struct instead of accessing the fields directly using
tvb_get_xxx or
by using separate variables holding it.

Similar for an "ip" tap, I think it would be natural for such a tap to
return a pointer to
an ip_header_struct (minus options?).

I think it would be inappropriate to extend this ip_header or tcp_header
struct with
other random fields only to accomodate a specific tap listeners though and
tap listeners that would
any other information outside of what is in the header struct would have to
do it itself maybe by walking the
edt->tree.

>
> I guess I need to choose my words a bit more carefully.  Filling a
> data structure does increase performance and is generally preferred
> when possible...  Things such as tap-rpcstat and tap-tcp_close can
> get significant performance improvement from using that mechanism.
> Listeners that use random fields from random protocols must rely on
> the epan_dissect_t instead, but pay the price of speed.
>
> I still wonder if a "if (tap_listener)" analogous to "if (proto_tree)"
> will ever pop up in dissector code.  So far, I think the overhead of
> tap listener specific information is minimal.  Dissection might even
> benefit from lines like "if (tap_listener || proto_tree)".  One
> example would be OSPF which might not want to go through dissection of
> each and every LSA to provide information for a tap listener that
> isn't even being used.

I think that there should not be any changes at all to the dissectors.
The dissectors should not be aware of any tap listeners.
(appart from providing a generic tap point)

> I will be interested to see what Ronnie comes up with in terms of some
> kind of intermediate mechanism for allowing fast access to certain
> common fields such as frame.len as well as slower access to other,
> more generic, fields/tap outputs.

frame.len is not useful as an example since that data can be exstracted from
the pinfo structure
but I see what you mean.
My view is that if a tap in a protocol dissector is to provide a pointer to
a structure, i.e.
tap specific data, that structure should only contain generic data specific
to that protocol,
something like the protocol header structure.
I do not think that that structure should contain any specific data only to
"help" a specific tap listener.
For now, tap listeners that would need other, random, data would have to
provide this themself
similar to how -z proto,colinfo,... does.


As for the TCP sequence analysis example earlier in the thread:
Currently this feature can not be implemented purely as taps since the taps
are only called the first time
a packet is dissected.
An alternative tap mechanism would have to be implemented wich would call
tap listener callbacks
every time except the first time a packet is dissected.

The TCP sequence analysis example is a good example why this would be
useful.
Dont hold your breath, I can not start designing or test this in the near
future since I am currently
fighting with gtk to get io,stat more generic and also a traffic-map.


ronnie sahlberg