lwIP - A Lightweight TCP/IP stack - Bugs: bug #50837, TCP: zero window probe doesn't...
You are not allowed to post comments on this tracker with your current authentication level.
bug #50837: TCP: zero window probe doesn't timeout
Submitter: | preet <preetpal> | ||
Submitted: | Thu 20 Apr 2017 07:12:52 PM UTC | ||
Category: | TCP | Severity: | 3 - Normal |
Item Group: | Faulty Behaviour | Status: | Fixed |
Privacy: | Public | Assigned to: | jcunningham |
Open/Closed: | Closed | Planned Release: | None |
lwIP version: | 2.0.0 |
Jump to the original submission
Fri 25 Aug 2017 07:01:08 PM UTC, comment #53: |
Simon Goldschmidt <goldsimon> |
Fri 25 Aug 2017 03:11:44 PM UTC, comment #52: I attempted to move tcp_split_unsent_seg() to tcp.c, but ran into the complication that tcp_create_segment() is being used, which is local to tcp_out.c
|
Joel Cunningham <jcunningham> |
Thu 24 Aug 2017 07:30:26 PM UTC, comment #51:
|
Simon Goldschmidt <goldsimon> |
Tue 22 Aug 2017 08:51:17 PM UTC, comment #50:
|
Joel Cunningham <jcunningham> |
Tue 22 Aug 2017 07:55:14 PM UTC, comment #49: OK. From reading the patch, it seems to look better now.
|
Simon Goldschmidt <goldsimon> |
Tue 22 Aug 2017 03:08:13 PM UTC, comment #48: Simon,
|
Joel Cunningham <jcunningham> |
Mon 21 Aug 2017 07:22:35 AM UTC, comment #47: Joel, I tried, but it says "revspec 'c2af41b' not found" |
Simon Goldschmidt <goldsimon> |
Fri 11 Aug 2017 06:49:57 PM UTC, comment #46: Will do, thanks! |
Joel Cunningham <jcunningham> |
Fri 11 Aug 2017 06:29:47 PM UTC, comment #45: Yeah, sorry, I tried to find the time but failed. Give me a week or so :-/ |
Simon Goldschmidt <goldsimon> |
Fri 11 Aug 2017 02:05:56 PM UTC, comment #44: Ping! Anyone have a chance to review my patch? |
Joel Cunningham <jcunningham> |
Fri 04 Aug 2017 07:16:40 PM UTC, comment #43: Attached patch re-works the persist timer to make the discussed changes. From commit message:
|
Joel Cunningham <jcunningham> |
Thu 27 Jul 2017 09:19:56 PM UTC, comment #42:
|
Joel Cunningham <jcunningham> |
Thu 27 Jul 2017 07:00:25 PM UTC, comment #41: That sounds like an interesting solution that could work for us!
|
Simon Goldschmidt <goldsimon> |
Wed 26 Jul 2017 09:46:54 PM UTC, comment #40: Anymore thoughts on how to move forward?
|
Joel Cunningham <jcunningham> |
Sun 02 Jul 2017 04:41:33 PM UTC, comment #39: Simon,
|
Joel Cunningham <jcunningham> |
Mon 26 Jun 2017 02:38:43 PM UTC, comment #38: I agree that treating the send window as effectively zero when the next unsent segment is larger, is becoming more messy to maintain.
|
Joel Cunningham <jcunningham> |
Thu 22 Jun 2017 06:22:11 PM UTC, comment #37: Reading comment #34 (which totally slipped through), I guess Joel's proposed fix for this looks good.
|
Simon Goldschmidt <goldsimon> |
Thu 22 Jun 2017 04:11:29 PM UTC, comment #36: Hi Simon,
|
Axel Lin <axellin> |
Thu 18 May 2017 02:59:54 PM UTC, comment #35: Minor correction in proposed check, the equality should be flipped so snd_wnd is larger than pcb->unsent->len:
|
Joel Cunningham <jcunningham> |
Thu 18 May 2017 02:56:49 PM UTC, comment #34: Fayek,
|
Joel Cunningham <jcunningham> |
Wed 17 May 2017 10:32:18 PM UTC, comment #33: Joel/Simon,
|
Fayek <fwahhab> |
Tue 16 May 2017 10:01:10 PM UTC, comment #32: Preet,
|
Joel Cunningham <jcunningham> |
Tue 16 May 2017 08:36:48 PM UTC, comment #31: I had to manually add the changes. The patch did not apply; I have never tried "git apply" command so maybe it was a user error on my end.
|
preet <preetpal> |
Fri 12 May 2017 11:57:22 PM UTC, comment #30: I will test this by Monday. Sorry about the delay guys. |
preet <preetpal> |
Fri 12 May 2017 07:06:20 PM UTC, comment #29: Preet? Did you have the chance to test this? |
Simon Goldschmidt <goldsimon> |
Tue 09 May 2017 02:10:12 PM UTC, comment #28: Simon,
|
Joel Cunningham <jcunningham> |
Mon 08 May 2017 08:07:27 PM UTC, comment #27: Joel, sorry for taking so long again.
|
Simon Goldschmidt <goldsimon> |
Thu 04 May 2017 04:56:43 AM UTC, comment #26: Attached patch adds zero-window probing timeout. The implementation was straight-forward, the only exception was that I couldn't easily re-use pcb->nrtx because when the persist timer started in tcp_receive(), there could be in-flight data and thus an active RTO timer running.
|
Joel Cunningham <jcunningham> |
Wed 03 May 2017 10:27:20 PM UTC, comment #25:
|
Joel Cunningham <jcunningham> |
Wed 03 May 2017 08:02:57 PM UTC, comment #24: As to the mbox not being able to buffer one whole TCP RX window: I'm tempted to use pbufs as an RX chain and remove the mbox from each netconn...
|
Simon Goldschmidt <goldsimon> |
Wed 03 May 2017 08:00:57 PM UTC, comment #23:
|
Simon Goldschmidt <goldsimon> |
Tue 02 May 2017 07:00:04 PM UTC, comment #22: Preet,
|
Joel Cunningham <jcunningham> |
Tue 02 May 2017 06:45:02 PM UTC, comment #21: Yes, I believe it is indeed memp errors. "lwip_stats.memp[128_pool]->err"
|
preet <preetpal> |
Tue 02 May 2017 06:28:25 PM UTC, comment #20: I've added bug #50912. However, note that in your packet traces, fixing mbox handling of RST does NOT change anything as the RST is rejected because of invalid seqno!
|
Simon Goldschmidt <goldsimon> |
Tue 02 May 2017 02:41:03 PM UTC, comment #19: I think the root of the issue is that the mailbox used to post a pbuf pointer is full, and the RST is dropped. But even if the client on the other side goes missing without sending the RST, we should still be able to reset the connection on the LWIP (server) side.
|
preet <preetpal> |
Tue 02 May 2017 02:19:49 PM UTC, comment #18:
|
Joel Cunningham <jcunningham> |
Tue 02 May 2017 10:23:56 AM UTC, comment #17: I haven't looked at the zero window timeout. I thought that would work, though. After all, the zerow window time shown in the pcap is "only" a little less than 7 minutes. I'm not sure this is enough to let a TCP connection time out. And it's certainly not "forever"!
|
Simon Goldschmidt <goldsimon> |
Tue 02 May 2017 03:05:19 AM UTC, comment #16: preet,
|
Joel Cunningham <jcunningham> |
Tue 02 May 2017 12:14:13 AM UTC, comment #15: Tested with LWIP 2.0.2 with the same behavior. I noticed this time that there were KEEP-ALIVE probes being sent, as customized by my application but it was still not being used to close the lingering/zombie connection. The "mbox" error count continues to climb while the CLIENT(windows/python) had already disconnected.
|
preet <preetpal> |
Tue 25 Apr 2017 08:32:32 PM UTC, comment #14: Windows Version: 7
|
preet <preetpal> |
Mon 24 Apr 2017 07:56:21 PM UTC, comment #13: My thought is that the zero window probe (from the server) would ensure the RST mechanisms is re-triggered once the client has closed the socket, otherwise we end up with a half open connection and send is stuck blocking, which should not happen.
|
Joel Cunningham <jcunningham> |
Mon 24 Apr 2017 07:22:05 PM UTC, comment #12:
|
Simon Goldschmidt <goldsimon> |
Sun 23 Apr 2017 02:42:11 PM UTC, comment #11: Simon,
|
Joel Cunningham <jcunningham> |
Fri 21 Apr 2017 07:38:50 PM UTC, comment #10: Joel, thanks for that excellent analysis. I wonder why the Win/Python client doesn't re-send that ACK. Is there something wrong with our implementation of RFC 5961 section 3.2?
|
Simon Goldschmidt <goldsimon> |
Fri 21 Apr 2017 06:13:20 PM UTC, comment #9: Joel,
|
preet <preetpal> |
Fri 21 Apr 2017 02:11:39 PM UTC, comment #8: Also, the comment in the below code makes it seem like we are doing non-standard behavior, but we aren't. It's following RFC 5961 section 3.2 |
Joel Cunningham <jcunningham> |
Fri 21 Apr 2017 01:31:05 PM UTC, comment #7: preet,
|
Joel Cunningham <jcunningham> |
Thu 20 Apr 2017 10:51:09 PM UTC, comment #6: Sorry to go back and forth (git submodule confusion).
|
preet <preetpal> |
Thu 20 Apr 2017 09:37:15 PM UTC, comment #5: The CLIENT was windows machine running a python script.
|
preet <preetpal> |
Thu 20 Apr 2017 09:28:36 PM UTC, comment #4: The target port was 6060, and I was collecting the memory statistics in parallel on port 4005. Attached are the PCAP file, and also my LWIP configuration options. Note that this is with 1.4.x version, and not the 2.x which I accidentally used while opening the issue. The MBOX err counter related to the socket continues to climb and the task that was serving port 6060 is stuck inside the send() function.
|
preet <preetpal> |
Thu 20 Apr 2017 07:59:04 PM UTC, comment #3: Thanks for the correction. I'm still not 100% clear what happened in preet's case. If RST was received, failed to deliver to the mbox, but the pcb would still freed as part of the RST processing, meaning there would be no more zero-window probes. I would also expect the blocking send to be returned with an error (maybe that's also a bug)? |
Joel Cunningham <jcunningham> |
Thu 20 Apr 2017 07:34:43 PM UTC, comment #2:
|
Simon Goldschmidt <goldsimon> |
Thu 20 Apr 2017 07:30:56 PM UTC, comment #1: Hi preet, couple of questions about your scenario:
|
Joel Cunningham <jcunningham> |
Thu 20 Apr 2017 07:12:52 PM UTC, original submission:
LWIP race condition:
|
preet <preetpal> |
Depends on the following items: None found
Items that depend on this one: None found
There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.
Follow 18 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2017-08-25 | jcunningham | Status | In Progress | Fixed | |
Open/Closed | Open | Closed | |||
2017-08-22 | jcunningham | Attached File | - | Added 0001-tcp-persist-timer-re-work-bug-50837-rebase.patch, #41616 | |
2017-08-04 | jcunningham | Attached File | - | Added 0001-tcp-persist-timer-re-work-bug-50837.patch, #41415 | |
2017-07-27 | jcunningham | Status | None | In Progress | |
2017-05-18 | jcunningham | Attached File | - | Added bug-50837-zero-win-probes -2.xlsx, #40733 | |
2017-05-17 | fwahhab | Attached File | - | Added lwip_5004_trace.pcapng, #40729 | |
2017-05-16 | jcunningham | Attached File | - | Added bug-50837-zero-win-probes.xlsx, #40717 | |
2017-05-16 | preetpal | Attached File | - | Added lwip_50837_patch_test___without_timeout.pcapng, #40714 | |
Attached File | - | Added lwip_50837_patch_test___with_5s_send_timeout.pcapng, #40715 | |||
2017-05-04 | jcunningham | Attached File | - | Added 0001-bug-50837-add-zero-window-probe-timeout.patch, #40576 | |
2017-05-03 | jcunningham | Assigned to | None | jcunningham | |
Summary | LWIP TCP/IP race condition | TCP: zero window probe doesn't timeout | |||
2017-05-02 | preetpal | Attached File | - | Added lwip202_with_timeout.pcapng, #40548 | |
Attached File | - | Added lwip202_WITHOUT_timeout.pcapng, #40549 | |||
2017-04-20 | preetpal | Attached File | - | Added lwipopts.h, #40457 | |
2017-04-20 | preetpal | Attached File | - | Added lwip_trace.pcapng, #40456 | |
2017-04-20 | preetpal | Carbon-Copy | - | Added preetpal |
Powered by Savane 3.13-f8d8.
Corresponding source code
Right, didn't think of that ;-)