[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: Tor Ipv6-Patch



On Mon, Dec 03, 2007 at 11:51:29PM +0100, Marcus Wolschon wrote:
> 
> Dear list,
> 
> I wrote an extensive patch to make tor support
> * hidden services on ipv6-addresses
> * binding for socks on an ipv6-address
> * convert a lot of the internal data-structures for the comming full
> ipv6-support.
> 
> I wrote it to tor-volunteers@xxxxxxxxxxxxxx on friday but got no
> answer thus I am posting them here.
> 
> The code may not use the prefered naming-conventions or the
> tor_address_t -data-structure (because that one uses socket_addr
> and we only need the actual address-part) but I guess a refactoring
> for it to meet the coding-guidelines is far less work then writing it
> and getting it to work in the first place.

Hi, Marcus!  Thanks, and good job!  The 0.2.0.x series is in
feature-freeze now, but I'll save this patch to be sure to merge it once
0.2.1.x is branched.

A few general points below in case you (or somebody else) wants to work
on this patch some more.  None of these are show-stoppers, but fixing
them will make the patch better.

 - You're right that the preferred way to store addresses that could be
   either IPv4 or IPv6 is indeed with tor_addr_t.  (Thanks for the
   reminder, BTW: I fixed tor_addr_t to be a tagged union of in_addr and
   in6_addr, not of sockaddr_in and sockaddr_in6.)

 - Generally speaking, we try to avoid duplicated "cut and paste" code;
   it makes it hard to be sure that we can update all instances of the
   original code path.  (connection_or_init_conn_from_address6, for
   example, is almost exactly the same as the existing
   connection_init_conn_from_address.)

 - It's good to break up patches into functional parts, since shorter
   patches are way easier to review.  For example, this patch includes
   lots of log cleanups that make the logs say "hidden service" instead
   of "service".  That's a cleanup, and that can go into 0.2.0.x on its
   own, but it doesn't seem to be dependent much on the rest of the
   patch.  Also, the different functional changes could probably be
   separated.

Thanks again for your patch!  I've wanted IPv6 support for a long time;
it will be great to finally get it in.

peace,
-- 
Nick



Attachment: pgp14HBpC4WrH.pgp
Description: PGP signature