• AVR Freaks

AnsweredHot!All Harmony versions: A minor issue with TCPIP_TCP_Flush()

Author
jdeguire
Super Member
  • Total Posts : 501
  • Reward points : 0
  • Joined: 2012/01/13 07:48:44
  • Location: United States
  • Status: offline
2019/12/10 06:37:35 (permalink)
0

All Harmony versions: A minor issue with TCPIP_TCP_Flush()

Like the thread title says, I found a pretty minor issue with the TCPIP_TCP_Flush() function that can affect you if you periodically call it.  From what I can tell, this function hasn't changed much too much throughout Harmony's history.  This issue is at least present in Harmony 2 and 3.
 
Anyway, we have some old code that was converted to use Harmony quite a long time ago.  This code would periodically flush the TCP send buffer in order to reduce packet latency (the old MLA stack and very early versions of Harmony did not support the Nagle Algorithm and had their own coalescing system**).  Recently, we found that if we (1) disconnect from the peer (a PC) using TCPIP_TCP_Disconnect() and then (2) continue to periodically call TCPIP_TCP_Flush(), then the stack will be essentially stuck in the FIN_WAIT_1 state forever if the peer is unresponsive to FINs.  This is because the TCPIP_TCP_Flush() function resets the retry counter and timer used by the FIN_WAIT_1 state to know when the bail out and just send an RST.
 
Fortunately, there are two potential simple fixes.  The first is to remember to check if you're still connected if you're going to call TCPIP_TCP_Flush() (or maybe just disable Nagle in Harmony 2 and 3 if you want that).  The second is to make a small modification to TCPIP_TCP_Flush() to check that for you so that you do not have to remember.  For this old code, I ended up just checking if we're still connected before flushing the buffer, but a modification to the stack might look something like this (note that TCPIP_TCP_Flush() calls _TcpFlush()):
 
static bool _TcpFlush(TCB_STUB* pSkt)
{
    // MOD:  Check for these states, which was not done before.
    // Do this only in these two states because this will reset a timer that other states need to
    // run and so flushing can prevent those states (like FIN_WAIT_1) from progressing.
    if((pSkt->smState == TCPIP_TCP_STATE_ESTABLISHED || pSkt->smState == TCPIP_TCP_STATE_CLOSE_WAIT) &&
        pSkt->txHead != pSkt->txUnackedTail && pSkt->remoteWindow != 0)
    { // The check remoteWindow != 0 stops us sending lots of
        // ACKs with len == 0, when the other host is slow
        // Send the TCP segment with all unacked bytes
        return _TcpSend(pSkt, ACK, SENDTCP_RESET_TIMERS) == 0;
    }

    return false;
}

 
Hopefully, this helps.
 
**In case you're curious, old versions of Harmony and the MLA stack would coalesce data until either 40ms have passed or until the Tx buffer is at least half-full.  The Harmony 2 and 3 stacks still do this, but also support the Nagle algorithm.  It does work well and helps keeps responsiveness reasonably good.
#1
rainad
Moderator
  • Total Posts : 1247
  • Reward points : 0
  • Joined: 2009/05/01 13:39:25
  • Location: 0
  • Status: offline
Re: All Harmony versions: A minor issue with TCPIP_TCP_Flush() 2019/12/10 08:37:18 (permalink) ☼ Best Answerby jdeguire 2019/12/10 09:34:33
5 (2)
Thank you for finding and reporting this. We'll look into it and provide/incorporate the fix ASAP. 
Calling explicitly TCP_FLush() is discouraged (unless you know that there's no more data), because it interferes with the TCP state machine and actually impacts negatively on the throughput. But calling it shouldn't create any problems, of course.
 
#2
jdeguire
Super Member
  • Total Posts : 501
  • Reward points : 0
  • Joined: 2012/01/13 07:48:44
  • Location: United States
  • Status: offline
Re: All Harmony versions: A minor issue with TCPIP_TCP_Flush() 2019/12/10 09:35:58 (permalink)
0
Awesome, thanks for the response!
 
Yup, I'll have to investigate if that call even needs to be in our code anymore, though at least it was simple to find a way around it.
#3
Jump to:
© 2020 APG vNext Commercial Version 4.5