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

IPv6 support! #109

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

IPv6 support! #109

wants to merge 15 commits into from

Conversation

SirLynix
Copy link

@SirLynix SirLynix commented May 22, 2019

Edit: since this PR (and other) will never be merged and ENet development is basically dead, I made my own fork of enet called enet6. See this message.

So yeah, I was thinking it would be nice to make a tradition out of a new IPv6 pull request every two years.

Long story short: I'm using ENet for a mobile game and we had to use IPv6 sockets because of Apple IPv6 policy. None of the previous IPv6 PR seemed to fit my needs and so here's mine. Has been tested on Windows, Linux, Android and iOS.

So basically this supports IPv4-only, IPv6-only and "any" (dual-stack) host/sockets, allowing servers to accept both IPv4 and IPv6 clients. It is able to decode IPv4 and IPv6 without using system functions (didn't want to bother with outdated functions and such) and will automatically convert resolved IPv4 addresses to IPv4-mapped IPv6 if required.
And of course it's a breaking change, I don't think it is possible to add IPv6 support to ENet without breaking API and/or ABI.

We made other changes to ENet (mostly to add support for encryption) and I tried to have only the IPv6-related stuff here. I didn't check if it compiles as is (it should, or be really near) and I didn't bother to update the doc, as this PR will probably never be merged anyway.

Edit: None of the protocol part has been touched, which means a program using ENet without this PR is able to connect without any issue to a program using ENet with this PR if the host has been created with either "IPv4" or "any" protocol (obviously this wouldn't work if the host were created using IPv6-only socket).

Feel free to reuse this code or to tell me how to improve it.

@lsalzman
Copy link
Owner

lsalzman commented Jun 2, 2019

I apologize, but I am really horribly busy with other obligations at the moment. When I am able, I will take a deeper look at this and what to do about it, but at first glance it appears well done.

include/enet/enet.h Outdated Show resolved Hide resolved
win32.c Outdated Show resolved Hide resolved
win32.c Outdated Show resolved Hide resolved
win32.c Outdated Show resolved Hide resolved
Jérôme Leclercq added 2 commits June 3, 2019 09:13
Those lines don't belong in this PR
@SirLynix
Copy link
Author

SirLynix commented Jun 8, 2019

Oops, I though I fixed all of those VLA. I don't think they are necessary, could you comment the line where they are so I can fix them?

@nxrighthere
Copy link

Nah, everything is okay I was a bit sleepy. 👍

@mindrage
Copy link

@DrLynix Hey there, nice work sofar ;3,
I tried compiling the latest one on MSVC 2017 and found some problems:

3 places have comparison operators directly towards a union type, which msvc apparently doesn't allow.
protocol.c:310 currentPeer -> address.host == host -> receivedAddress.host
protocol.c:1022 host -> receivedAddress.host != peer -> address.host
protocol.c:1024 peer -> address.host != ENET_HOST_BROADCAST

recievedAddress.host is the union of v4 and v6 addresses, might need to compare to the contents somehow.

ENET_HOST_BROADCAST is not defined as well, couldn't be found from a solution based.

Project was generated with cmake -G "Visual Studio 15 2017 Win64" then build via the solution.

@SirLynix
Copy link
Author

@mindrage Thank you for letting me know, looks like I forgot to commit my protocol.c changes. Here you are.

protocol.c Outdated Show resolved Hide resolved
@PlasmaMichael
Copy link

Hi everybody!
Any news on IPv6?
Thanks!

@chewi
Copy link

chewi commented Mar 28, 2020

Frustratingly for distro maintainers, SuperTuxKart allows you to use the system enet rather than their bundled version, unless you want IPv6 support because only their bundled version has that.

@Benau
Copy link

Benau commented Apr 2, 2020

Btw you probably need to handle scope id to allow fe80:: connection

@cztk
Copy link

cztk commented Jun 12, 2021

cztk@9bb8463 merged
cztk@c114bcd fixed cmake build
hope to test things anytime soon but for now just this

@KTRosenberg
Copy link

KTRosenberg commented Dec 30, 2021

Might there be an update on this? It would be great to have support for ipv6-only networks + mixed ipv6/ipv4 clients.

@SirLynix
Copy link
Author

I resolved two conflicts in win32.c but can't do much more at this point.

@KTRosenberg
Copy link

I resolved two conflicts in win32.c but can't do much more at this point.

I see. I also have a similar use case--an iOS application. Can your ipv6 branch (on your fork) connect to both ipv6 and ipv4 clients by default? Thanks.

@SirLynix
Copy link
Author

Sure @KTRosenberg, if you use dual stack host (any) then IPv4 clients (regular enet) and IPv6 clients (my fork) will be able to connect to it.
Also, my fork on iOS should be able to connect to a regular enet host without issue, that's what we were doing in my company back then.

@KTRosenberg
Copy link

Sure @KTRosenberg, if you use dual stack host (any) then IPv4 clients (regular enet) and IPv6 clients (my fork) will be able to connect to it.
Also, my fork on iOS should be able to connect to a regular enet host without issue, that's what we were doing in my company back then.

I probably lack experience with this topic, but is there not a setting in which the same socket can receive both ipv4 and ipv6? Or are you recommending that I have two servers running at the same time, and clients choose to connect via ipv4 or ipv6? I am not sure if the user should have the burden of choosing, so maybe I misunderstood something.

@SirLynix
Copy link
Author

You are right, there is a socket setting that allows an IPv6 socket to accept IPv4 clients, this is what we call "dual stack" and this is what my fork does when you create an host with "any" address type.

@KTRosenberg
Copy link

You are right, there is a socket setting that allows an IPv6 socket to accept IPv4 clients, this is what we call "dual stack" and this is what my fork does when you create an host with "any" address type.

Ah ok. One last question would be — in your fork, how is that address type specified? Is there an enum and config struct that handles this? Thanks for your patience.

@SirLynix
Copy link
Author

Ah ok. One last question would be — in your fork, how is that address type specified? Is there an enum and config struct that handles this? Thanks for your patience.

Yes there is an enum that you can pass as the first parameter of enet_host_create, like this:

ENetAddress address;
ENetHost * server;

enet_address_build_any(&address, ENET_ADDRESS_TYPE_IPV6);
address.port = 1234;

server = enet_host_create (ENET_ADDRESS_TYPE_ANY, /* has to match address.type, except if ENET_ADDRESS_TYPE_ANY which means dual-stack*/
                             & address /* the address to bind the server host to */, 
                             32      /* allow up to 32 clients and/or outgoing connections */,
                              2      /* allow up to 2 channels to be used, 0 and 1 */,
                              0      /* assume any amount of incoming bandwidth */,
                              0      /* assume any amount of outgoing bandwidth */);

Everything else is pretty much the same

@nyabinary
Copy link

Any news?

@nyabinary
Copy link

(lmao I'm just casually waiting for when this happens so I can finally stop using the builtin enet in Godot)

@fuck-shithub
Copy link

We made other changes to ENet (mostly to add support for encryption)

Can you please share this? I'm interested in having encryption in enet too.

@SirLynix
Copy link
Author

SirLynix commented Jan 13, 2023

Can you please share this? I'm interested in having encryption in enet too.

Sorry for the late reply, basically we implemented a "compressor" encrypting using ChaCha20 (with libsodium), we had to change enet a little bit to allow compressor to output data larger than their input (otherwise enet ignore them, which is quite normal for real compressor) and pass the ENetPeer pointer as a parameter (to be able to retrieve their encryption key).

I don't recall all the details as I'm no longer working there (this PR is quite a bit old).

Btw I resolved the conflicts, if one day you consider merging this PR @lsalzman (I understand it breaks the API and you cannot release it as a minor version, maybe consider to make a major version? people would love enet 2.0 with IPv6 support I think)

@Leseratte10 Leseratte10 mentioned this pull request Mar 16, 2023
address.c Outdated Show resolved Hide resolved
@SirLynix
Copy link
Author

SirLynix commented Oct 3, 2023

Hi folks.

It pains me to have to do this, but it's a shame that a library like this doesn't support IPv6 in 2023, despite four pull requests opened for ten years.

I can understand why you (@lsalzman) don't want to merge any of those PR like this since it breaks API or may implement this in a way you dislike. But I think it's pretty clear people want a major version increase to support IPv6 (since this cannot be done without breaking the API/ABI).

I can understand you lack time to consider this, but you have a lot of people ready to do whatever you want in order to improve ENet. And I really though it would happen. And then I read #78 where it looks like you don't want people to contribute on GitHub, or to make the efforts to tell people how to do it.

So I think it's time to fork my way out and try to improve ENet on my side.

I merged this pull request into my own enet fork which I called "enet6", I also added a timed out event and I'm considering adding an encryption layer/callback (edit: done). If people want to make pull request be sure they will be considered and not left to rot for years.

I also updated the docs and added updated code examples.

You may wonder why I bother so much: I'm a game networking teacher. I teach people how to bring multiplayer to their games, and I've been teaching them how to use ENet for years, and I can no longer do this while teaching them it's important to support IPv6.

I've been trying to use the amazing ENet-CSharp (which at this point is basically ENet 2) by @nxrighthere but unfortunately it only uses IPv6 sockets which may not be supported on some networks/computers (I had students failing to connect using ENet-CSharp this week but successfully connect using enet). My IPv6 implementation is more intrusive (it changes the API) but it's able to work with IPv4 sockets (and will resolve a domain to an IPv4 on computers not supporting IPv6).

So, enet6 is available on every platform (you can find binaries for windows/mingw/linux/macos/ios/android here: https://github.com/SirLynix/enet6/releases) and if you're using the amazing XMake build system, you can use it in your project by simply

add_rules("mode.debug", "mode.release")
add_requires("enet6")

target("my_project")
    add_files("src/main.c")
    add_packages("enet6")

with this XMake will download and compile enet6 on your computer and configure the compiler to have the right include dirs/link dirs/links so you can use it out of the box.

Final words: I'm sorry to have to do this. If one day ENet 2 happens with IPv6 support (and hopefully other exciting changes) I would be more than happy to delete my fork.

…TYPE_ANY

In v6.0.0 this function could only output IPv4 addresses when using ENET_ADDRESS_TYPE_IPV4. If ENET_ADDRESS_TYPE_ANY were to be used IPv4 were converted to IPv6.

This behavior was changed/fixed so that :
- ENET_ADDRESS_TYPE_IPV4: Returns IPv4
- ENET_ADDRESS_TYPE_IPV6: Returns an IPv6 or a IPv4-mapped IPv6 if no IPv6 is known
- ENET_ADDRESS_TYPE_ANY: Returns IPv4 or IPv6 (priorize IPv6s)
IPV6_V6ONLY has to be set before binding the socket
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.