Issues with SubmitTcpBuffer4Tx

[John.Ohl]

When using SPEED_TCP, I've encountered a couple of issues and found what I hope are solutions, any feedback would be helpful.

I noticed that on a lossy and/or very slow network, its possible to run out of buffers very easily with GetTcpTxBuffer. I realize this could happen, but after comparing SockSubmitTcpBuffer4Tx with DoRawSend in tcp.cpp I noticed a few inconstancies and what I think are issues.

By adding a missing check to ensure we don't exceed our outstanding ACKS, it's possible to assume a return of 0 with SubmitTcpBuffer4Tx means to try again, rather than silently filling the outstanding acks list and returning NULL via GetTcpTxBuffer.

Would it not make more sense to return a value from SubmitTcpBuffer4Tx (such as 0) to indicate a retry is needed, than failing when getting a new buffer?

I added the following to tcp.cpp in SockSubmitTcpBuffer4Tx

Code: Select all

   // Stole this from the standard DoRawSend function, it happens on lossy / slow networks!
   if (outstanding_acks >= MAX_OUTSTANDING_ACK_BUFFERS) { return 0; }

Just before

Code: Select all

   int bytes_to_send = modulodiff( his_window, send_not_acked );

------

It also appears that the SockSubmitTcpBuffer4Tx function is performing some unnecessary checks/operations. It checks if the TXBuffer WAS full, and then checks if it is full, and no matter what the if statement will ALWAYS fail. So the entire statement can be removed. After comparing /w DoRawSend, it makes sense since we don't pull anything from the TXBuffer.

Code: Select all

        int wasfull = TxBuffer.Full();

         if ( ( wasfull ) && ( !TxBuffer.Full() ) )
         {
            SetWriteAvail(GetMyFd() );
         }

------

Lastly I noticed that the critical section (near the end of SockSubmitTcpBuffer4Tx) doesn't hold the lock while performing a few checks and calculations and then subsequently executing TcpSendwSum. Whereas DoRawSend holds the lock throughout the entire function.

I'm assuming, though not 100% positive that some race conditions may exist with checking the status' of the socket at the beginning of the function call in SockSubmitTcpBuffer4Tx and then calculating the modulodiff of the windows could cause issues if two different threads are submitting simultaneously outside of a lock.

Any thoughts on these 3, either separately or altogether?

Thanks!!