Wireshark-dev: Re: [Wireshark-dev] manual address resolution is broken
From: Evan Huus <eapache@xxxxxxxxx>
Date: Tue, 28 May 2013 21:27:52 -0400
Further follow-up to this (because it keeps getting more complicated :P)

The original version used regular glib memory. It was changed to use
seasonal memory so that changing files would flush the stored name
resolutions. However this is clearly a problem in some situations
(it's what triggered your original investigation). Additionally, this
will not migrate as-is to wmem (the replacement for emem and seasonal
memory) because the wmem 'file' scope is actually only available when
a file is open. Trying to access it when no file is open will throw an
assertion.

Since determining when to free entries seems like it will be
complicated, reverting to a manually-managed scheme might be the thing
to do. Alternatively, it may make more sense to create an internal
wmem pool for each type of name resolution that can be cleared (ie a
pool for hosts file, a pool for manual, etc), so that clearing any
individual name resolution section is as simple as calling free_all on
the associated pool. One of the things on my todo list for wmem is a
proper hash-table implementation, so that might also simplify things
(if I ever get around to it).

Hope the above ends up being enlightening, not confusing.
Evan

On Tue, May 28, 2013 at 9:07 PM, Evan Huus <eapache@xxxxxxxxx> wrote:
> On Tue, May 28, 2013 at 8:32 PM, Ed Beroset <beroset@xxxxxxxxxxxxxx> wrote:
>> Anders Broman wrote:
>>
>>>
>>> Ed Beroset wrote:
>>>>
>>>> My inclination would be for option 2 be the default, but with option
>>>> 1 being available as a configuration checkbox.
>>>
>>> Yes this sounds like the thing to do for me to, regarding address
>>> resolution there has been discussions of a rewrite using "normal" hash
>>> tables
>>> And options not to dump NRB:s
>>> https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7380
>>> https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=8349
>>
>>
>> https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=8462
>>
>> So with this bug report I already knew about, that's three different ones
>> with different aspects, ideas, problems and goals.  As I understand it,
>> there are potentially four different (potential) sources for name
>> resolution.  They are 1) a named hosts file (not necessarily the system
>> hosts file) 2) whatever is behind OS gethostbyaddr() call 3) NRB in capture
>> file and 4) manually entered names.
>>
>> For name resolution, I'm thinking that it might be useful to allow the user
>> to select both the order for resolution and whether each is used or not.
>> Just to make it even more complex, it may be desirable for the user to dump
>> some but not all of the names when a new capture file is loaded.  For
>> example, one might reasonably wish to dump the previous NRB names, but keep
>> the manually entered ones.
>>
>> There should probably also be some kind of options for saving as well.
>> Specifically, it might be useful to allow the user to save a combined hosts
>> file (which could include names resolved via any of the methods listed
>> above) and/or save to an NRB or strip all of it out of the NRB.
>>
>> Also, I think Evan Huus's comment about changing to use glib hash tables is
>> a good one, and would propose to incorporate that as well.
>>
>> Too complex?  Did I miss anything?  I think it's better to decide the
>> behavior first and then code it, so thanks very much for your comments.
>
> All of the above sounds good to me. Also, the request from 8349 (only
> write NRB entries for subset being exported) would be nice to have as
> well if it's possible to do it efficiently. I think it should be
> pretty straight-forward to add a pointer to frame_data or packet_info
> or something and build the NRB at save time, but you'll have to touch
> the wiretap code as well, so it could get messy.
>
> Something else to keep in mind is the Qt gui, since we eventually plan
> to migrate away from the current GTK gui in favour of Qt. I don't know
> the state of name resolution in the current Qt gui, but since the GTK
> options are more than a bit haphazard and hard to manage, designing
> (and/or implementing) a good way for Qt to deal with it would be a
> bonus.
>
> It certainly seems like a big project, but even just getting one of
> the parts fixed would be a big win. Thanks in advance :)
> Evan
>
> P.S. As always, many incremental patches are better than one monster
> at the end, both because they're easier to merge and because they mean
> faster design feedback. Please CC me if you file a new bug to track
> this.