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

ei_connect does not handle IPv6 #5068

Open
sax opened this issue Jul 22, 2021 · 8 comments
Open

ei_connect does not handle IPv6 #5068

sax opened this issue Jul 22, 2021 · 8 comments
Assignees
Labels
enhancement help wanted Issue not worked on by OTP; help wanted from the community priority:low team:VM Assigned to OTP team VM

Comments

@sax
Copy link

sax commented Jul 22, 2021

Problem statement:

I'm working on an Elixir project that uses the membrane framework, depending heavily on cnodes. In general everything has just worked, but my team recently tried to deploy to a PaaS (fly.io) which creates an IPv6 wireguard mesh on which nodes can connect. We started the application with RELEASE_DISTRIBUTION of name, and started epmd with -proto_dist inet6_tcp, whereupon the application could not connect to any cnodes started by membrane.

Later we discovered the ei_connect documentation, where it clearly states:

The addr argument of the listen, accept, and connect callbacks refer to appropriate address structure for currently used protocol. Currently ei only supports IPv4. That is, at this time addr always points to a struct sockaddr_in structure.

It seems like epmd has had contributions in the past few years to support IPv6, but this work has not tricked down into the C functions for cnodes.

Proposed solution

If possible, it would be great for the C libraries shipped with erl_interface to support both IPv4 and IPv6.

Alternatives

Our primary workaround for right now is to avoid using this particular PaaS, and deploy to a platform that allows us to connect application nodes via an IPv4 network.

If we do find ourselves deploying somewhere where nodes must be connected via IPv6, our secondary workaround is to start the application epmdless using ERL_DIST_PORT, so -start_epmd false, but with the node name including the IPv6 address, then make sure that all cnodes are started with node name including 127.0.0.1. Our thought is that application nodes can connect to each other via specific ports on IPv6 without epmd, and cnodes can connect via epmd on localhost.

@rickard-green
Copy link
Contributor

Unfortunately IPv6 support for erLinterface currently has a low priority for us, so this is not something we will be working on in the near future. If you want to help out, a pull request implementing IPv6 support is welcome.

@rickard-green rickard-green added help wanted Issue not worked on by OTP; help wanted from the community priority:low labels Aug 9, 2021
@mat-hek
Copy link

mat-hek commented Aug 10, 2021

Hi there, we can try to contribute, though we'd really appreciate some guidelines on what exactly needs to be changed. Thanks

@rickard-green
Copy link
Contributor

I'm not able to tell you exactly what needs to be changed, but I can give you a couple of general guidelines.

Compatibility is important. The more the better, but some incompatibilities may have to be accepted. I would say binary incompatible changes are acceptable. That is, requiring the user to recompile the code that he/she wants to link against the ei library is ok, but the user should not have to change his/hers existing code if it should work with ipv4.

It would be nice if there wasn't an explosion of new functions, but it is perhaps unavoidable.

I also just noted that the Erl_IpAddr is not correctly documented. It is documented as a struct, but it is a pointer to a struct.

@mat-hek
Copy link

mat-hek commented Aug 11, 2021

Ok, so from my brief research and according to this article the things to solve are:

  • Erl_IpAddr is a typedef to a pointer to struct in_addr, which is only able to carry IPv4 address

    A solution could be to change it to a pointer to a custom struct:

    struct erl_ipaddr {
      union {
        uint32_t s_addr;
        unsigned char s6_addr[16];
      };
      enum erl_ip_version ip_version; // is something like that defined somewhere? IPv4 would have to be under 0 to make it backwards compatible
    }
  • ei_connect_helper uses sockaddr_in

    It could determine the IP version from erl_ipaddr struct and use either sockaddr_in or sockaddr_in6

It seems that ei_portio doesn't require any changes - it should work with sockaddr_in6 out of the box.

@rickard-green
Copy link
Contributor

Ok, so from my brief research and according to this article the things to solve are:

* `Erl_IpAddr` [is a typedef](https://github.com/erlang/otp/blob/c41c3553d6978d1d2e909f826c0f33499e845265/lib/erl_interface/include/ei.h#L382) to a pointer to `struct in_addr`, which is only able to carry IPv4 address
  A solution could be to change it to a pointer to a custom struct:
  ```c
  struct erl_ipaddr {
    union {
      struct in_addr s_addr;
      struct in6_addr s_addr6;
    };
    enum erl_ip_version ip_version; // is something like that defined somewhere? IPv4 would have to be under 0 to make it backwards compatible
  }
  ```

This would require modification of users existing code when the struct is used as input, since the ip_version field needs to be set. Also the ipv4 address would be accessed via s_addr.s_addr instead of s_addr. The following could be used and setting s_addr = 0 (invalid ipv4 address) to indicate that the ipv6 field is to be used.

struct erl_ipaddr {
    unsigned s_addr;
    unsigned char s6_addr[16];
};
* ei_connect_helper [uses](https://github.com/erlang/otp/blob/c41c3553d6978d1d2e909f826c0f33499e845265/lib/erl_interface/include/ei.h#L1230) `sockaddr_in`
  It could determine the IP version from `erl_ipaddr` struct and use either `sockaddr_in` or `sockaddr_in6`

It seems that ei_portio doesn't require any changes - it should work with sockaddr_in6 out of the box.

At least tcp_socket() needs to be fixed since it uses AF_INET in the call to socket(). It is probably easiest solved by implementing a tcp6_socket() function and set a another default callback structure for ipv6.

The ErlConnect structure also needs to be extended with an ipv6 address.

Note that I cannot give you a complete list of what needs to be done, since I do not have time to look through everything.

@rickard-green
Copy link
Contributor

@mat-hek The ip-address 0.0.0.0 will only work in the connect case since it is equals INADDR_ANY. RFC 5737 specifies address ranges that should only be used for documentation and examples, so an address in those ranges would work better. Adding the address in a define, for example named ERL_INVALID_IPV4_ADDR, would be useful. Another idea that came up when we discussed this internally was to allow the user to specify an "invalid" address him/herself in a call prior to using it in a struct that should contain an ipv6 address. Going with a fixed address in the ranges mentioned above should at least be good enough to start with.

@mat-hek
Copy link

mat-hek commented Aug 19, 2021

@rickard-green thanks for all the hints. Have you considered adding ei_use_ipv6(ei_cnode* ec) that would have to be called before ei_connect_xinit?

@rickard-green
Copy link
Contributor

rickard-green commented Aug 19, 2021

@mat-hek No, but I like it!

Even better would be int ei_address_family(int af, ei_cnode *ec) or similar that makes it possible to extend with support for even more address types in the future (returning an error if address family is not supported). Today Erl_IpAddr is typedef:ed as typedef struct in_addr *Erl_IpAddr; which can be changed to typedef void *Erl_IpAddr; and then simply casting it to a struct in6_addr * if AF_INET6 has been set as address family.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted Issue not worked on by OTP; help wanted from the community priority:low team:VM Assigned to OTP team VM
Projects
None yet
Development

No branches or pull requests

4 participants