On 2013-08-28, at 1:22 AM, Anders Broman <[email protected]> wrote:

> Evan Huus skrev 2013-08-28 00:17:
>> On Tue, Aug 27, 2013 at 6:02 PM, Anders Broman <[email protected]> wrote:
>>> Joerg Mayer skrev 2013-08-27 23:24:
>>> 
>>>> On Tue, Aug 27, 2013 at 05:09:19PM -0400, Evan Huus wrote:
>>>>>> IIRC, two-pass allows for most/all of the reassembly/request-response
>>>>>> stuff,
>>>>>> which we want to do sometimes. Any ideas why we have to keep some
>>>>>> information
>>>>>> indefinitely?
>>>>> Two-pass requires us to keep *all* the state around through the first pass
>>>>> so that it is available during the second pass (at which point it can be
>>>>> discarded).  Even in single-pass mode, there is some state that we can't
>>>>> always immediately discard. If I see a fragment of a TCP message then it
>>>>> doesn't make sense to discard that until the other fragments have arrived
>>>>> and been reassembled. If I see a request, I probably need to keep state
>>>>> from that request until the response (which may never show up).
>>>>> 
>>>>> We already do reassembly and a lot of other stateful work in single-pass
>>>>> mode. The only thing two-pass mode provides is the ability to "see the
>>>>> future" (for example, saying: this request has a response 5 packets 
>>>>> later).
>>>> So (assuming we really free everything we could already) could add a
>>>> possibly configurable foresight horizon of 10000 packets. If a packet
>>>> number is older than 10000 packets, forget it?
>>> I haven't really looked but we do keep the reassembled fragments around 
>>> even in tshark as there is no
>>> mechanism to discard them selectively if run by tshark as opposed to 
>>> wireshark and that's the really big memory eater I would think.
>> 
>> This is the primary problem. Any state saved with p_add_proto_data, or the 
>> reassembly API, or the conversation API, or (god-forbid) globals is not 
>> being freed right now. Any memory allocated with seasonal/file scope is not 
>> being freed right now. We do free frame data, column data and misc. other 
>> bits and pieces, but the majority of the state we create is not freed until 
>> the end of the file.
>> 
>> As Anders says, this is because we have no way right now to selectively 
>> discard it: much of the data is stored in a way that we can only get rid of 
>> all of it, or none. Part of that is because there was no se_free call (for 
>> which wmem_free is the convenient solution) and part of that is just because 
>> nobody's ever added it to, for example, the reassembly API.
>> 
>> I'm sure there are some significant improvements we could make if somebody 
>> figures out how, but you also have to beware of bugs like 9027 [1] which is 
>> occurring precisely because we were trying to free old reassembly data. It 
>> turns out there are certain cases where that data, at least, could still be 
>> in use. The logic to selectively free state is not simple, and is often 
>> dissector-dependent.
>> 
>> Cheers,
>> Evan
>> 
>> [1] https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=9027
>> 
>> P.S. Given the above it would actually be fairly easy to create a fully 
>> state-less tshark: make file-scope memory redirect to packet-scope, and make 
>> our stateful APIs (p_add_proto_data, reassembly, conversations) no-ops. I'm 
>> sure this will break some dissectors that use globals, but they should be 
>> fixed anyways :)
> 
> >make our stateful APIs (p_add_proto_data, reassembly, conversations) no-ops
> I'm not sure I get this, p_add_proto_data might work as a no-op in tshark's 
> case but isn't conversation data
> reassembly, needed to fully dissect a "future" frame?

That would be the other downside of state-less tshark. You would get no 
reassembly, no request-response matching, no conversations etc. On the other 
hand, memory usage would never go much above baseline :)

> A way to fix the reassembly case might be to store the fully reassembled data 
> with p_add_proto_data rather than
> in a hash_table and in tshark's case discard it when finished with the frame 
> this might have other benefits as well.
> Unfortunately it means changing all dissectors using reassembly but if the 
> most frequently used ones
> where done it might fix most use cases ( TCP, HTTP...?).
> 
>> 
>> 
>> ___________________________________________________________________________
>> Sent via:    Wireshark-dev mailing list <[email protected]>
>> Archives:    http://www.wireshark.org/lists/wireshark-dev
>> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>>              mailto:[email protected]?subject=unsubscribe
> 
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <[email protected]>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>             mailto:[email protected]?subject=unsubscribe
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <[email protected]>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:[email protected]?subject=unsubscribe

Reply via email to