tcp: bug #50614: move unsent queue check/pcb->snd_buf update

This commit corrects what looks like an ancient incorrect organization
of the logic for processing an ACK which acks new data.  Once moved,
we can also change to using TCP_SEQ_LEQ on ackno instead of TCP_BETWEEN
because ackno has already been checked against snd_nxt

The work of checking the unsent queue and updating pcb->snd_buf (both
steps required for new data ACK) should be located under the conditional
that checks TCP_SEQ_BETWEEN(ackno, pcb->lastack+1, pcb->snd_nxt)

The comment following the unsent queue check/pcb->snd_buf update even
indicates "End of ACK for new data processing" when the logic is clearly
outside of this check

From what I can tell, this mis-organization isn't causing any incorrect
behavior since the unsent queue checked that ackno was between start of
segment and snd_nxt and recv_acked would be 0 during pcb->snd_buf update.
Instead this is waisted work for duplicate ACKS (can be common) and other
old ACKs
This commit is contained in:
Joel Cunningham 2017-03-22 16:19:23 -05:00
parent 720f9b3a0b
commit c722261142

View File

@ -1188,10 +1188,6 @@ tcp_receive(struct tcp_pcb *pcb)
nd6_reachability_hint(ip6_current_src_addr());
}
#endif /* LWIP_IPV6 && LWIP_ND6_TCP_REACHABILITY_HINTS*/
} else {
/* Out of sequence ACK, didn't really ack anything */
tcp_send_empty_ack(pcb);
}
/* We go through the ->unsent list to see if any of the segments
on the list are acknowledged by the ACK. This may seem
@ -1200,8 +1196,8 @@ tcp_receive(struct tcp_pcb *pcb)
->unsent list after a retransmission, so these segments may
in fact have been sent once. */
while (pcb->unsent != NULL &&
TCP_SEQ_BETWEEN(ackno, lwip_ntohl(pcb->unsent->tcphdr->seqno) +
TCP_TCPLEN(pcb->unsent), pcb->snd_nxt)) {
TCP_SEQ_LEQ(lwip_ntohl(pcb->unsent->tcphdr->seqno) +
TCP_TCPLEN(pcb->unsent), ackno)) {
LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_receive: removing %"U32_F":%"U32_F" from pcb->unsent\n",
lwip_ntohl(pcb->unsent->tcphdr->seqno), lwip_ntohl(pcb->unsent->tcphdr->seqno) +
TCP_TCPLEN(pcb->unsent)));
@ -1227,6 +1223,10 @@ tcp_receive(struct tcp_pcb *pcb)
}
pcb->snd_buf += recv_acked;
/* End of ACK for new data processing. */
} else {
/* Out of sequence ACK, didn't really ack anything */
tcp_send_empty_ack(pcb);
}
LWIP_DEBUGF(TCP_RTO_DEBUG, ("tcp_receive: pcb->rttest %"U32_F" rtseq %"U32_F" ackno %"U32_F"\n",
pcb->rttest, pcb->rtseq, ackno));