[tor-bugs] #5053 [Tor Bridge]: Fix IPv6 implementation for bridge statistics
Tor Bug Tracker & Wiki
torproject-admin at torproject.org
Sun Feb 26 09:38:08 UTC 2012
#5053: Fix IPv6 implementation for bridge statistics
------------------------+---------------------------------------------------
Reporter: karsten | Owner:
Type: defect | Status: needs_review
Priority: major | Milestone: Tor: 0.2.3.x-final
Component: Tor Bridge | Version:
Keywords: | Parent:
Points: | Actualpoints:
------------------------+---------------------------------------------------
Comment(by ln5):
Replying to [comment:19 nils]:
> > - I would've collapsed more of the functions into one too,
> > f.ex. geoip_get_country_by_ip() by adding a 'family' argument.
>
> This is a bit tricky seeing as the arguments and data structures are all
different types. geoip_get_country_by_addr already does the selection on
address family and dispatches to the proper routine. Or maybe I'm not
understanding what you're suggesting here.
Ah, I missed geoip_get_country_by_addr(). It indeed does what I
suggest. It'd be nice to hide geoip_get_country_by_ip() and
geoip_get_country_by_ipv6() by making them static. Unless callers
will get inefficient or ugly by calling tor_addr_from_ipv4n()?
> > - I'd renamed data types, lists and functions specific to IPv4 (like
> > struct geoip_entry_t) but I'd like to hear others view on this.
>
> I would like to hear others views on this as well. I went for having a
smaller changeset rather than renaming all of them, but it's easy enough
to rename them.
That change would go in a separate commit, preferrably predating your
large commit with the real implementation.
> > - There's tor_addr_compare() which I think should be used instead of
memcmp().
>
> Unless I'm missing some, all of the memcmp cases are when we have
specifically a in6_addr. I guess we could use tor_addr_from_in6 to make a
tor_addr_t and then use tor_addr_compare, but that seems like a lot of
overhead and isn't nearly as clear.
Aha! struct geoip_ipv6_entry_t has in6_addr's, not tor_addr_t's. Makes
sense then.
> I chose not to have a combined geoip database using tor_addr_t because
of the additional memory usage (there are about 150,000 entries in the
ipv4 geoip database) and it seemed a bit wasteful, but if we want to waste
the extra 4 MB RAM we could use generic tor_addr_t addresses for the in-
memory structure.
No, separate structures is the right thing IMO.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/5053#comment:20>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
More information about the tor-bugs
mailing list