diff --git a/CHANGELOG b/CHANGELOG index aa750c0e..43619011 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -131,6 +131,11 @@ HISTORY ++ Bugfixes: + 2010-02-20: Simon Goldschmidt + * tcp.h, tcp.c, tcp_in.c, tcp_out.c: Task #10088: Correctly implement + close() vs. shutdown(). Now the application does not get any more + recv callbacks after calling tcp_close(). Added tcp_shutdown(). + 2010-02-19: Simon Goldschmidt * mem.c/.h, pbuf.c: Renamed mem_realloc() to mem_trim() to prevent confusion with realloc() diff --git a/UPGRADING b/UPGRADING index 78a1f18d..2b0f600f 100644 --- a/UPGRADING +++ b/UPGRADING @@ -1,7 +1,7 @@ This file lists major changes between release versions that require ports or applications to be changed. Use it to update a port or an application written for an older version of lwIP to correctly work -on a newer version. +with newer versions. (CVS HEAD) @@ -12,7 +12,12 @@ on a newer version. * Replaced struct ip_addr by typedef ip_addr_t. - * Raw API: when calling tcp_abort() from a raw API TCP callback function, + * Raw API: Changed the semantics of tcp_close() (since it was rather a + shutdown before): Now the application does *NOT* get any calls to the + recv callback after calling tcp_close() - this means that it also does + not get the NULL-pbuf which tells it the remote side has closed, too! + + * Raw API: When calling tcp_abort() from a raw API TCP callback function, make sure you return ERR_ABRT to prevent accessing unallocated memory. (ERR_ABRT now means the applicaiton has called tcp_abort!) diff --git a/src/core/tcp.c b/src/core/tcp.c index 51dcc576..8ee33aea 100644 --- a/src/core/tcp.c +++ b/src/core/tcp.c @@ -112,7 +112,9 @@ tcp_tmr(void) } /** - * Closes the connection held by the PCB. + * Closes the TX side of a connection held by the PCB. + * For tcp_close(), a RST is sent if the application didn't receive all data + * (tcp_recved() not called for all data passed to recv callback). * * Listening pcbs are freed and may not be referenced any more. * Connection pcbs are freed if not yet connected and may not be referenced @@ -125,15 +127,19 @@ tcp_tmr(void) * @return ERR_OK if connection has been closed * another err_t if closing failed and pcb is not freed */ -err_t -tcp_close(struct tcp_pcb *pcb) +static err_t +tcp_close_shutdown(struct tcp_pcb *pcb, u8_t rst_on_unacked_data) { err_t err; -#if TCP_DEBUG - LWIP_DEBUGF(TCP_DEBUG, ("tcp_close: closing in ")); - tcp_debug_print_state(pcb->state); -#endif /* TCP_DEBUG */ + if (rst_on_unacked_data && (pcb->state != LISTEN)) { + if ((pcb->refused_data != NULL) || (pcb->rcv_wnd != TCP_WND)) { + /* Not all data received by application, send RST to tell the remote + side about this. */ + tcp_rst(pcb->snd_nxt, pcb->rcv_nxt, &pcb->local_ip, &pcb->remote_ip, + pcb->local_port, pcb->remote_port); + } + } switch (pcb->state) { case CLOSED: @@ -204,6 +210,73 @@ tcp_close(struct tcp_pcb *pcb) return err; } +/** + * Closes the connection held by the PCB. + * + * Listening pcbs are freed and may not be referenced any more. + * Connection pcbs are freed if not yet connected and may not be referenced + * any more. If a connection is established (at least SYN received or in + * a closing state), the connection is closed, and put in a closing state. + * The pcb is then automatically freed in tcp_slowtmr(). It is therefore + * unsafe to reference it (unless an error is returned). + * + * @param pcb the tcp_pcb to close + * @return ERR_OK if connection has been closed + * another err_t if closing failed and pcb is not freed + */ +err_t +tcp_close(struct tcp_pcb *pcb) +{ +#if TCP_DEBUG + LWIP_DEBUGF(TCP_DEBUG, ("tcp_close: closing in ")); + tcp_debug_print_state(pcb->state); +#endif /* TCP_DEBUG */ + + /* Set a flag not to receive any more data... */ + pcb->flags |= TF_RXCLOSED; + /* ... and close */ + return tcp_close_shutdown(pcb, 1); +} + +/** + * Causes all or part of a full-duplex connection of this PCB to be shut down. + * This doesn't deallocate the PCB! + * + * @param pcb PCB to shutdown + * @param shut_rx shut down receive side if this is != 0 + * @param shut_tx shut down send side if this is != 0 + * @return ERR_OK if shutdown succeeded (or the PCB has already been shut down) + * another err_t on error. + */ +err_t +tcp_shutdown(struct tcp_pcb *pcb, int shut_rx, int shut_tx) +{ + if (pcb->state == LISTEN) { + return ERR_CONN; + } + if (shut_rx) { + /* shut down the receive side: free buffered data... */ + if (pcb->refused_data != NULL) { + pbuf_free(pcb->refused_data); + pcb->refused_data = NULL; + } + /* ... and set a flag not to receive any more data */ + pcb->flags |= TF_RXCLOSED; + } + if (shut_tx) { + /* This can't happen twice since if it succeeds, the pcb's state is changed. + Only close in these states as the others directly deallocate the PCB */ + switch (pcb->state) { + case SYN_RCVD: + case ESTABLISHED: + case CLOSE_WAIT: + return tcp_close_shutdown(pcb, 0); + } + } + /* @todo: return another err_t if not in correct state or already shut? */ + return ERR_OK; +} + /** * Abandons a connection and optionally sends a RST to the remote * host. Deletes the local protocol control block. This is done when diff --git a/src/core/tcp_in.c b/src/core/tcp_in.c index 8b48e213..cc83065d 100644 --- a/src/core/tcp_in.c +++ b/src/core/tcp_in.c @@ -338,6 +338,11 @@ tcp_input(struct pbuf *p, struct netif *inp) /* 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); if (err == ERR_ABRT) { goto aborted; @@ -576,8 +581,10 @@ tcp_process(struct tcp_pcb *pcb) return ERR_OK; } - /* Update the PCB (in)activity timer. */ - pcb->tmr = tcp_ticks; + if ((pcb->flags & TF_RXCLOSED) == 0) { + /* Update the PCB (in)activity timer unless rx is closed (see tcp_shutdown) */ + pcb->tmr = tcp_ticks; + } pcb->keep_cnt_sent = 0; tcp_parseopt(pcb); @@ -676,10 +683,8 @@ tcp_process(struct tcp_pcb *pcb) tcp_ack_now(pcb); pcb->state = CLOSE_WAIT; } - } - /* incorrect ACK number */ - else { - /* send RST */ + } else { + /* incorrect ACK number, send RST */ tcp_rst(ackno, seqno + tcplen, &(iphdr->dest), &(iphdr->src), tcphdr->dest, tcphdr->src); } diff --git a/src/include/lwip/tcp.h b/src/include/lwip/tcp.h index 4ec59fc5..159c0f8b 100644 --- a/src/include/lwip/tcp.h +++ b/src/include/lwip/tcp.h @@ -167,6 +167,7 @@ struct tcp_pcb * tcp_listen_with_backlog(struct tcp_pcb *pcb, u8_t backlog); void tcp_abandon (struct tcp_pcb *pcb, int reset); #define tcp_abort(pcb) tcp_abandon((pcb), 1) err_t tcp_close (struct tcp_pcb *pcb); +err_t tcp_shutdown(struct tcp_pcb *pcb, int shut_rx, int shut_tx); /* Flags for "apiflags" parameter in tcp_write and tcp_enqueue */ #define TCP_WRITE_FLAG_COPY 0x01 @@ -370,6 +371,7 @@ struct tcp_pcb { #define TF_ACK_NOW ((u8_t)0x02U) /* Immediate ACK. */ #define TF_INFR ((u8_t)0x04U) /* In fast recovery. */ #define TF_TIMESTAMP ((u8_t)0x08U) /* Timestamp option enabled */ +#define TF_RXCLOSED ((u8_t)0x10U) /* rx closed by tcp_shutdown */ #define TF_FIN ((u8_t)0x20U) /* Connection was closed locally (FIN segment enqueued). */ #define TF_NODELAY ((u8_t)0x40U) /* Disable Nagle algorithm */ #define TF_NAGLEMEMERR ((u8_t)0x80U) /* nagle enabled, memerr, try to output to prevent delayed ACK to happen */ @@ -521,13 +523,13 @@ err_t lwip_tcp_event(void *arg, struct tcp_pcb *pcb, else (ret) = ERR_OK; \ } while (0) -#define TCP_EVENT_RECV(pcb,p,err,ret) \ - do { \ - if((pcb)->recv != NULL) { \ - (ret) = (pcb)->recv((pcb)->callback_arg,(pcb),(p),(err)); \ - } else { \ - (ret) = tcp_recv_null(NULL, (pcb), (p), (err)); \ - } \ +#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) #define TCP_EVENT_CONNECTED(pcb,err,ret) \