Ethereal-dev: Re: [ethereal-dev] Why are we doing a conversation_init in rescan_packets in fil

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

From: Guy Harris <gharris@xxxxxxxxxxxx>
Date: Mon, 21 Aug 2000 23:42:14 -0700
On Tue, Aug 22, 2000 at 02:21:29PM +0900, Richard Sharpe wrote:
> Why are we doing a conversation_init in rescan_packets?

Because I added a call to "conversation_init()" in "colorize_packets()"
in revision 1.120 of "file.c", and that was carried along.

The comment for the checkin is:

	Provide a general mechanism by which dissectors can register
	"init" routines, which are called before a dissection pass is
	made over all the packets in a capture - the "init" routine
	would clear out any state information that needs to be
	initialized before such a dissection pass.

	Make the NCP, SMB, AFS, and ONC RPC dissectors register their
	"init" routines with that mechanism, have the code that reads in
	a capture file call the routine that calls all registered "init"
	routines rather than calling a wired-in set of "init" routines,
	and also have the code that runs a filtering or colorizing pass
	over all the packets call that routine, as a filtering or
	colorizing pass is a dissection pass.

	Have the ONC RPC "init" routine zero out the table of RPC calls,
	so that it completely erases any state from the previous
	dissection pass (so that, for example, if you run a filtering
	pass, it doesn't mark any non-duplicate packets as duplicates
	because it remembers them from the previous pass).

The third paragraph suggests that there is currently one reason why we
*do* have to call "protocol_init()" in "rescan_packets()".

In this particular case, the dissector is, in effect, assuming that it
will be called only once for a given packet, and that packets will be
handed to it in the order in which they appear in the capture.

That's obviously not the case in Ethereal, even if you *don't* rescan
the packets - after the first pass through the packets when reading the
file in, packets can be dissected in an arbitrary order, as the user can
just start scrolling and clicking the packet list at random.

In that particular case, the only difference between the way the RPC
dissector handles duplicate and non-duplicate packets is that, for
duplicate packets:

	1) it sticks into the Info column a "dup XID XXXX" indication;

	2) it adds some additional hidden fields to the protocol tree.

The former only shows up when constructing the packet list for display
(although, with my under-development changes to have the column text
constructed only when the row is actually to be displayed, that won't be
the case), and the latter only shows up if you filter on the hidden
fields in question, so re-initializing the dissector on a rescan caused
the problem not to show any very visible symptoms.

However, there could well be problems of that sort that can't be hidden
with a hack such as that.

This is the problem for which I infer you created the mechanism for
hanging per-protocol data off of the frame_data structure for a packet -
remember the stuff that the dissector figured out on the initial
sequential pass, so that it can retrieve that information when the
packet is next dissected, without having to reconstruct that
information, as correctly figuring that information out may be possible
only during a sequential pass.

There *might* be cases where dissectors depend on discovering
conversations anew, which might cause the same problem.

I suspect the correct fix is to ensure that dissectors only compute that
sort of non-local information if the "visited" flag in the "frame_data"
structure is 0, and that if they need that information to dissect the
packet, they attach that information to the packet if "visited" is 0,
and attempt to fetch it if "visited" is non-0.

Once that's done, "protocol_init()" and "conversation_init()" need only
be called when a new file is loaded.