Added IPv6 support for client by Gibus21250 · Pull Request #107 · BeamMP/BeamMP-Launcher

Gibus21250 · 2024-08-14T13:26:55Z

Hello, I worked on the client side to fully support IPv6 (and still IPv4 of course)
And I changed some mechanisms that don't break everything:

The DNS resolver can now return IPv4 or IPv6, and prefer IPv6 (as OS like Windows do)

I think I have tested everything:

joining a server by hostname who is listening on IPv6 (direct join)
joining a server by IPv6 (direct join)
Tested with vanilla and modded server, the download socket has been adapted

I have also added vcpkg, as beamng-server has, it is simpler for me to "clone and go" this project.

The server side has needed some modification, which I let you check it on the server repo! (ill add the PR link after)

I hope everything is good to go!

This DNS lookup prefer IPv6, and return an IPv4 as fallback. If the client doesnt have IPv6 compatibility, then the DNS probe gonna return IPv4 only

Gibus21250 · 2024-08-14T13:27:50Z

Re, the PR in link for IPv6 support, on the server side:
BeamMP/BeamMP-Server#359
Thanks!

lionkor

Generally great job, please see if you can address the comments I made.

Could you also explain to my what the rationale is behind adding so many comments and doing so many unrelated changes? I'm just curious.

lionkor · 2024-08-17T18:16:14Z

src/Network/Core.cpp

-        Terminate = true;
-        return;
+
+    const std::regex ipv4v6Pattern(R"(((^\h*((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]).){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\h*(|/([0-9]|[1-2][0-9]|3[0-2]))$)|(^\h*((([0-9a-f]{1,4}:){7}([0-9a-f]{1,4}|:))|(([0-9a-f]{1,4}:){6}(:[0-9a-f]{1,4}|((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9a-f]{1,4}:){5}(((:[0-9a-f]{1,4}){1,2})|:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9a-f]{1,4}:){4}(((:[0-9a-f]{1,4}){1,3})|((:[0-9a-f]{1,4})?:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9a-f]{1,4}:){3}(((:[0-9a-f]{1,4}){1,4})|((:[0-9a-f]{1,4}){0,2}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9a-f]{1,4}:){2}(((:[0-9a-f]{1,4}){1,5})|((:[0-9a-f]{1,4}){0,3}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9a-f]{1,4}:){1}(((:[0-9a-f]{1,4}){1,6})|((:[0-9a-f]{1,4}){0,4}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(:(((:[0-9a-f]{1,4}){1,7})|((:[0-9a-f]{1,4}){0,5}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:)))(%.+)?\h*(|/([0-9]|[0-9][0-9]|1[0-1][0-9]|12[0-8]))$)))");


Can we not just let the connect() / bind() syscalls handle the validation check of ipv6's? :)

lionkor · 2024-08-17T18:17:06Z

src/Network/Core.cpp

        Data = Code + HTTP::Get("https://backend.beammp.com/servers-info");
        break;
    case 'C':
+        //TODO StartSync Here


please either do or remove the todo, or create an issue

lionkor · 2024-08-17T18:18:12Z

src/Network/Core.cpp

-        info("Game Connected!");
+        info("Game Connected to LUA interface!");
        GameHandler(CSocket);
-        warn("Game Reconnecting...");
+        warn("Game reconnecting to LUA interface...");


I think this was fine, why the change?

lionkor · 2024-08-17T18:24:38Z

src/Network/Core.cpp

    LSocket = socket(res->ai_family, res->ai_socktype, res->ai_protocol);
    if (LSocket == -1) {
        debug("(Core) socket failed with error: " + std::to_string(WSAGetLastError()));
-        freeaddrinfo(res);


Is there a reason we don't need freeaddrinfo anymore?

lionkor · 2024-08-17T18:25:21Z

src/Network/DNS.cpp

+    //Loop all and return it by prefeence ipv6
+    for (struct addrinfo* ptr = addresses; ptr != nullptr; ptr = ptr->ai_next) {
+        char ipstr[INET6_ADDRSTRLEN] = { 0 };
+
+        if (ptr->ai_family == AF_INET6) {
+            struct sockaddr_in6* ipv6 = (struct sockaddr_in6*)ptr->ai_addr;
+            inet_ntop(AF_INET6, &(ipv6->sin6_addr), ipstr, sizeof(ipstr));
+            resolved = ipstr;
+            break; //Break if IPv6 finded
+        }
+        else if (ptr->ai_family == AF_INET) {
+            struct sockaddr_in* ipv4 = (struct sockaddr_in*)ptr->ai_addr;
+            inet_ntop(AF_INET, &(ipv4->sin_addr), ipstr, sizeof(ipstr));
+            resolved = ipstr;
+        }
+    }


Usually you just loop over resolved addresses, attempt to connect, and take the first one that connects. Is there a good reason why we don't just do that here?

lionkor · 2024-08-17T18:30:42Z

src/Network/VehicleData.cpp

+    if (AF == AF_INET) {
+        // IPv4
+        struct sockaddr_in serverAddrV4;
+        memset(&serverAddrV4, 0, sizeof(sockaddr_in));
+        serverAddrV4.sin_family = AF_INET;
+        serverAddrV4.sin_port = htons(Port);
+        inet_pton(AF_INET, IP.c_str(), &serverAddrV4.sin_addr);
+        memcpy(ToServer, &serverAddrV4, sizeof(sockaddr_in));
+        addrLen = sizeof(sockaddr_in);
+    } else {
+        // IPv6
+        struct sockaddr_in6 serverAddrV6;
+        memset(&serverAddrV6, 0, sizeof(sockaddr_in6));
+        serverAddrV6.sin6_family = AF_INET6;
+        serverAddrV6.sin6_port = htons(Port);
+        inet_pton(AF_INET6, IP.c_str(), &serverAddrV6.sin6_addr);
+        memcpy(ToServer, &serverAddrV6, sizeof(sockaddr_in6));
+        addrLen = sizeof(sockaddr_in6);
+    }


this is almost duplicated down in VehicleEvent, could we pull this out into a function maybe?

I'have tested the behavior waited

Based on Windows's documentation, the entire app need at least one WSAStarup call, other gonna juste increment a counter, for wich the app need the same number of WSACleanup. https://learn.microsoft.com/fr-fr/windows/win32/api/winsock/nf-winsock-wsastartup To simplify, and to increase a lot the visibility, the app will call one WSAStartup, and one WSACleanup at all. This is done on CoreNetwork function.

lionkor and others added 15 commits June 27, 2024 09:34

temporarily set dev to true always

7b733bf

update gitignore

a6e765a

added vcpkg auto download

fd0bef4

refractor DNS lookup

e033251

This DNS lookup prefer IPv6, and return an IPv4 as fallback. If the client doesnt have IPv6 compatibility, then the DNS probe gonna return IPv4 only

comments

41ed061

Added mecanism resolve if needed

6d0afdb

Added TCP client IPv6

3c0e56d

Added UDP Client support

1f5978b

lisibility

16458bb

Update .gitignore

739ef66

added vcpkg again

f819586

added download IPv4/IPv6

d563bfe

Update .gitmodules

f732cac

added submodule vcpkg

d0bbf87

header linux

376dc9a

Gibus21250 mentioned this pull request Aug 14, 2024

IPv6 support and added custom Ip config parameter BeamMP/BeamMP-Server#359

Closed

lionkor suggested changes Aug 17, 2024

View reviewed changes

Gibus21250 and others added 12 commits August 17, 2024 23:07

Let resolveHost check IP format and resolving if needed

6f75f9f

I'have tested the behavior waited

remove forgotten TODO

1965046

Correct freeaddrinfo

8f27dba

Merge branch 'BeamMP:master' into master

5bd42a1

Create function to create socket and fill sockaddrinfo struct

926e4e0

Merge branch 'master' of https://github.com/Gibus21250/BeamMP-Launcher

8fed6e0

Added INVALID_SOCKET macro for linux

4534e44

Update Core to use initSocket

7906953

make Ressoucre use initSocket

56d1d86

Added platform specific network error

9d984e9

Simplify initSocket

c87a6cd

Adapt new initSocket

13c14cc

Gibus21250 added 4 commits August 19, 2024 17:02

Use initSocket

b91298f

use INVALID_SOCKET macro for coherence

981e213

Linux small ajustement

625116d

Gibus21250 force-pushed the master branch from 4988bd2 to 625116d Compare August 19, 2024 16:44

lionkor marked this pull request as draft October 4, 2024 21:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added IPv6 support for client#107

Added IPv6 support for client#107
Gibus21250 wants to merge 31 commits intoBeamMP:masterfrom
Gibus21250:master

Gibus21250 commented Aug 14, 2024

Uh oh!

Gibus21250 commented Aug 14, 2024

Uh oh!

lionkor left a comment

Uh oh!

lionkor Aug 17, 2024

Uh oh!

lionkor Aug 17, 2024

Uh oh!

lionkor Aug 17, 2024

Uh oh!

lionkor Aug 17, 2024

Uh oh!

lionkor Aug 17, 2024

Uh oh!

lionkor Aug 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Gibus21250 commented Aug 14, 2024

Uh oh!

Gibus21250 commented Aug 14, 2024

Uh oh!

lionkor left a comment

Choose a reason for hiding this comment

Uh oh!

lionkor Aug 17, 2024

Choose a reason for hiding this comment

Uh oh!

lionkor Aug 17, 2024

Choose a reason for hiding this comment

Uh oh!

lionkor Aug 17, 2024

Choose a reason for hiding this comment

Uh oh!

lionkor Aug 17, 2024

Choose a reason for hiding this comment

Uh oh!

lionkor Aug 17, 2024

Choose a reason for hiding this comment

Uh oh!

lionkor Aug 17, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants