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

Invalid argument exception during socket.accept on AF_INET6 #164

Closed
buhman opened this Issue May 19, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@buhman
Member

buhman commented May 19, 2017

minimal-ish testcase and exception: https://gist.github.com/8d647f4c8368bc318e73833474601526

If I remove the setsockopt call on line 192, the testcase works as expected.

Is this a bug, or am I not creating an IPv6 socket correctly?

@buhman

This comment has been minimized.

Member

buhman commented May 19, 2017

I found this from the ipv6(7) manpage:

The default value for [IPV6_V6ONLY] is defined by the contents of the file /proc/sys/net/ipv6/bindv6only. The default value for that file is 0 (false).

I did a bit of testing, and found this behavior with respect to the sockets created by accept:

connecting listening accept
socket type address type socket type address type socket type v6only value
AF_INET6 ipv6 AF_INET6 ipv6 AF_INET6 1
AF_INET6 mapped-ipv4 AF_INET6 mapped-ipv4 AF_INET6 0
AF_INET6 mapped-ipv4 AF_INET ipv4 AF_INET6 0
AF_INET ipv4 AF_INET6 mapped-ipv4 AF_INET X

It seems like because:

  • accept-v6onlyness seems correct to begin with
  • we should probably respect the system default
  • IPV6_V6ONLY doesn't seem to be supported on all INET6 sockets

... that we should remove trio/socket.py#L191

@njsmith

This comment has been minimized.

Member

njsmith commented May 19, 2017

Right, there are two issues here: (1) what we should be doing with IPV6_V6ONLY by default (which overlaps with #72), and (2) fixing the exception.

Am I reading your table correctly as saying that the problem happens when you set up a listen socket with IPV6_V6ONLY set to False, then accept a v4 connection, and then trio tries to set IPV6_V6ONLY on the new labeled-as-ipv6-but-secretly-actually-a-v4-socket? That makes a lot of sense :-). So at the very least we should tolerate an error here.

As to the larger question of defaults, there are a few competing considerations:

The problem with accepting the system default is that it leads to non-portable programs (in particular in this case: test on linux, everything works fine because in fact everyone leaves bindv6only set to 0, and then you're broken on windows because they make IPV6_V6ONLY default to enabled. And in practice, setting V6ONLY=False doesn't actually make programs magically handle ipv4 and ipv6 equally, because e.g. people expect that their web server's connection logs will contain ipv4 addresses rather than ipv6-mapped-ipv4 addresses. So I figured better to have consistency and then let people pick if they want something different... libuv does something similar, though IIRC they have the opposite default from trio's current setting. I don't have a strong opinion on which default is better :-)

But then there's another issue: initially I hoped that trio.socket would be the standard user-level API for doing networking in trio, but in the course of implementing SSL support have come around to the conclusion that we need a higher-level layer that provides a dedicated "stream" abstraction. There's a huge work-in-progress branch in #107 with the initial part of this. As part of this, I've defined a SocketStream class that adapts a socket into the Stream interface; and probably I will also add high-level helpers for connecting to a host+port and for listening on a port, that are defined at this level. So... if trio.socket is becoming the "low-level raw" networking API, maybe it does make more sense to remove the default-setting code from here, and instead handle it in the higher-level layer. (So in particular, the "listen on this port" interface would automatically create both ipv4 and and ipv6 sockets, and transparently listen on both of them.)

@buhman

This comment has been minimized.

Member

buhman commented May 19, 2017

and then trio tries to set IPV6_V6ONLY on the new labeled-as-ipv6-but-secretly-actually-a-v4-socket?

This is also a problem, but what I was originally trying to describe, completely ignoring SocketType.__init__ for a moment, and the ipv6-mapped-ipv4 stuff, is that if you do the equivalent of:

with trio.socket.socket(trio.socket.AF_INET6) as socket:
    socket.bind((host, port))
    socket.listen()

    accept_socket = await socket.accept()
    assert accept_socket._sock.family == AF_INET6
    accept_socket.setsockopt(IPPROTO_IPV6, IPV6_V6ONLY, False)

...that the accept_socket.setsockopt call will throw OSError: [Errno 22] Invalid argument, and it doesn't matter what value you pass to IPV6_V6ONLY.

In the table, I'm just trying to show that it probably doesn't make sense to try to IPV6_V6ONLY-normalize the sockets returned by accept().

@njsmith njsmith closed this in #166 May 20, 2017

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