Ethereal-dev: Re: [Ethereal-dev] Tap extensions. Comments please.

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

From: "Ronnie Sahlberg" <sahlberg@xxxxxxxxxxxxxxxx>
Date: Tue, 10 Sep 2002 08:12:05 +1000
----- Original Message -----
From: "Guy Harris"
Sent: Tuesday, September 10, 2002 7:58 AM
Subject: Re: [Ethereal-dev] Tap extensions. Comments please.


> On Tue, Sep 10, 2002 at 07:48:40AM +1000, Ronnie Sahlberg wrote:
> > But it is the epan_dissect_run() that is the expensive one? right?
>
> Yes, as I said.
>
> > the one which actually dissects the packet?
>
> Yes.
>
> > It doesnt matter much if we have it in the inner most loop or not
> > since in the normal case the inner loop in reality has an ifstatement
that
> > mostly triggers only once per loop.
>
> If it doesn't matter whether the expensive routine is in the inner loop
> or not, why are you worrying about any of this?
>
> Or is the "it" in "It doesnt matter much if we have it in the inner most
> loop or not" some routine *other* than "epan_dissect_run()"?
>
> If it does matter, why not just implement what I suggested and be done
> with it?

I didnt explain it properly.
In tap.c/tap_push_tapped_queue() there are two loops.
In order to reduce the number of expensive calls to epan_dissect_run() it
would have to be moved outside BOTH loops, i.e. it is more important to move
it outside the outer-loop than just outside the inner loop (but still inside
the outer loop)

>
> > So, perhaps if one could change epan_dissect_prime_dfilter() call to say
> > "Gimme all
> > fields, Im interested in them all" then one could move it, and more
> > important epan_dissect_run() to outside the loops?
>
> I.e., it *DOES* matter whether you move "epan_dissect_run()" outside the
> loop?

YES.

>
> > Would saying something like : ALL fields are interesting : degrade
> > perfrmance?
>
> I have no idea.  I suspect it would.
>
> It would probably degrate it more than having two loops would.
>
> > Maybe but before actually measuring the impact I would guess
> > that the multiple calls to epan_dissect_run() would be even worse.
>
> Yes, it probably would.
>
> But you have more alternatives than
>
> 1) calling "epan_dissect_run()" in the loop
>
> or
>
> 2) calling "epan_dfilter_prime_dfilter()" and specifying that
>    all fields are interesting.
>
> In particular, you have the alternative that I suggested in my previous
> e-mail, namely
>
> epan_dissect_new()
> loop over all entries:
> epan_dfilter_prime_dfilter()
> end-loop
> epan_dissect_run()
> loop over all entries:
> dfilter_apply_edt()
> end-loop

Aha!   We have a winner.
So I assume it is possible to call epan_dfilter_prime_dfilter() multiple
times and then it will just aggregate the number of interesting fields ?

I will try that in tap.c to see if it would work.


>
> (in my previous e-mail, I left out the "end-loop" clauses as I thought
> it was obvious that the indentation indicated what was being done in the
> loop or not; perhaps that's the source of the confusion here).

Sorry for me misunderstanding, when leaving out the end-loop i just though
you tried to describe both loops,
the first loop being the outer and the second the inner.

>
> That has the advantage that
>
> 1) you don't say "all fields are interesting", with the
>    potential performance hit;

Yes. good point. I was not aware that one could do
epan_dfilter_prime_dfilter()
multiple times.

>
> 2) you don't have to figure *how* to say "all fields are
>    interesting", which "epan_dfilter_prime_dfilter()" doesn't
>    make easy (the list of fields it marks as interesting come
>    from a dfilter_t, so you'd have to construct a dfilter_t that
>    uses every single field, or add another routine to iterate
>    through *ALL* registered fields and mark them as interesting.

Yes.


Thanks a lot. I will try to implement the construct above and see how it
works.