Re: [Zenmap-Patch] Reducing Topology Noise

[Jay Bosamiya <jaybosamiya () gmail com>]

Dan,

Committed (with the cleanup) as r33377.

I have reworked the patch for the differing_length suggestion that you
made.
All other functionality is left unaffected.

Please find the patch attached.

In this patch, the key can become None, however (only for
descendant_key) and so the tests for "None" are required.

Feedback is welcome, as always :)

Cheers,
Jay

On Tuesday 29 July 2014 11:44 PM, Daniel Miller wrote:
> Jay,
>
> Sorry it has taken so long for me to get to this. Overall, I like it.
> Here are a few thoughts:
>
> 1. differing_length.xml doesn't seem to be different from old to new,
> and I think it probably should be. Right now it looks like:
>
>        / b -> X \
> .. -> a          c -> ..   
>        \ X -> X /
>
> but I think a better way would be:
>
>        / b \
> .. -> a     X -> c -> ..   
>        \ X /
>
> Unfortunately, this means a more difficult reworking of your patch. As
> I see it, your patch treats an anon hop as collapsible if it has the
> same distance and the same "known ancestor" (hop with a real IP that
> is closer than this anon hop). To do this sort of collapse, you'd have
> to also allow hops with the same distance and the same "known
> descendant" (hop with a real IP that is farther than this anon hop). I
> think you could do this by entering the same node into
> invalid_node_cache under 2 different keys: (pre_hop["ipaddr"],
> pre_hop_distance) and (post_hop["ipaddr"], post_hop_distance), if that
> makes sense. Of course, be careful that hop_split_at_different_real is
> not affected!
>
> 2. A minor cleanup thing: since you set key = (pre_hop["ipaddr"],
> pre_hop_distance), key cannot be None, so take that condition out of
> the following if statement. By the same token, the condition "if key
> is not None and key in invalid_node_cache:" can never be false, since
> if "key not in invalid_node_cache" then the previous block will put it
> into the cache. Just make the node assignment an unconditional statement.
>
> Other than that, the patch looks good. You may want to just commit the
> patch as it is now (possibly with that small cleanup in 2) and then
> consider my suggested change as a separate effort. Thanks!
>
> Dan

Attachment: ancestor_descendant.patch
Description:

_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/