Re: Have tshark discard state when doing ring-buffer capture

[Evan Huus <eapache () gmail com>]

> On Jul 1, 2014, at 3:51, Anders Broman <anders.broman () ericsson com> wrote:
>
> Hi,
> This is perhaps a better place to have the discussion of the implementation, copying info from gerrit, etc here:
>  
> Anders wrote:
> > I suppose tshark epan new calls the dissector initial routines clearing all hashtables etc,during heavy load that 
> > might trigger packet loss I guess.
>  
> Evan wrote:
>  
> > As Anders correctly pointed out in I7d8f84b2e, constantly resetting state will
> > turn init_dissection into a bit of a hot path. Especially as we will already
> > bear the overhead of switching files, we don't want to fall any further behind
> > than we have to.
> > This change includes three unrelated optimizations that reduce the cost of
> > init_dissection by about 40% as measured by callgrind:
> > - only initialize ares/ADNS if that preference is enabled (this of course only
> >   applies if you specify -n to tshark or otherwise disable the preference)
> > - use memcpy instead of a loop in sigcomp UDVM init
> > - use memcpy instead of a loop in bootp dissector
> ( Commited https://code.wireshark.org/review/2738 )
> > The only remaining obvious hot spot in this path is reassembly_table_init since
> > it is called by so many dissectors. Suggestions (perhaps to get rid of the
> > GPtrArray) welcome.
>  
> Anders wrote:
>  
> > Could switching of files be made more efficient by preopening the next ring buffer file and delegate closing and 
> > opening next file to a new tread?

I don't think there's a way to preopen the next file because it hasn't been created until packets have actually been 
written to it. We could move the close into a separate thread, but I doubt that's going to be slow anyways.

> Another possibility that might boost performance is to remove reassembled data from the hash table and store it in 
> per-packet-data reducing the size of the hash table and hopefully making lookups faster as well as destroying the 
> hash table  (wmem hash tables?) or use
> g_hash_table_remove_all ()

Not clear on what you mean by storing reassembled data in per-packet data (allocate it in file scope?). You are right 
though that many hash tables are currently completely destroyed and re-created where it would be more efficient to 
simply remove all elements.
> Did you profile ring buffer switchover? ( how?).

I profiled init_dissection using callgrind's --toggle-collect flag [1]. I imagine you could profile buffer switchover 
similarly by specifying the capture_input_new_file function. Specifying --collect-systime would probably be useful as 
well to see how much time is being spent in system calls.

[1] http://valgrind.org/docs/manual/cl-manual.html#cl-manual.options.collection

> So in general I think you are right this *is* the expected behavior of tshark with ring buffers, but I fear that it 
> might not be as useful as expected under heavy traffic because of potential packet drops during file switchover of 
> files unless we can make it more efficient. I have no data verifying that this is the case so if we could device a 
> measurement that would be great.

I don't have access to the traffic load that would trigger this. If someone else does it should be pretty easy to do a 
before/after comparison of how well ring-buffered tshark keeps up.

> Regards
> Anders
>  
> From: wireshark-dev-bounces () wireshark org [mailto:wireshark-dev-bounces () wireshark org] On Behalf Of Evan Huus
> Sent: den 30 juni 2014 23:19
> To: Wireshark Developer List
> Subject: [Wireshark-dev] Have tshark discard state when doing ring-buffer capture
>  
> I was kind of expecting this change to generate more controversy, so I'll give it another few days but if nobody 
> objects I'll merge it then.
>
> https://code.wireshark.org/review/2669
>
> I don't currently plan on putting it in 1.12 so that we have a full dev cycle to work out any subtle implications, 
> but I know it's a fairly heavily-requested feature so I'm willing to entertain the notion if somebody wants to argue 
> for it.
>
> Evan
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe