Skip to content

IPv6 support and added custom Ip config parameter#359

Closed
Gibus21250 wants to merge 8 commits intoBeamMP:minorfrom
Gibus21250:minor
Closed

IPv6 support and added custom Ip config parameter#359
Gibus21250 wants to merge 8 commits intoBeamMP:minorfrom
Gibus21250:minor

Conversation

@Gibus21250
Copy link

Hello, I make small adjustments on the server side to support IPv6 (in reference to my works on the client side to support IPv6 too)
I added the parameter "IP" on the config file, now we can set the IP of the BeamMP to listen on.
The server sends this param in the post produced by the heartbeat.

The PR in link for IPv6 support, on the client side:
BeamMP/BeamMP-Launcher#107
I hope everything is good to go!

Gibus21250 and others added 7 commits July 25, 2024 20:07
Added IPv6 support, added a new arg for the server's config to handle custom IP to listen to
When shutting down the server, invalide move on an invalide socket cause crash
Now the server post the IP to client (as backend beammp) on his heartbeat.
@lionkor
Copy link
Contributor

lionkor commented Aug 17, 2024

IPv6 already implemented in #349, is this potentially the same feature?

Copy link
Contributor

@lionkor lionkor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for contributing! :)

As I said we already are adding IPv6 support to the Server in the linked PR.

There are a lot of unrelated changes in here, too. Could you please remove all changes that are not specifically related to the custom IP config parameter (which would be a nice addition)? :)

@Gibus21250
Copy link
Author

Hi, thanks for contributing! :)

As I said we already are adding IPv6 support to the Server in the linked PR.

There are a lot of unrelated changes in here, too. Could you please remove all changes that are not specifically related to the custom IP config parameter (which would be a nice addition)? :)

I have checked on the client repo only if IPv6 was wip, but it doesnt, and assume here wasn't too and was confirmed when looking in the code on master branch.

My approach:

The features coming with custom IPv6 come with the fact to be able to modify it in the config file, I can't remove the IPv6 logic, without rewrite a lot, for nothing because i'll need to rewrite again for the PR pinged ?
So now administrator can make BeamMP listen to an specific IPv4 or IPv6.

The PR pinged:

The server is listening to all IPv4 and IPv6 of all interfaces.

Arguments

As a system can now have multiple IPv6 globaly rootable (or even multiple IPv4), administrator probably want to map the BeamMP server on a specific globaly rootable IPv6, or even a specific IPv4 ?

I think we need to give the posibility to choose on what IP listening, and maybe on multiple IPs ?
So it's more natural to specify an IPv4, or an IPv6 on the config file that the server gonna listen on, rather than bind an IPv4, and a boolean to listen on ALL IPv6, without the choice on which ?

If BeamMP really need the ability to handle IPv4 and IPv6 listening at the same time, we need to double each endpoint, wich imply a little more of work but would provide precise control.

Recap

Give the possibility to specify an IPv6 (globaly rootable or local-link IPv6, even), which give much more flexibility for people.
To "merge" our two approche, i can set the default value of "IP" on "::" with IPv6_ONLY = false on the config system ?
So by default, the server is listening on all IPv4 and IPv6. If a specific address is set, the server would bind to that address only, overriding the default behavior, and that's it!

What do you think of this approach ?

@lionkor
Copy link
Contributor

lionkor commented Aug 31, 2024

I think it makes little sense to implement IPv6 twice. Please reduce your PR to only the new features you're adding above the IPv6.

@lionkor lionkor marked this pull request as draft August 31, 2024 18:33
@Gibus21250
Copy link
Author

I think it makes little sense to implement IPv6 twice. Please reduce your PR to only the new features you're adding above the IPv6.

This feature add the possibility to modify the IP. The implementation provided for Ipv6 is a small "good side effect".

This needed to modify all the socket creation, so of course that the little modification that has been made to "implement IPv6" need to be overwritten at all. This is not a double implementation, but an implementation as simple it can be.

This implementation is flexible, as the other isn't.
Now there is a little conflict that you can easly resolve!

@WiserTixx WiserTixx closed this Nov 23, 2024
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.

3 participants