Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dual stack IPv6 support. #864

Merged
merged 3 commits into from
Nov 2, 2017
Merged

Add dual stack IPv6 support. #864

merged 3 commits into from
Nov 2, 2017

Conversation

jgosmann
Copy link
Collaborator

@jgosmann jgosmann commented Feb 7, 2017

This implements support for IPv6 with a dual stack solution supporting IPv4 and IPv6 at the same time. In theory this also allows to listen on multiple ports or network interfaces at the same time. But this is not exposed within main.py, so the nengo command only allows to listen on all localhost addresses (no password protection) or all interfaces (with password protection). I assume this to be fine in almost all cases and do not intend to change this as long as there are no such requests because it would complicate the interface without need.

Due to the current implementation of the WebSocket origin checking the GUI does not support access by IP address (e.g. 127.0.0.1 or [::]), except when started with a password.

Addresses #21.

@tcstewar
Copy link
Collaborator

tcstewar commented Feb 8, 2017

How does this PR interact with #808? Should I merge #808 first (which I really should have done months ago), and then look at this? Or are they pretty independent?

@jgosmann
Copy link
Collaborator Author

jgosmann commented Feb 9, 2017

Completely forgot about that one. I'd suggest merging this one first. They are independent for the most part, but there are a few things in #808 which I now think are not right to do (changing “localhost” to “::”, switching the whole server to AF_INET6 with no fallback to AF_INET). It's probably easier to sort those out after rebasing on this PR.

The changes in #808 should allow the access via IP address together with this PR.

@tcstewar
Copy link
Collaborator

For my Windows 10 laptop, this fails to allow connections. It looks like self.bindings is empty. self.server_name is localhost and then this for loop fails to find anything to use: https://github.com/nengo/nengo_gui/pull/864/files#diff-6b004fd2ea83a7abcc0443f852b14d6fR149

@jgosmann
Copy link
Collaborator Author

What does the socket.getaddrinfo(*server_address) call return? (Or as an approximation what does socket.getaddrinfo('localhost', 8080) return?

@tcstewar
Copy link
Collaborator

What does the socket.getaddrinfo(*server_address) call return? (Or as an approximation what does socket.getaddrinfo('localhost', 8080) return?

Here's the result:

[(<AddressFamily.AF_INET6: 23>, 0, 0, '', ('::1', 8080, 0, 0)), (<AddressFamily.AF_INET: 2>, 0, 0, '', ('127.0.0.1', 8080))]

@jgosmann
Copy link
Collaborator Author

That problem should be fixed (but there might be more).

@tcstewar
Copy link
Collaborator

That problem should be fixed (but there might be more).

:) Here's the new error:

Traceback (most recent call last):
  File "C:\Users\Terry\py3\Scripts\nengo-script.py", line 11, in <module>
    load_entry_point('nengo-gui', 'console_scripts', 'nengo')()
  File "c:\users\terry\documents\github\nengo_gui\nengo_gui\main.py", line 77, in main
    page_settings=page_settings)
  File "c:\users\terry\documents\github\nengo_gui\nengo_gui\gui.py", line 43, in __init__
    self.model_context, server_settings, page_settings)
  File "c:\users\terry\documents\github\nengo_gui\nengo_gui\guibackend.py", line 366, in __init__
    self, self.settings.listen_addr, GuiRequestHandler)
  File "c:\users\terry\documents\github\nengo_gui\nengo_gui\server.py", line 240, in __init__
    DualStackHttpServer.__init__(self, *args, **kwargs)
  File "c:\users\terry\documents\github\nengo_gui\nengo_gui\server.py", line 156, in __init__
    self.bindings.append(self.Binding(fam, addr))
  File "c:\users\terry\documents\github\nengo_gui\nengo_gui\server.py", line 126, in __init__
    socket.IPPROTO_IPV6, socket.IPV6_V6ONLY, 1)
AttributeError: module 'socket' has no attribute 'IPPROTO_IPV6'

@jgosmann
Copy link
Collaborator Author

But socket.AF_INET6 is defined and socket.has_ipv6 is True?

@jgosmann
Copy link
Collaborator Author

Hm, this seems to be a known Python bug?

@jgosmann
Copy link
Collaborator Author

Does the socket module have socket.IPPROTO_IPV4?

@jgosmann
Copy link
Collaborator Author

The magic value to add is 41. (I suppose it's not 42 because we start counting at 0? 😉 )

@jgosmann jgosmann self-assigned this Oct 30, 2017
@jgosmann
Copy link
Collaborator Author

Should be fixed.

@tcstewar
Copy link
Collaborator

Should be fixed.

THat's now working for me in Windows 10 (py27, py35, py36)... I'm going to try it on a Windows 8 machine tonight....

@tcstewar
Copy link
Collaborator

This also works fine on Windows 8... :)

However, Travis-CI seems upset by something about this... is this a problem with Travis-CI or with this PR?

@tcstewar
Copy link
Collaborator

I can try rebasing to master (and force-pushing) and seeing if that helps....

@jgosmann
Copy link
Collaborator Author

jgosmann commented Oct 31, 2017 via email

@tcstewar
Copy link
Collaborator

Probably travis... I think the tests have been broken for a while? Can't verify right now ...

I'll do the rebase and see if that helps...

@tcstewar
Copy link
Collaborator

(er, never mind, you're still assigned to it so I shouldn't do a force-push on it.... especially since I just asked you about that today! :) )

@jgosmann jgosmann removed their assignment Oct 31, 2017
@jgosmann
Copy link
Collaborator Author

you're still assigned

Sorry, forgot to unassign myself after adding the last fix.

The tests are failing on the same way on master and the last succesful “build” has been 7 months ago ...

@jgosmann
Copy link
Collaborator Author

Related PR #839.

@tcstewar
Copy link
Collaborator

tcstewar commented Nov 2, 2017

I'm all set with this.. thank you for your patience on this one.... :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants