Opened 11 years ago

Closed 5 years ago

Last modified 5 years ago

#155 closed defect (fixed)

handle IPv6

Reported by: Brian Warner Owned by:
Priority: major Milestone: 0.12.4
Component: network Version: 0.4.1
Keywords: Cc: Zooko, clashthebunny@…

Description (last modified by Brian Warner)

Tahoe#867 is about supporting IPv6 in a tahoe grid. For that to work, we need v6 support in Foolscap. This depends upon getting v6 support in Twisted (for which you should look at Twisted#3014, still open as of august-2010).

Thanks to Zooko's persuasion in #60, FURLs with v6 addresses are likely to be tolerated (i.e. ignored) by current versions of Foolscap (0.2.6 or later, so everything since may-2008). In particular, any connection hint with more than one colon is ignored. So backwards compatibility shouldn't be a problem.

The FURL syntax for a v6 address would be pb://TUBID@ipv6:[fe80::1%lo0]:12345/SWISS, and the syntax for a v4+v6 FURL would be pb://TUBID@127.0.0.1:12345,ipv6:[::1]:12345/SWISS or pb://TUBID@ipv4:127.0.0.1:12345,ipv6:[::1]:12345/SWISS .

The new parsing rules will have a legacy-ipv4 rule that matches hints with a single colon, so anything else will be in the form TYPE:ADDRESS:PORT. (future schemes that don't have two-part address+port identifiers, like Tor hidden services or #151 I2P endpoints will need an extra colon, like i2p::I2PHASH.b32.i2p, to avoid being confused with legacy IPV4ADDR:PORT pairs).

Once the Twisted work is done, the foolscap-side work is a new parser in source:foolscap/referenceable.py (decode_location_hints) to spot the v6 addresses, then work in negotiate.TubConnector to stop ignoring non-ipv4 hints and call reactor.connectTCP6 or whatever the new API is. We'll also need code to parse the Tub.listenOn strports spec to handle v6, and something for setLocationAutomatically to take a v6 hint and construct a FURL from it.

Change History (21)

comment:1 Changed 10 years ago by Brian Warner

Description: modified (diff)

Twisted-12.0.0 (released 10-Feb-2012) appears to contain enough support (in reactor.listenTCP and reactor.connectTCP) to enable client/server handling of v6 address literals. I *think* that's enough to let us:

  • use v6 address literals in Tub.listenOn
  • connect to v6 address literals found in FURL connection hints

If we also extend Tub.setLocationAutomatically to learn how to detect v6 addresses on local interfaces, then we could get numeric v6 addresses in automatically-generated connection hints.

What Twisted doesn't handle yet is DNS resolution, so we can't yet tolerate is folks modifying their FURL's connection hints to replace the numeric IPv6 address with a DNS name (that has an A6 or AAAA record).

The Tub.listenOn and Listener code may need some work: I don't know if the current use of internet.TCPServer is v4-only or not.

comment:2 Changed 9 years ago by Marcus Wanner

https://github.com/marcuswanner/foolscap/tree/ipv6

Would like a quick review of this to make sure I'm doing things right. Unit tests pass, but I have not added any for ipv6.

On a side note, do you know if anything needs to be done with tahoe to make it work with ipv6, or will it just work now that this is done?

comment:3 in reply to:  2 Changed 9 years ago by Zooko

Replying to marcusw:

On a side note, do you know if anything needs to be done with tahoe to make it work with ipv6, or will it just work now that this is done?

There are some notes about what would be required in Tahoe-LAFS over here:

https://tahoe-lafs.org/trac/tahoe-lafs/ticket/867

comment:4 Changed 9 years ago by Zooko

marcusw: nice work writing these patches! ☺

I'll post some comments on the patches on github.

comment:5 Changed 9 years ago by Zooko

Cc: Zooko added

comment:6 Changed 9 years ago by Zooko

I didn't notice this comment from ClashTheBunny on github. I'll try to figure out how to get github to notify me about such things, but also I'm copying it in here from https://github.com/marcuswanner/foolscap/commit/d890c7739008103ebdd2ecdb5dab971ef9cee484 :

I believe that one of the things that need to be updated is the minimum required twisted for foolscap to now work. It could be stated in the documentation that for IPv6 to work twisted 12.1.0 or greater is required, but it could just be that from this version on 12.1.0 or greater is required. Right now it seems that setup.py refers to ">= 2.4.0" and the README file refers to "2.5.0 or later".

We wouldn't want to over complicate this and add weird if twisted < 12.1.0 statements. Plus, it would allow getting rid of the custom made parse_strport(). When do you choose backwards compatibility over moving forward when the future is pretty clearly IPv6?

comment:7 Changed 9 years ago by Zooko

I'm okay with requiring Twisted ≥ 12.1.

comment:8 Changed 9 years ago by Randall Mason

Owner: set to Randall Mason

comment:9 Changed 9 years ago by Randall Mason

I emailed Marcus about IPv6 in Tahoe and he mentioned that he is busy with school, so I took a look at foolscap and made some updates to versions, tests, and places where tcp4 is hard coded. You can see the differences between foolscap master and my ipv6 branch here (including Marcus's wonderful patches):

https://github.com/ClashTheBunny/foolscap/compare/master...ipv6

It currently passes most tests, but I'm trying to get foolscap to pass a few of the tests that I wrote that are analogs to current ipv4 tests. If somebody could review the tests that I've written, I would be very grateful. That's the start of what I want to make sure is in order, which I believe it is in order.

Just a note about Marcus's patch: it's been working perfectly on my dual stack Linux distros with my work with Tahoe-LAFS. You can follow my work on github here if you're interested:

https://github.com/ClashTheBunny/tahoe-lafs/compare/master...ipv6

and on tahoe's trac here:

IPv6 in Tahoe-LAFS

comment:10 Changed 9 years ago by Randall Mason

Keywords: review-needed added

If somebody could review the tests that I've written, I would be very grateful. They all pass on bindv6only hosts with both IPv6 and IPv4, dual-stack hosts with both IPv6 and IPv4, and hosts with IPv4 only. I don't have access to a machine yet with only IPv6. If the tests are good, then the code changes need review.

Notes on how to do the tests I'm talking about on Linux for reproducibility and clarity:

for disablev6 in 0 1
do
    for sysctlvar in net.ipv6.conf.all.disable_ipv6 net.ipv6.conf.default.disable_ipv6 net.ipv6.conf.lo.disable_ipv6
    do
      sudo sysctl -w $sysctlvar=$disablev6
    done
    if [ $disablev6 -eq 0 ]
    then
        for bindv6only in 0 1
        do
            echo $bindv6only | sudo tee /proc/sys/net/ipv6/bindv6only
            trial foolscap
        done
    else
        trial foolscap
    fi
done | tee testResults.txt | grep -e skip -e success

Which results in:

PASSED (skips=4, successes=486)
PASSED (skips=4, successes=486)
No IPv6, skipping
No IPv6, skipping
PASSED (skips=19, successes=471)

I don't know if there is a way to automate this in Python's regular tests; it requires root. I think we just need build bots that have this built into the build procedure or are permanently set up this way.

It is based on these code snippets:

Disable IPv6:

for sysctlvar in net.ipv6.conf.all.disable_ipv6 net.ipv6.conf.default.disable_ipv6 net.ipv6.conf.lo.disable_ipv6
do
  sudo sysctl -w $sysctlvar=1
done

Enable IPv6:

for sysctlvar in net.ipv6.conf.all.disable_ipv6 net.ipv6.conf.default.disable_ipv6 net.ipv6.conf.lo.disable_ipv6
do
  sudo sysctl -w $sysctlvar=0
done

Disable dual-stack (bindv6only):

echo 1 | sudo tee /proc/sys/net/ipv6/bindv6only;

Enable dual-stack (disable bindv6only):

echo 0 | sudo tee /proc/sys/net/ipv6/bindv6only;

comment:11 Changed 9 years ago by Randall Mason

Owner: Randall Mason deleted

comment:12 Changed 9 years ago by Randall Mason

Cc: clashthebunny@… added

comment:13 Changed 6 years ago by Brian Warner

I think our push to use Endpoints (#203) should give us v6 compatibility. The two big pieces are:

  • Allow tub.listenOn("tcp6:%d"), which (if v6 is present in the kernel) should listen on a port that accepts connections via both ipv4 and ipv6 (TCP ports share a namespace). We need to confirm that this won't explode if v6 support is not present, or that there's some reasonable way to fall back to v4-only.
  • Use HostnameEndpoint for "tcp:" connection hints instead of the current v4-only TCP4ClientEndpoint. This gives us v6 client support as long as the server's DNS includes AAAA records. I would have used this already, but there's a bug in Twisted (twisted#8014) which throws Unhandled Error In Deferreds when foolscap sees multiple hints, so we need to get that fixed first.

We might also want code to parse the hints for colon-hex IPv6 address literals, which might justify a separate "tcp6:" hint (but which I'm hoping to avoid in general). The idea is that a "tcp:" hint contains either a DNS name (which might resolve to v4, v6, or a combination of both), or a dotted-quad v4 literal. We could extend the "tcp:" syntax to allow a square-bracketed colon-hex v6 literal, or somehow parse it without brackets (sounds like a bad idea, to be honest).

When I get done with some other work, I'll study the changes that marcusw and ClashTheBunny have done, and see how much of that can still be used in an Endpoint-centric world.

comment:14 Changed 6 years ago by Brian Warner

Component: negotiationnetwork

comment:15 Changed 6 years ago by Brian Warner

Rats, Twisted-15.5.0 still has this problem, which causes test_negotiate.ThreeInParallel.test1 to fail.

comment:16 Changed 6 years ago by Brian Warner

Yay, the Twisted fix has landed! The next Twisted release will work for us.

So the next steps for this issue are:

  • fix the rest of #203 by using Endpoints on the listening side, so it's possible to listen on an ipv6 port
  • use the tests by marcusw and ClashTheBunny to exercise v6 in the unit tests on hosts that can handle it
Last edited 6 years ago by Brian Warner (previous) (diff)

comment:17 Changed 6 years ago by Brian Warner

I've landed the HostnameEndpoint change in [711990e1], so now remote hints that use a DNS name that map to an AAAA record (and maybe an A6 record, I'm not sure) should get a v6 connection.

What we're still lacking:

  • #203, use Endpoints on the listening side, to listen on ipv6 ports
  • a parser than can handle square-bracketed colon-hex v6 literal addresses in hints

comment:18 Changed 5 years ago by Brian Warner

I think dawuud is going to work on the parser. The goal is to accept all three address formats in a tcp: -type hint:

  • tcp:example.org:12345
  • tcp:10.1.2.3:4567
  • tcp:[2600:3c01::f03c:91ff:fe93:d272]:12345

We'll want to change foolscap/connections/tcp.py, to add a new COLON_HEX_RESTR regexp (which should match on, and include, the [ ] brackets). Then NEW_STYLE_HINT gets updated to include COLON_HEX_RESTR as a third option, and DefaultTCP.hint_to_endpoint needs to do a host = host.lstrip("[").rstrip("]") to pull the brackets off.

We need to check the tor/socks handlers to make sure they'll probably accept/reject/strip any ipv6 addresses they get, since they're probably sharing the regexp strings.

I think we should be able to add a test that exercises tcp:[::1]:PORT (i.e. loopback), although it might be good to test that the host is capable of making such connections. But for sure we can have a test that mocks out HostnameEndpoint and makes sure the parser gives it the right address string (without the square brackets).

We have no plans to do automatic IPv6 address detection (via ifconfig/etc) like we used to do for v4.

comment:19 Changed 5 years ago by dawuud

fixed. here's a PR: https://github.com/warner/foolscap/pull/30

i see now a couple of things i'll fix based on warner's above comments. so by the time warner looks at the PR it should be cleaned up and ready for review.

comment:20 Changed 5 years ago by Brian Warner

Keywords: review-needed removed
Milestone: undecided0.13.0
Resolution: fixed
Status: newclosed

Fixed, in [fd116ab8] . Thanks!

comment:21 Changed 5 years ago by Brian Warner

Milestone: 0.13.00.12.4
Note: See TracTickets for help on using tickets.