Jimmy,
 
your suggestion sounds good. I implemented the changes and tested them with my ssl analyzer. Everything worked as expected. :-)
 
Attached you can find the patch for these changes (to be applied with -p0 in the bro directory, after applying all of the other binpac patches that I provided before). Could you test this further with whatever binpac analyzers you have got?
 
Anyone else any comments on this issue (Ruoming)?
 
Tobias

 
On 5/30/07, jmzhou.ml@gmail.com <jmzhou.ml@gmail.com> wrote:
On Tue, 29 May 2007, Tobias Kiesling wrote:

> On 5/29/07, jmzhou.ml@gmail.com <jmzhou.ml@gmail.com> wrote:
>
>> Patch 6: I think there is another issue here: if some buffering request
>> has been sent to the flow buffer, and the flow buffer is not ready with
>> partial data, we will end up with entering the loop - a waste of CPU
>> cycles.
>
>
> Maybe you are right, but what would you suggest as change? Do you want to
> check whether the buffer is ready before entering the loop? But then it has
> to be ensured that the buffer is properly initialized in any case.  At the
> moment I cannot see all the consequences of such a change. And do you think
> that the impact on performance is really relevant?

The cost can be a little bit expensive if there are many layers of parsing.
You end up with many unnecessary calls to parsing functions and condition
jumps. One possible approach is like this:

. add a new boolean member have_pending_request to FlowBuffer, initialized
as false.

. set have_pending_request to true in call NewFrame and NewLine.

. reset have_pending_request to false in call DiscardData.

. change the while loop condition to:
    while (flow_buffer->data_available() &&
        (!flow_buffer->have_pending_request() || flow_buffer->ready()))

Analysis:

  1. The first time, data_available = true, !have_pending_request = true,
we enter into the loop. Good.

  2. All data are consumed, data_available = false, we do not enter into
the loop. Good.

  3. A request is not satisfied because of partial data: data_available =
true, !have_pending_request = false, ready = false, we do not enter into
the loop. Good.

  4. Parsing is forced to stop because of exception. data_available = false.
We do not enter into the loop. Good.

  5. Parsing current dataunit has finished, we still have residual data -
wrong data? data_available = true, !have_pending_request = false, ready =
true (from last parsing). We enter into the loop, and start a new round of
parsing. As expected. Good.

So far so good. :-)

Jimmy

____________________________________________________________
The future is not set.  There is no fate but what we make
for ourselves.             - Terminator II, Judgment Day
------------------------------------------------------------