From a7e18bf7851624e7770466adb79db3aa6dcaf058 Mon Sep 17 00:00:00 2001 From: slipher Date: Fri, 13 Dec 2024 20:04:54 -0600 Subject: [PATCH 1/5] NET_OpenIP: fix using wrong port num --- src/engine/qcommon/net_ip.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/engine/qcommon/net_ip.cpp b/src/engine/qcommon/net_ip.cpp index 125f2e1366..f9ba0a360d 100644 --- a/src/engine/qcommon/net_ip.cpp +++ b/src/engine/qcommon/net_ip.cpp @@ -1630,7 +1630,7 @@ static void NET_OpenIP() if ( net_enabled->integer & NET_ENABLEV4 ) { - for ( i = ( port6 == PORT_ANY ? 1 : MAX_TRY_PORTS ); i; i-- ) + for ( i = ( port == PORT_ANY ? 1 : MAX_TRY_PORTS ); i; i-- ) { ip_socket = NET_IPSocket( net_ip->string, port, &boundto4, &err ); From 413ef577537c5d599a1a2d118fd6035dbc747b2a Mon Sep 17 00:00:00 2001 From: slipher Date: Fri, 13 Dec 2024 21:10:24 -0600 Subject: [PATCH 2/5] Fix /net_restart on Windows with running server It was producing errors about the port being already in use, and thus switching to port 27961 instead of 27960. This was caused by the socket being inherited by NaCl processes. --- src/engine/qcommon/net_ip.cpp | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/engine/qcommon/net_ip.cpp b/src/engine/qcommon/net_ip.cpp index f9ba0a360d..5ed12e70b6 100644 --- a/src/engine/qcommon/net_ip.cpp +++ b/src/engine/qcommon/net_ip.cpp @@ -935,7 +935,13 @@ SOCKET NET_IPSocket( const char *net_interface, int port, struct sockaddr_in *bi Log::Notice( "Opening IP socket: %s:%s", net_interface ? net_interface : "0.0.0.0", port ? va( "%i", port ) : "*" ); - if ( ( newsocket = socket( PF_INET, SOCK_DGRAM, IPPROTO_UDP ) ) == INVALID_SOCKET ) +#ifdef _WIN32 + newsocket = WSASocketW( PF_INET, SOCK_DGRAM, IPPROTO_UDP, nullptr, 0, WSA_FLAG_NO_HANDLE_INHERIT ); +#else + newsocket = socket( PF_INET, SOCK_DGRAM, IPPROTO_UDP ); +#endif + + if ( newsocket == INVALID_SOCKET ) { *err = socketError; Log::Warn( "NET_IPSocket: socket: %s", NET_ErrorString() ); @@ -1020,7 +1026,13 @@ SOCKET NET_IP6Socket( const char *net_interface, int port, struct sockaddr_in6 * // Print the name in brackets if there is a colon: Log::Notice( "Opening IP6 socket: %s%s%s:%s", brackets ? "[" : "", net_interface ? net_interface : "[::]", brackets ? "]" : "", port ? va( "%i", port ) : "*" ); - if ( ( newsocket = socket( PF_INET6, SOCK_DGRAM, IPPROTO_UDP ) ) == INVALID_SOCKET ) +#ifdef _WIN32 + newsocket = WSASocketW( PF_INET6, SOCK_DGRAM, IPPROTO_UDP, nullptr, 0, WSA_FLAG_NO_HANDLE_INHERIT ); +#else + newsocket = socket( PF_INET6, SOCK_DGRAM, IPPROTO_UDP ); +#endif + + if ( newsocket == INVALID_SOCKET ) { *err = socketError; Log::Warn( "NET_IP6Socket: socket: %s", NET_ErrorString() ); @@ -1223,7 +1235,13 @@ void NET_OpenSocks( int port ) Log::Notice( "Opening connection to SOCKS server." ); - if ( ( socks_socket = socket( AF_INET, SOCK_STREAM, IPPROTO_TCP ) ) == INVALID_SOCKET ) +#ifdef _WIN32 + socks_socket = WSASocketW( AF_INET, SOCK_STREAM, IPPROTO_TCP, nullptr, 0, WSA_FLAG_NO_HANDLE_INHERIT ); +#else + socks_socket = socket( AF_INET, SOCK_STREAM, IPPROTO_TCP ); +#endif + + if ( socks_socket == INVALID_SOCKET ) { Log::Warn( "NET_OpenSocks: socket: %s", NET_ErrorString() ); return; From 52835b9dec530bc3375b21aba38e0e98f9537a5f Mon Sep 17 00:00:00 2001 From: slipher Date: Fri, 29 Nov 2024 00:46:14 -0600 Subject: [PATCH 3/5] Refactor NET_Config; drop cvar modified checks NET_Config(bool enableNetworking) was a function to either open the network sockets if the arg is true, or close them if it is false. It checked the modified flag of a bunch of net_ cvars, with the idea that if it is requested to enable networking while it was already enabled, to not do anything unless the configuration is changed. But actually this situation can never happen, so we can drop the modify checks and always reinitialize. NET_Config( true ) was only done in 4 places, none of which could possibly omit the network (re-)initialization: - NET_Init - only called once on startup - NET_Restart - the point of /net_restart is to force reinitialization - SV_Startup - this is only done in clients and starting the server is a state change for clients so restart can't be skipped - SV_Shutdown - this is only done in clients and stopping the server is a state change for clients so restart can't be skipped. Also split NET_Config into two functions NET_EnableNetworking and NET_DisableNetworking for better readability. --- src/engine/qcommon/net_ip.cpp | 157 ++++++++++------------------------ src/engine/qcommon/qcommon.h | 3 +- src/engine/server/sv_init.cpp | 4 +- 3 files changed, 49 insertions(+), 115 deletions(-) diff --git a/src/engine/qcommon/net_ip.cpp b/src/engine/qcommon/net_ip.cpp index 5ed12e70b6..e7637bb1bf 100644 --- a/src/engine/qcommon/net_ip.cpp +++ b/src/engine/qcommon/net_ip.cpp @@ -35,6 +35,7 @@ Maryland 20850 USA. #include "qcommon/q_shared.h" #include "qcommon/qcommon.h" #include +#include "engine/framework/Application.h" #include "engine/framework/Network.h" #include "server/server.h" @@ -120,11 +121,6 @@ namespace net { static bool usingSocks = false; static bool networkingEnabled = false; -#ifndef BUILD_SERVER -static bool serverMode = false; -#else -static const bool serverMode = true; -#endif cvar_t *net_enabled; @@ -1598,7 +1594,7 @@ static int NET_EnsureValidPortNo( int port ) NET_OpenIP ==================== */ -static void NET_OpenIP() +static void NET_OpenIP( bool serverMode ) { int i; int err = 0; @@ -1689,10 +1685,8 @@ static void NET_OpenIP() NET_GetCvars ==================== */ -static bool NET_GetCvars() +static void NET_GetCvars() { - int modified; - #ifdef BUILD_SERVER // I want server owners to explicitly turn on IPv6 support. net_enabled = Cvar_Get( "net_enabled", "1", CVAR_LATCH ); @@ -1702,147 +1696,82 @@ static bool NET_GetCvars() * used if available due to ping */ net_enabled = Cvar_Get( "net_enabled", "3", CVAR_LATCH ); #endif - modified = net_enabled->modified; - net_enabled->modified = false; net_ip = Cvar_Get( "net_ip", "0.0.0.0", CVAR_LATCH ); - modified += net_ip->modified; - net_ip->modified = false; - net_ip6 = Cvar_Get( "net_ip6", "::", CVAR_LATCH ); - modified += net_ip6->modified; - net_ip6->modified = false; - net_port = Cvar_Get( "net_port", va( "%i", PORT_SERVER ), CVAR_LATCH ); - modified += net_port->modified; - net_port->modified = false; - net_port6 = Cvar_Get( "net_port6", va( "%i", PORT_SERVER ), CVAR_LATCH ); - modified += net_port6->modified; - net_port6->modified = false; // Some cvars for configuring multicast options which facilitates scanning for servers on local subnets. net_mcast6addr = Cvar_Get( "net_mcast6addr", NET_MULTICAST_IP6, CVAR_LATCH ); - modified += net_mcast6addr->modified; - net_mcast6addr->modified = false; - #ifdef _WIN32 net_mcast6iface = Cvar_Get( "net_mcast6iface", "0", CVAR_LATCH ); #else net_mcast6iface = Cvar_Get( "net_mcast6iface", "", CVAR_LATCH ); #endif - modified += net_mcast6iface->modified; - net_mcast6iface->modified = false; net_socksEnabled = Cvar_Get( "net_socksEnabled", "0", CVAR_LATCH ); - modified += net_socksEnabled->modified; - net_socksEnabled->modified = false; - net_socksServer = Cvar_Get( "net_socksServer", "", CVAR_LATCH ); - modified += net_socksServer->modified; - net_socksServer->modified = false; - net_socksPort = Cvar_Get( "net_socksPort", "1080", CVAR_LATCH ); - modified += net_socksPort->modified; - net_socksPort->modified = false; - net_socksUsername = Cvar_Get( "net_socksUsername", "", CVAR_LATCH ); - modified += net_socksUsername->modified; - net_socksUsername->modified = false; - net_socksPassword = Cvar_Get( "net_socksPassword", "", CVAR_LATCH ); - modified += net_socksPassword->modified; - net_socksPassword->modified = false; - - return modified ? true : false; } -/* -==================== -NET_Config -==================== -*/ -void NET_Config( bool enableNetworking ) +void NET_EnableNetworking( bool serverMode ) { - bool modified; - bool stop; - bool start; -#ifndef BUILD_SERVER - bool svRunning; -#endif - // get any latched changes to cvars - modified = NET_GetCvars(); -#ifndef BUILD_SERVER - svRunning = !!com_sv_running->integer; - modified |= ( svRunning != serverMode ); -#endif + NET_GetCvars(); - if ( !net_enabled->integer ) - { - enableNetworking = false; - } + // always cycle off and on because this function is only called on a state change or forced restart + NET_DisableNetworking(); - // if enable state is the same and no cvars were modified, we have nothing to do - if ( enableNetworking == networkingEnabled && !modified ) + if ( !( net_enabled->integer & ( NET_ENABLEV4 | NET_ENABLEV6 ) ) ) { return; } - start = enableNetworking; - if ( enableNetworking == networkingEnabled ) - { - stop = enableNetworking; - } - else + networkingEnabled = true; + + NET_OpenIP( serverMode ); + NET_SetMulticast6(); + SV_NET_Config(); +} + +void NET_DisableNetworking() +{ + if ( !networkingEnabled ) { - stop = !enableNetworking; + return; } -#ifndef BUILD_SERVER - serverMode = svRunning; -#endif - networkingEnabled = enableNetworking; + networkingEnabled = false; - if ( stop ) + if ( ip_socket != INVALID_SOCKET ) { - if ( ip_socket != INVALID_SOCKET ) - { - closesocket( ip_socket ); - ip_socket = INVALID_SOCKET; - } + closesocket( ip_socket ); + ip_socket = INVALID_SOCKET; + } - if ( multicast6_socket != INVALID_SOCKET ) + if ( multicast6_socket != INVALID_SOCKET ) + { + if ( multicast6_socket != ip6_socket ) { - if ( multicast6_socket != ip6_socket ) - { - closesocket( multicast6_socket ); - } - - multicast6_socket = INVALID_SOCKET; + closesocket( multicast6_socket ); } - if ( ip6_socket != INVALID_SOCKET ) - { - closesocket( ip6_socket ); - ip6_socket = INVALID_SOCKET; - } + multicast6_socket = INVALID_SOCKET; + } - if ( socks_socket != INVALID_SOCKET ) - { - closesocket( socks_socket ); - socks_socket = INVALID_SOCKET; - } + if ( ip6_socket != INVALID_SOCKET ) + { + closesocket( ip6_socket ); + ip6_socket = INVALID_SOCKET; } - if ( start ) + if ( socks_socket != INVALID_SOCKET ) { - if ( net_enabled->integer ) - { - NET_OpenIP(); - NET_SetMulticast6(); - SV_NET_Config(); - } + closesocket( socks_socket ); + socks_socket = INVALID_SOCKET; } } @@ -1868,7 +1797,7 @@ void NET_Init() Log::Notice( "Winsock Initialized" ); #endif - NET_Config( true ); + NET_EnableNetworking( Application::GetTraits().isServer ); Cmd_AddCommand( "net_restart", NET_Restart_f ); } @@ -1885,7 +1814,7 @@ void NET_Shutdown() return; } - NET_Config( false ); + NET_DisableNetworking(); #ifdef _WIN32 WSACleanup(); @@ -1948,8 +1877,12 @@ NET_Restart_f */ void NET_Restart_f() { - NET_Config( false ); + NET_DisableNetworking(); SV_NET_Config(); Net::ShutDownDNS(); - NET_Config( true ); +#ifdef BUILD_SERVER + NET_EnableNetworking( true ); +#else + NET_EnableNetworking( !!com_sv_running->integer ); +#endif } diff --git a/src/engine/qcommon/qcommon.h b/src/engine/qcommon/qcommon.h index 1dbdf3e0e7..dc01323ef2 100644 --- a/src/engine/qcommon/qcommon.h +++ b/src/engine/qcommon/qcommon.h @@ -149,7 +149,8 @@ extern cvar_t *net_enabled; void NET_Init(); void NET_Shutdown(); void NET_Restart_f(); -void NET_Config( bool enableNetworking ); +void NET_EnableNetworking( bool serverMode ); +void NET_DisableNetworking(); void NET_SendPacket( netsrc_t sock, int length, const void *data, const netadr_t& to ); diff --git a/src/engine/server/sv_init.cpp b/src/engine/server/sv_init.cpp index 9cff95dfa0..8a3c55510b 100644 --- a/src/engine/server/sv_init.cpp +++ b/src/engine/server/sv_init.cpp @@ -345,7 +345,7 @@ void SV_Startup() Cvar_Set( "sv_running", "1" ); #ifndef BUILD_SERVER - NET_Config( true ); + NET_EnableNetworking( true ); #endif // Join the IPv6 multicast group now that a map is running, so clients can scan for us on the local network. @@ -767,7 +767,7 @@ void SV_Shutdown( const char *finalmsg ) Cvar_Set( "sv_running", "0" ); #ifndef BUILD_SERVER - NET_Config( true ); + NET_EnableNetworking( false ); #endif SV_NET_Config(); // clear master server DNS queries From 077a8bbb2102707bd1e326e896e58ab54aa837f6 Mon Sep 17 00:00:00 2001 From: slipher Date: Fri, 13 Dec 2024 21:16:04 -0600 Subject: [PATCH 4/5] NET_Shutdown: fix bogus early return --- src/engine/qcommon/net_ip.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/engine/qcommon/net_ip.cpp b/src/engine/qcommon/net_ip.cpp index e7637bb1bf..0b26f1c920 100644 --- a/src/engine/qcommon/net_ip.cpp +++ b/src/engine/qcommon/net_ip.cpp @@ -1809,16 +1809,14 @@ NET_Shutdown */ void NET_Shutdown() { - if ( !networkingEnabled ) - { - return; - } - NET_DisableNetworking(); #ifdef _WIN32 - WSACleanup(); - winsockInitialized = false; + if ( winsockInitialized ) + { + WSACleanup(); + winsockInitialized = false; + } #endif } From fbb5dd57924e47b912a8b79cc3f46654951131a7 Mon Sep 17 00:00:00 2001 From: slipher Date: Wed, 22 Jan 2025 09:11:00 -0600 Subject: [PATCH 5/5] Comment on confusing NET_EnableNetworking calls --- src/engine/server/sv_init.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/engine/server/sv_init.cpp b/src/engine/server/sv_init.cpp index 8a3c55510b..306b5e24d5 100644 --- a/src/engine/server/sv_init.cpp +++ b/src/engine/server/sv_init.cpp @@ -345,6 +345,7 @@ void SV_Startup() Cvar_Set( "sv_running", "1" ); #ifndef BUILD_SERVER + // For clients, reconfigure to open server ports. NET_EnableNetworking( true ); #endif @@ -767,6 +768,7 @@ void SV_Shutdown( const char *finalmsg ) Cvar_Set( "sv_running", "0" ); #ifndef BUILD_SERVER + // For clients, reconfigure to close server ports. NET_EnableNetworking( false ); #endif