Thu 25 Jun 2009 09:58:45 AM UTC, comment #14:
I've checked in some code which I hope should fix this, but I would welcome a second opinion if you're able to test it.
|
Thu 30 Apr 2009 04:19:42 PM UTC, comment #13:
In previous patch I moved tcplen calculation. It was wrong.
Now corrected patch.
(file #18063)
|
Fri 24 Apr 2009 12:15:44 PM UTC, comment #12:
I hope to find some time to look at both of these bugs.
|
Fri 24 Apr 2009 11:31:17 AM UTC, comment #11:
If somebody want patch together with
changes for bug #26267 (snd_nxt replaced with snd_max)
Actually it is one error.
(file #18007)
|
Thu 23 Apr 2009 05:39:07 PM UTC, comment #10:
Sorry, I clicked the wrong bug lol
|
Thu 23 Apr 2009 05:38:00 PM UTC, comment #9:
Since this is definitively not our bug, I'm closing it as invalid.
As to struct copying: I think the current implementation is good enough, but we could discuss about checking where we use it (especially regarding pointer casting).
|
Thu 23 Apr 2009 01:16:24 PM UTC, comment #8:
It works for me.
See applied patch fin_in_seq.diff
(file #18000)
|
Thu 23 Apr 2009 12:20:03 PM UTC, comment #7:
It should work better here and other places.
case FIN_WAIT_1:
tcp_receive(pcb);
-if (flags & TCP_FIN) {
+if (recv_flags == TF_GOT_FIN) {
Flag TF_GOT_FIN means that FIN segment was received in sequence.
I'll try it and say about results.
|
Thu 23 Apr 2009 11:51:09 AM UTC, comment #6:
and may be we can call tcp_update_rcv_ann_wnd() only once,
when inseg segment processed and oos queue processed.
|
Thu 23 Apr 2009 11:32:25 AM UTC, comment #5:
Agree with you,
replace with
pcb->rcv_nxt = seqno + tcplen;
and may be we can move string
where recalculated length of trimmed segment
tcplen = TCP_TCPLEN(&inseg);
to the place where it was trimmed
/* We have to trim the second edge of the incoming
segment. */
inseg.len = (u16_t)(pcb->ooseq->tcphdr->seqno - seqno);
pbuf_realloc(inseg.p, inseg.len);
+tcplen = TCP_TCPLEN(&inseg);
|
Thu 23 Apr 2009 09:40:54 AM UTC, comment #4:
The code came from Kelvin Lawson in December 2005:
http://lists.nongnu.org/archive/html/lwip-users/2005-12/msg00034.html
I don't understand how that code could make any difference either: if the received packet is a duplicate it wouldn't get there as its seqno would not be equal to rcv_nxt.
|
Thu 23 Apr 2009 09:26:50 AM UTC, comment #3:
Reply to comment #2
It looks good, but I don't understand why this code for checking
if (pcb->state != CLOSE_WAIT) {
was written early.
For this code condition
if (pcb->rcv_nxt == seqno) {
is already true.
It means
pcb->rcv_nxt += tcplen;
equal to
pcb->rcv_nxt = seqno + tcplen;
|
Thu 23 Apr 2009 08:18:47 AM UTC, comment #2:
As we know we're dealing with an in sequence packet at that point, could we not replace with:
pcb->rcv_nxt = seqno + tcplen;
This would work for all states I think. Any duplicate FINs would not come here as they'd be out of the window.
|
Thu 23 Apr 2009 08:10:25 AM UTC, comment #1:
This was also mentioned in BUG #26267.
I think there might be something to fix in tcp_receive() too:
/* First received FIN will be ACKed +1, on any successive (duplicate)
* FINs we are already in CLOSE_WAIT and have already done +1.
*/
if (pcb->state != CLOSE_WAIT) {
pcb->rcv_nxt += tcplen;
}
I think we should do the same for the CLOSING state where we have also already received a FIN.
|
Wed 22 Apr 2009 11:02:05 PM UTC, original submission:
After lwip initiates closing a connection, an out-of-order FIN will transition the state from FIN_WAIT_1 or FIN_WAIT_2 to TIME_WAIT. Any retransmitted segments that arrive after the FIN are then ACK'ed but never delivered to the application.
The following patch for tcp_in.c (1.3.0-STABLE) uses the same transition criteria as the ESTABLISHED state, and appears to fix the problem.
@@ -640,8 +640,8 @@
}
break;
case FIN_WAIT_1:
- tcp_receive(pcb);
- if (flags & TCP_FIN) {
+ accepted_inseq = tcp_receive(pcb);
+ if ((flags & TCP_FIN) && accepted_inseq) {
if (flags & TCP_ACK && ackno == pcb->snd_nxt) {
LWIP_DEBUGF(TCP_DEBUG,
("TCP connection closed %"U16_F" -> %"U16_F".\n", inseg.tcphdr->src, inseg.tcphdr->dest));
@@ -659,8 +659,8 @@
}
break;
case FIN_WAIT_2:
- tcp_receive(pcb);
- if (flags & TCP_FIN) {
+ accepted_inseq = tcp_receive(pcb);
+ if ((flags & TCP_FIN) && accepted_inseq) {
LWIP_DEBUGF(TCP_DEBUG, ("TCP connection closed %"U16_F" -> %"U16_F".\n", inseg.tcphdr->src, inseg.tcphdr->dest));
tcp_ack_now(pcb);
tcp_pcb_purge(pcb);
|