On Wed, Jun 11, 2003 at 11:18:55AM -0700, Guy Harris wrote:
> On Wed, Jun 11, 2003 at 10:38:43AM -0700, Jeff Rizzo wrote:
> >
> > However, I just noticed that when I add a filter to search on sflow
> > agent address, it re-draws the traffic summary screen with
> > the dissected summary info for the last sample in the packet,
>
> This suggests that you're only calling the other dissector if the "tree"
> argument is non-null. When the file is read in, each frame is
> dissected, in order to
>
> 1) build up any state needed by the dissectors later;
>
> 2) set the column text.
>
> However, when you filter the display, the protocol tree has to be
> constructed (a display filter checks values in the protocol tree), so
> the "tree" argument is non-null; if, in that case, you call the other
> dissectors, they *do* get a chance to change the column text.
>
> Subdissectors must be called regardless of whether the "tree" argument
> is null or not; otherwise, they might not get called in the first pass
> through the file, and might not get a chance to correctly set the
> columns or construct necessary state. In this particular case, that
> might not matter, but that principle should still be followed.
Hm. I did it the way I did because the example (from README.developer)
seemed to indicate that I should. So, what you're saying is that
I should always dissect the entire packet first, since I basically
need to in order to decide which pieces to pass off to the
subdissectors? Whether or not the protocol tree is built?
I can certainly do that, but the comments in the example made me
concerned about performance.
>
> > which is *not* what I want. Is there any way to prevent this?
>
> Before you call the other dissector, mark the columns as "read-only"
> by calling
>
> col_set_writable(pinfo->cinfo, FALSE);
>
> and save the addresses in the "pinfo" structure as well. When the
> dissector returns, restore the addresses and set the writability of the
> columns appropriately:
>
Great! This is exactly what I was looking for. Thanks.
>
> The exception catching is done so that if you get an exception thrown
> (which you probably will, as, presumably, the entire sampled packet
> isn't in the sFlow packet), we don't report it as a "Short Frame" or
> "Malformed Packet", and we don't leave the addresses, etc. set by the
> subdissector.
Yep, I was already doing that.
>
> > Also, when I'm ready to submit the dissector (when I solve this
> > problem, or possibly before :), should I attach the patch file, or send
> > a pointer to where it can be retrieved?
>
> Attaching the patch file is easiest.
OK, I'll do that when the time comes, then. Thanks a bunch for
your feedback.
+j
--
Jeff Rizzo http://boogers.sf.ca.us/~riz