[lwip-devel] [task #10370] Raw API: Callback for received FIN to confirm connection is closed

[Iordan Neshev]

URL:
  <http://savannah.nongnu.org/task/?10370>

                 Summary: Raw API: Callback for received FIN to confirm
connection is closed
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: iordan_neshev
            Submitted on: Wed 05 May 2010 09:17:12 PM GMT
                Category: None
         Should Start On: Wed 05 May 2010 12:00:00 AM GMT
   Should be Finished on: Wed 05 May 2010 12:00:00 AM GMT
                Priority: 5 - Normal
                  Status: None
                 Privacy: Public
        Percent Complete: 0%
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None
                  Effort: 50.00

    _______________________________________________________

Details:

Recently the TCP_EVENT_RECV macro was changed  - 
it no longer calls the user-defined recv callback when TF_RXCLOSED is set
(which is set when we locally close the connection with
tcp_close/tcp_shutdown). Now it calls the default callback (tcp_recv_null)
instead.

In the near past the (raw) application had to call tcp_close() in order to
locally initiate the connection closing. The peer ACKs our close request by
sending us FIN. This FIN triggers this:

TCP_EVENT_RECV(pcb, NULL, ERR_OK, err);
in tcp_in.c: tcp_input(), line 346.

In the previous version the user-defined recv callback was called with pbuf p
== NULL. That's how the applications were informed that the link is closed by
the remote end.

In the new code TF_RXCLOSED is rised when tcp_close is called, preventing the
TCP_EVENT_RECV() macro to call the user-defined recv callback.

This functionality is needed in my application - I send files to a FTP
server. When I send all the data, I call tcp_close() and I wait for recv
callback to fire with p == NULL (i.e. I'm waiting for the FIN). This is the
only way for me to be sure that the whole file is uploaded.

There must be a way to inform the application for this event.
If calling the callback with p==NULL is no longer possible, it could be
called with err == ERR_CLSD instead.

Currently the macro looks like this:
#define TCP_EVENT_RECV(pcb,p,err,ret)                              \
  do {                                                             \
    if(((pcb)->recv != NULL) && (!((pcb)->flags & TF_RXCLOSED))) { \
      (ret) = (pcb)->recv((pcb)->callback_arg,(pcb),(p),(err));    \
    } else {                                                       \
      (ret) = tcp_recv_null(NULL, (pcb), (p), (err));              \
    }                                                              \
  } while (0)

One possible solution is to make it call
"(ret) = (pcb)->recv((pcb)->callback_arg,(pcb),(p),(err));"
even when the TF_RXCLOSED is rised, by ORing the condition with (err ==
ERR_CLSD):

#define TCP_EVENT_RECV(pcb,p,err,ret)                              \
  do {                                                             \
    if(((pcb)->recv != NULL) &&                                    \
      ((!((pcb)->flags & TF_RXCLOSED)) || ((err) == ERR_CLSD) )) { \
          (ret) = (pcb)->recv((pcb)->callback_arg,(pcb),(p),(err));    \
    } else {                                                       \
      (ret) = tcp_recv_null(NULL, (pcb), (p), (err));              \
    }                                                              \
  } while (0)


Then if we change 
TCP_EVENT_RECV(pcb, NULL, ERR_OK, err); with
TCP_EVENT_RECV(pcb, NULL, ERR_CLSD, err);
the callback will be called with p==NULL and err==ERR_CLSD,
which is enough for me to conclude the peer ACKed my close request.

The proposed macro change has the negative consequence that if there is no
user-defined recv callback, then the default recv callback tcp_recv_null()
will be called with err==ERR_CLSD instead of ERR_OK, which will lead to a bug
(tcp_close() would not be called).

Another, better way is to define this macro:
#define TCP_EVENT_RECV_FIN(pcb)                              \
  do {                                                             \
    if((pcb)->recv != NULL ) { \
          (pcb)->recv((pcb)->callback_arg,(pcb),NULL,ERR_CLSD);    \
    } \
  } while (0)


and call it after the original TCP_EVENT_RECV(pcb, NULL, ERR_OK, err);

The code would look like this:

        /* If a FIN segment was received, we call the callback
           function with a NULL buffer to indicate EOF. */
        if (recv_flags & TF_GOT_FIN) {
          /* correct rcv_wnd as the application won't call tcp_recved()
             for the FIN's seqno */
          if (pcb->rcv_wnd != TCP_WND) {
            pcb->rcv_wnd++;
          }

          TCP_EVENT_RECV(pcb, NULL, ERR_OK, err);               // this will 
call the
default recv callback
          TCP_EVENT_RECV_FIN(pcb);      // this will call the user-defined
callback if one is defined

          if (err == ERR_ABRT) {
            goto aborted;
          }

Note that err is not touched by TCP_EVENT_RECV_FIN(), it stays as
TCP_EVENT_RECV() has left it. This is something that we should discuss later
if you accept this approach.

By the way, when the peer closes the connection (i.e. he initiates the
close), everything is like before - the recv callback gets called with
p==NULL, so my application has no problems when it downloads files.

I'm currently using this new macro as workaround to the introduced problem.
So what do you think?





    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/task/?10370>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/