[Freeciv-Dev] server i/o patch

[Lauri Tarkkala <ltarkkal@xxxxxxxxxxxxxxx>]

Having played freeciv on civserver.freeciv.org, a couple of times 
the server started lagging unbearably for some short intervals, yet 
the latency for e.g. a ping was almost constant for the whole time.

After this, I decided to take a look at the freeciv networking code.
IMHO there a couple of minor issues with the code in question,
briefly:

- It basically uses blocking I/O for writing to sockets on the
  server side. Even if the socket is in nonblocking mode, the
  common/packets.c:write_socket_data() function loops untill it
  has written all data. 

  This allows one badly lagged player to make the game
  unplayable for everyone else, due to the flow control of
  TCP.

- The freeciv server performs user-level buffering, via
  connection_do_unbuffer() and connetion_do_buffer() functions
  to send bigger blocks of data to a socket at once. This
  functionality is basically the same as that disabled by the
  TCP_NODELAY TCP protocol socket option, which allows the
  freeciv "packets" to be transmitted with less latency. A quick
  experiment made me think this "helped a little bit", when
  moving units around.

- TCP connections timeout far too slowly, for them to be of
  any use in an "almost-real-time" game like freeciv.

I took a brief look at the networking code of freeciv 1.11.4, and
came up with the following patch against it. The patch does
the following modifications.

- The server is modified to use nonblocking I/O for writing
  to sockets when NONBLOCKING_SOCKETS is available. This means
  that write_socket_data() will write as much data as is possible
  to a socket without blocking, and leave the rest in the
  connections[i].send_buffer.

- It adds two new options "tcptimeout" and "netwait" in game.h
  and stdinhand.h.

- I renamed sniff_packets() to io_handler(), and cleaned it
  up slightly (so I could understand what it was doing ;-)
  into three functions io_handler(), io_console_input()
  and io_console_init(), basically taking the console input
  crap into separate function. It isn't clean or perfect, but 
  it is (only) slightly more readable...

  io_handler() tests for the possibility of flushing the
  buffers filled by write_socket_data(), in addition to 
  the stuff done by sniff_packets()

  io_handler() also takes as input a mode parameter, which
  may have either a value of 1 or 2. io_handler(1) functions
  almost like the previous sniff_packets(), io_handler(2)
  does not read any input from the client connections, but
  spends at most "netwait" seconds flushing the socket
  output buffers. Server console input is still accepted.

  io_handler() also tests if a TCP connection has been
  blocking for more than "tcptimeout" seconds without 
  any data being written to it, and if this is so, it
  then closes that client connection.

  The "lastwrite" field was added to the "connection" structure
  for keeping track of this. Initialization of this field
  was added to sernet.c and clinet.c

- I added a function "my_nodelay(int sockfd) into netintf.c,
  which sets the TCP_NODELAY flag if available.
  This is called from client/clinet.c:connect_to_server() and
  server/sernet.c:server_accept_connection(), when creating sockets
  for client<->server I/O.

- configure.in was modified to test for the precense of netinet/tcp.h 
  header file (contains definition of TCP_NODELAY)

- The write_socket_data() function is written so, that it should
  work without buffering if the socket is in blocking mode (as it is
  in the client), and it should still work if there are no NONBLOCKING
  sockets available..

- civserver.c was modified to call io_handler(1) and io_handler(2)
  instead of sniff_packets().

- civserver.c was modified to include the TCP port the server is
  running on in the greeting message (I tend to forget which civserver
  I have logged onto ;-)

The netwait parameter is probably not very useful, but it might
be an interesting experiment. The tcptimeout parameter is probably
more useful, but determining this would require some experimentation. 

The usefullness of both parameters may be increased by tuning the TCP 
buffering parameters to better suit a freeciv like application. Using
the socket high and low watermarks might also be a useful experiment.

The patch is attached as a text file. I tried not to break anything,
and tried not to do any unnecessary modifications. I hope it
could receive some testing. Any and all comments are welcome.

The patch may be applied as:

$ cd freeciv-1.11.4
$ patch -p1 <../freeciv-1.11.4-tcp.patch

The following files are modified:

client/clinet.c
common/game.h
common/netintf.c
common/netintf.h
common/packets.c
common/packets.h
configure
configure.in
server/civserver.c
server/console.c
server/sernet.c
server/sernet.h
server/stdinhand.c

I just subscribed to the freeciv-dev mailing list, and am not
yet very familiar with the freeciv source, but flame away ;-)

Lauri

-- 
"The credit belongs to the man in the arena whose face is marred by dust and
sweat and blood, who strives valiantly, who errs, and who comes up short again
and again, who knows the great enthusiasms, the great devotions, and spends
himself in a worthy cause. The man who at best knows the triumph of high
achievement and who at worst, if he fails, fails while daring greatly, so that
his place will never be with those cold timid souls who never knew victory or
defeat." - Theodore Roosevelt

freeciv-1.11.4-tcp.patch
Description: Text document