Bug 3999 - Fix for net_qport always set to 0.
Status: RESOLVED FIXED
Alias:
Product: ioquake3
Classification: Unclassified
Component: Misc
Version: GIT MASTER
Hardware: PC All
: P3 normal
Assignee: Zachary J. Slater
QA Contact: ioquake3 bugzilla mailing list
URL:
Depends on:
Blocks:
 
Reported: 2009-03-01 20:48 EST by Kyle Monson
Modified: 2009-03-03 05:24:53 EST
0 users

See Also:


Attachments
Patch to fix net_qport bug. (1.32 KB, patch)
2009-03-02 00:04 EST, Kyle Monson

Description Kyle Monson 2009-03-01 20:48:25 EST
The randomization of net_qport is broken. This will fix it.

*** current ioquake3/code/qcommon/common.c      Sat Jan  3 13:06:28 2009
--- code/code/qcommon/common.c  Sun Mar  1 16:45:40 2009
*************** Com_Init
*** 2509,2514 ****
--- 2509,2515 ----
  */
  void Com_Init( char *commandLine ) {
        char    *s;
+       int             qport;
  
        Com_Printf( "%s %s %s\n", Q3_VERSION, PLATFORM_STRING, __DATE__ );
  
*************** void Com_Init( char *commandLine ) {
*** 2627,2633 ****
        com_version = Cvar_Get ("version", s, CVAR_ROM | CVAR_SERVERINFO );
  
        Sys_Init();
!       Netchan_Init( Com_Milliseconds() & 0xffff );    // pick a port value that should be nice and random
        VM_Init();
        SV_Init();
  
--- 2628,2639 ----
        com_version = Cvar_Get ("version", s, CVAR_ROM | CVAR_SERVERINFO );
  
        Sys_Init();
! 
!       // Pick a port value that should be nice and random.
!       // As machines get faster continuing to use Com_Milliseconds 
!       // results in a smaller and smaller range of qport values.
!       Com_RandomBytes( (byte*)&qport, sizeof(int) );
!       Netchan_Init( qport & 0xffff ); 
        VM_Init();
        SV_Init();

*** current ioquake3/code/sys/sys_main.c        Sat Jan  3 13:06:46 2009
--- code/code/sys/sys_main.c    Sun Mar  1 16:17:31 2009
*************** int main( int argc, char **argv )
*** 534,539 ****
--- 534,544 ----
  
        Sys_PlatformInit( );
  
+       // Set the initial time base.
+       // If not called here com_frameTime will always be zero.
+       // com_frameTime should be pseudo random.
+       Sys_Milliseconds();
+ 
        Sys_ParseArgs( argc, argv );
        Sys_SetBinaryPath( Sys_Dirname( argv[ 0 ] ) );
        Sys_SetDefaultInstallPath( DEFAULT_BASEDIR );
Comment 1 Kyle Monson 2009-03-02 00:04:16 EST
Created attachment 2001 [details]
Patch to fix net_qport bug.

OK, better description this time:

net_qport is the mechanism that quake 3 uses to distinguish between clients running on the same machine or behind a buggy nat. net_qport is supposed to be randomly generated on start up. Currently net_qport is always zero. 

Repro:

Start the game and pull down console.
/net_qport.

Actual:
"net_qport is "0" the default

Expected:
"net_qport is <some random number between 0 and 64k> the default"

The net result is that more than one client behind a buggy nat cannot connect to the same server.

The problem is that a call to Sys_Milliseconds in main was lost when the unix and win32 main functions were merged some time ago. This makes the call to Com_Milliseconds when generating net_qport the first call to Sys_Milliseconds resulting in a zero value every time. 

There is also a new problem with using Com_Milliseconds as systems get faster. The range of values generated using this method gets smaller and smaller. Using a debug build I saw values just over 1000 constantly. I elected to use Com_RandomBytes to generate a random int to use for Netchan_Init as this better makes use of the entire range of possible values.

Restoring the call to Sys_Milliseconds in main is still required. com_frameTime depends on Com_Milliseconds returning a non-zero value when it it called. Without the Sys_Milliseconds call in main we shift the problem to a different system.
Comment 2 Tim Angus 2009-03-02 13:02:34 EST
The patch you pasted inline looks weird, but the attached patch looks good. You could just leave the comments out though. Comments that describe past bugs are pretty redundant usually, and often confusing as the code bears no resemblance to what they're talking about.
Comment 3 Kyle Monson 2009-03-03 00:56:32 EST
Yeah, but it felt like the call to Sys_Milliseconds may have been dropped because it someone didn't know what it was really for. I wanted to make sure other coders knew that there was a good reason for it so it didn't get axed again. In this case server id's would not be random because com_frameTime would not be random.

I can see your point with the qport selection code.
Comment 4 Ludwig Nussel 2009-03-03 05:24:53 EST
fixed yesterday