From d4b169a6deba676b8c6015b9f85e95d81736b59a Mon Sep 17 00:00:00 2001 From: goldsimon Date: Sun, 12 Feb 2012 14:14:19 +0100 Subject: [PATCH] partly fixed bug #25882: TCP hangs on MSS > pcb->snd_wnd (by not creating segments bigger than half the window) --- CHANGELOG | 4 ++++ src/core/tcp_in.c | 9 ++++++++- src/core/tcp_out.c | 11 ++++++----- src/include/lwip/tcp.h | 1 + 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index dd709867..ce8faea1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -62,6 +62,10 @@ HISTORY ++ Bugfixes: + 2012-02-12: Simon Goldschmidt + * tcp.h, tcp_in.c, tcp_out.c: partly fixed bug #25882: TCP hangs on + MSS > pcb->snd_wnd (by not creating segments bigger than half the window) + 2012-02-11: Simon Goldschmidt * tcp.c: fixed bug #35435: No pcb state check before adding it to time-wait queue while closing diff --git a/src/core/tcp_in.c b/src/core/tcp_in.c index 520815aa..4ec971ac 100644 --- a/src/core/tcp_in.c +++ b/src/core/tcp_in.c @@ -484,6 +484,7 @@ tcp_listen_input(struct tcp_pcb_listen *pcb) npcb->rcv_nxt = seqno + 1; npcb->rcv_ann_right_edge = npcb->rcv_nxt; npcb->snd_wnd = tcphdr->wnd; + npcb->snd_wnd_max = tcphdr->wnd; npcb->ssthresh = npcb->snd_wnd; npcb->snd_wl1 = seqno - 1;/* initialise to seqno-1 to force window update */ npcb->callback_arg = pcb->callback_arg; @@ -635,6 +636,7 @@ tcp_process(struct tcp_pcb *pcb) pcb->rcv_ann_right_edge = pcb->rcv_nxt; pcb->lastack = ackno; pcb->snd_wnd = tcphdr->wnd; + pcb->snd_wnd_max = tcphdr->wnd; pcb->snd_wl1 = seqno - 1; /* initialise to seqno - 1 to force window update */ pcb->state = ESTABLISHED; @@ -834,7 +836,7 @@ tcp_oos_insert_segment(struct tcp_seg *cseg, struct tcp_seg *next) * data, and if so frees the memory of the buffered data. Next, is places the * segment on any of the receive queues (pcb->recved or pcb->ooseq). If the segment * is buffered, the pbuf is referenced by pbuf_ref so that it will not be freed until - * i it has been removed from the buffer. + * it has been removed from the buffer. * * If the incoming segment constitutes an ACK for a segment that was used for RTT * estimation, the RTT is estimated here as well. @@ -869,6 +871,11 @@ tcp_receive(struct tcp_pcb *pcb) (pcb->snd_wl1 == seqno && TCP_SEQ_LT(pcb->snd_wl2, ackno)) || (pcb->snd_wl2 == ackno && tcphdr->wnd > pcb->snd_wnd)) { pcb->snd_wnd = tcphdr->wnd; + /* keep track of the biggest window announced by the remote host to calculate + the maximum segment size */ + if (pcb->snd_wnd_max < tcphdr->wnd) { + pcb->snd_wnd_max = tcphdr->wnd; + } pcb->snd_wl1 = seqno; pcb->snd_wl2 = ackno; if (pcb->snd_wnd == 0) { diff --git a/src/core/tcp_out.c b/src/core/tcp_out.c index ef182550..50e1b6bf 100644 --- a/src/core/tcp_out.c +++ b/src/core/tcp_out.c @@ -229,7 +229,7 @@ tcp_pbuf_prealloc(pbuf_layer layer, u16_t length, u16_t max_length, LWIP_UNUSED_ARG(apiflags); LWIP_UNUSED_ARG(first_seg); /* always create MSS-sized pbufs */ - alloc = pcb->mss; + alloc = max_length; #else /* LWIP_NETIF_TX_SINGLE_PBUF */ if (length < max_length) { /* Should we allocate an oversized pbuf, or just the minimum @@ -369,6 +369,8 @@ tcp_write(struct tcp_pcb *pcb, const void *arg, u16_t len, u8_t apiflags) u16_t concat_chksummed = 0; #endif /* TCP_CHECKSUM_ON_COPY */ err_t err; + /* don't allocate segments bigger than half the maximum window we ever received */ + u16_t mss_local = LWIP_MIN(pcb->mss, pcb->snd_wnd_max/2); #if LWIP_NETIF_TX_SINGLE_PBUF /* Always copy to try to create single pbufs for TX */ @@ -427,7 +429,7 @@ tcp_write(struct tcp_pcb *pcb, const void *arg, u16_t len, u8_t apiflags) /* Usable space at the end of the last unsent segment */ unsent_optlen = LWIP_TCP_OPT_LENGTH(last_unsent->flags); - space = pcb->mss - (last_unsent->len + unsent_optlen); + space = mss_local - (last_unsent->len + unsent_optlen); /* * Phase 1: Copy data directly into an oversized pbuf. @@ -520,7 +522,7 @@ tcp_write(struct tcp_pcb *pcb, const void *arg, u16_t len, u8_t apiflags) while (pos < len) { struct pbuf *p; u16_t left = len - pos; - u16_t max_len = pcb->mss - optlen; + u16_t max_len = mss_local - optlen; u16_t seglen = left > max_len ? max_len : left; #if TCP_CHECKSUM_ON_COPY u16_t chksum = 0; @@ -530,7 +532,7 @@ tcp_write(struct tcp_pcb *pcb, const void *arg, u16_t len, u8_t apiflags) if (apiflags & TCP_WRITE_FLAG_COPY) { /* If copy is set, memory should be allocated and data copied * into pbuf */ - if ((p = tcp_pbuf_prealloc(PBUF_TRANSPORT, seglen + optlen, pcb->mss, &oversize, pcb, apiflags, queue == NULL)) == NULL) { + if ((p = tcp_pbuf_prealloc(PBUF_TRANSPORT, seglen + optlen, mss_local, &oversize, pcb, apiflags, queue == NULL)) == NULL) { LWIP_DEBUGF(TCP_OUTPUT_DEBUG | 2, ("tcp_write : could not allocate memory for pbuf copy size %"U16_F"\n", seglen)); goto memerr; } @@ -1062,7 +1064,6 @@ tcp_output_segment(struct tcp_seg *seg, struct tcp_pcb *pcb) /* Add any requested options. NB MSS option is only set on SYN packets, so ignore it here */ - //LWIP_ASSERT("seg->tcphdr not aligned", ((mem_ptr_t)seg->tcphdr % MEM_ALIGNMENT) == 0); opts = (u32_t *)(void *)(seg->tcphdr + 1); if (seg->flags & TF_SEG_OPTS_MSS) { *opts = TCP_BUILD_MSS_OPTION(pcb->mss); diff --git a/src/include/lwip/tcp.h b/src/include/lwip/tcp.h index e6b6a292..c6e61ad1 100644 --- a/src/include/lwip/tcp.h +++ b/src/include/lwip/tcp.h @@ -225,6 +225,7 @@ struct tcp_pcb { window update. */ u32_t snd_lbb; /* Sequence number of next byte to be buffered. */ u16_t snd_wnd; /* sender window */ + u16_t snd_wnd_max; /* the maximum sender window announced by the remote host */ u16_t acked;