TCP_HAVERCVDFIN

[Sam Kumar]

Hello,
I am working with the code for the TCP Stack. I noticed that in tcp_fsm.h,
there is a macro called TCP_HAVERCVDFIN. It is defined as follows:

#define TCPS_HAVERCVDFIN(s) ((s) >= TCPS_TIME_WAIT)

In "TCP/IP Illustrated, Volume 2" (by Wright and Stevens), the authors
mention this. On page 807, they write:
"... the name TCPS_HAVERCVDFIN is misleading. A FIN has also been received
in the CLOSE_WAIT, CLOSING, and LAST_ACK states."

However, I feel that there parts of the code that act as if
TCPS_HAVERCVDFIN were defined as

#define TCPS_HAVERCVDFIN(s) ((s) == TCPS_TIME_WAIT || (s) ==
TCPS_CLOSE_WAIT || (s) == TCPS_LAST_ACK || (s) == TCPS_CLOSING)

Let me point out the parts of the code and comments in the code that appear
to assume this:

1. In tcp_input.c, line 2499:
                /*
                 * If this is the first time we've seen a
                 * FIN from the remote, this is not a
                 * duplicate and it needs to be processed
                 * normally.  This happens during a
                 * simultaneous close.
                 */
                if ((thflags & TH_FIN) &&
                    (TCPS_HAVERCVDFIN(tp->t_state) == 0)) {
                    tp->t_dupacks = 0;
                    break;
                }
The comment suggests that the body of the if statement should only be
executed when processing the first FIN from the remote; however, due to the
definition of TCPS_HAVERCVDFIN, it seems that it would be executed for
duplicate FINs too.

2. In tcp_input.c, line 2922:
    if ((thflags & TH_URG) && th->th_urp &&
        TCPS_HAVERCVDFIN(tp->t_state) == 0) {

        ... (I omitted some code here for clarity)

        /*
         * If this segment advances the known urgent pointer,
         * then mark the data stream.  This should not happen
         * in CLOSE_WAIT, CLOSING, LAST_ACK or TIME_WAIT STATES since
         * a FIN has been received from the remote side.
         * In these states we ignore the URG.

The comment suggests that URG is ignored if the TCP is in the CLOSE_WAIT,
CLOSING, or LAST_ACK states, but I don't see any code enforcing this. If
TCPS_HAVERCVDFIN were defined to include these states, then the code would
be consistent with the comment. Wright and Stevens point this out (page
983): "The macro TCPS_HAVERCVDFIN is true only for the TIME_WAIT state, so
the URG is processed in any other state. This is contrary to a comment
appearing later in the code stating that the URG flag is ignored in the
CLOSE_WAIT, CLOSING, LAST_ACK, or TIME_WAIT states."

3. In tcp_input.c, line 2993:
    /*
     * Process the segment text, merging it into the TCP sequencing queue,
     * and arranging for acknowledgment of receipt if necessary.
     * This process logically involves adjusting tp->rcv_wnd as data
     * is presented to the user (this happens in tcp_usrreq.c,
     * case PRU_RCVD).  If a FIN has already been received on this
     * connection then we just ignore the text.
     */
    if ((tlen || (thflags & TH_FIN)) &&
        TCPS_HAVERCVDFIN(tp->t_state) == 0) {

The comment indicates that the segment text is ignored if we already
received a FIN, but this is only if the case if TCPS_HAVERCVDFIN is changed
to include the CLOSE_WAIT, CLOSING, or LAST_ACK states.

4. In tcp_output.c, line 621:
     * Don't send an independent window update if a delayed
     * ACK is pending (it will get piggy-backed on it) or the
     * remote side already has done a half-close and won't send
     * more data.  Skip this if the connection is in T/TCP
     * half-open state.
     */
    if (recwin > 0 && !(tp->t_flags & TF_NEEDSYN) &&
        !(tp->t_flags & TF_DELACK) &&
        !TCPS_HAVERCVDFIN(tp->t_state)) {

Due to the definition of TCPS_HAVERCVDFIN, a window update may be sent even
if the connection peer has already sent a FIN (contrary to the comment).

So it seems to me that TCPS_HAVERCVDFIN should be defined differently, to
include the CLOSE_WAIT, CLOSING, or LAST_ACK states. However, there is one
part of the code that appears to use the existing definition. See
tcp_input.c, line 3062:
        if (TCPS_HAVERCVDFIN(tp->t_state) == 0) {
            socantrcvmore(so);
            /*
             * If connection is half-synchronized
             * (ie NEEDSYN flag on) then delay ACK,
             * so it may be piggybacked when SYN is sent.
             * Otherwise, since we received a FIN then no
             * more input can be expected, send ACK now.
             */
            if (tp->t_flags & TF_NEEDSYN)
                tp->t_flags |= TF_DELACK;
            else
                tp->t_flags |= TF_ACKNOW;
            tp->rcv_nxt++;
        }
If a duplicate FIN is received (because our ACK was lost), then we need to
retransmit the ACK. I'm not sure if the above code in the if statement
needs to be executed for that to happen (maybe someone could clarify that?)

In summary I suspect that the TCPS_HAVERCVDFIN macro needs to be redefined,
but I'm not sure whether that is actually the case. I wanted to open a
discussion about this to explore whether this is a legitimate issue.