lwIP - A Lightweight TCP/IP stack - Tasks: task #14128, Appropriate byte counting/stretch...
You are not allowed to post comments on this tracker with your current authentication level.
task #14128: Appropriate byte counting/stretch ACK support
Submitter: | Joel Cunningham <jcunningham> | ||
Submitted: | Mon 22 Aug 2016 04:15:28 PM UTC | ||
Category: | TCP | Should Start On: | Mon 22 Aug 2016 12:00:00 AM UTC |
Should be Finished on: | Mon 22 Aug 2016 12:00:00 AM UTC | Priority: | 5 - Normal |
Status: | Done | Privacy: | Public |
Assigned to: | jcunningham | Percent Complete: | 100% |
Open/Closed: | Closed | Planned Release: | 2.1.0 |
Effort: | 0.00 |
Jump to the original submission
Thu 20 Apr 2017 09:02:17 PM UTC, comment #32: |
Joel Cunningham <jcunningham> |
Thu 20 Apr 2017 08:54:52 PM UTC, comment #31: Oh, right. How could I have missed that ;-) |
Simon Goldschmidt <goldsimon> |
Thu 20 Apr 2017 08:53:09 PM UTC, comment #30: My change adds two variables:
|
Joel Cunningham <jcunningham> |
Thu 20 Apr 2017 08:49:30 PM UTC, comment #29: I don't get why it requires 8 bytes for LWIP_WND_SCALE==1, but I'll check after you pushed. |
Simon Goldschmidt <goldsimon> |
Thu 20 Apr 2017 08:41:14 PM UTC, comment #28: Cool. |
Simon Goldschmidt <goldsimon> |
Thu 20 Apr 2017 08:36:07 PM UTC, comment #27: I did find a 2 byte hole. unsent_oversize included 2 bytes of padding. Tests done with MSVC. So if we are building with LWIP_WND_SCALE set to 0, bytes_acked can go in this hole, otherwise with LWIP_WND_SCALE set to 1, it goes after the hole (and doesn't create any new hole since tcpwnd_size_t is 4 bytes). Here's my testing:
|
Joel Cunningham <jcunningham> |
Thu 20 Apr 2017 07:25:38 PM UTC, comment #26: Looks good to me now :-)
|
Simon Goldschmidt <goldsimon> |
Wed 19 Apr 2017 09:27:24 PM UTC, comment #25: New patched attached that removes options and old cwnd growth code |
Joel Cunningham <jcunningham> |
Wed 19 Apr 2017 05:36:16 PM UTC, comment #24: I think the only valid reason to keep it as an option would be to get a small stack vs. a faster stack. I don't know how much code is added by this patch, but I think TCP has grown quite a but since 1.2.0 (just an example version to compare to). |
Simon Goldschmidt <goldsimon> |
Wed 19 Apr 2017 04:42:09 PM UTC, comment #23: > Looks good, too. The downside is that this is another option noone will ever change. Since the default is 'enabled', maybe it's better to just use it and dump the old behaviour? All the #if code looks so ugly...
|
Joel Cunningham <jcunningham> |
Tue 18 Apr 2017 08:12:40 PM UTC, comment #22: Looks good, too. The downside is that this is another option noone will ever change. Since the default is 'enabled', maybe it's better to just use it and dump the old behaviour? All the #if code looks so ugly...
|
Simon Goldschmidt <goldsimon> |
Thu 13 Apr 2017 12:43:04 AM UTC, comment #21: New patch (0001-tcp-task-14128-Appropriate-Byte-Counting-support.patch) implementing additional RTO tracking requirement and adding a unit test for RTO tracking.
|
Joel Cunningham <jcunningham> |
Fri 31 Mar 2017 04:38:55 PM UTC, comment #20: Just pushing them is fine. |
Simon Goldschmidt <goldsimon> |
Fri 31 Mar 2017 03:13:23 PM UTC, comment #19: Ping! I've got a couple of de-duplication changes in TCP unit tests. Want me to post patches here or just push them? No functional changes, just elimination of bunch of duplicate setup code repeated per test |
Joel Cunningham <jcunningham> |
Thu 23 Mar 2017 06:30:16 PM UTC, comment #18: Here's an update on the additional RTO support. My idea of checking unsent/unacked queue against pcb->snd_nxt isn't 100% full-proof because snd_nxt may be advanced if new segments are sent during the RTO period given the cwnd opens enough. I can provide some examples if this isn't easily understood
|
Joel Cunningham <jcunningham> |
Wed 15 Mar 2017 02:40:48 PM UTC, comment #17: Simon,
|
Joel Cunningham <jcunningham> |
Tue 14 Mar 2017 07:14:37 AM UTC, comment #16: Hmm, seems to me that it would be good to have this enabled. Code size increasement/slowdown is tiny, the only real downside is the added 'bytes_acked' member. But I'm OK with enabling this: otherwise, probably noone would enable it and noone would get the performance benefits.
|
Simon Goldschmidt <goldsimon> |
Mon 13 Mar 2017 10:11:02 PM UTC, comment #15: Any thoughts on the defaults? |
Joel Cunningham <jcunningham> |
Wed 08 Mar 2017 10:22:53 PM UTC, comment #14: Simon,
|
Joel Cunningham <jcunningham> |
Wed 08 Mar 2017 07:47:45 PM UTC, comment #13: The implementation looks fine, but pelase add "LWIP_" to new defines to prevent confusion with other libraries (I wish we could enforce this on all defines, but that will break ports or require a huge portiability-define-file...) |
Simon Goldschmidt <goldsimon> |
Tue 07 Mar 2017 05:40:00 AM UTC, comment #12: I did some more investigation of NetBSD and it has two sysctl variables that control ABC, tcp.abc.enable and tcp.abc.aggressive. With NetBSD 7.0.2, both of these are set to 1, indicating ABC is on by default and is using an L value of 2 * SMSS. Hopefully this is helpful in evaluating what our defaults for LwIP should be
|
Joel Cunningham <jcunningham> |
Sat 04 Mar 2017 12:04:02 AM UTC, comment #11: I'm further down the rabbit hole now.
|
Joel Cunningham <jcunningham> |
Fri 03 Mar 2017 08:45:00 PM UTC, comment #10: Mailing list post, including patch: http://marc.info/?l=linux-netdev&m=138324287502447&w=2
|
Joel Cunningham <jcunningham> |
Fri 03 Mar 2017 07:52:24 PM UTC, comment #9: Simon,
|
Joel Cunningham <jcunningham> |
Fri 03 Mar 2017 06:56:15 PM UTC, comment #8: Seems like Linux does not bound the CWND increase for a single ACK, this seems wrong. RFC 5681 "RECOMMEND"s:
|
Ambroz Bizjak <abizjak> |
Fri 03 Mar 2017 06:44:26 PM UTC, comment #7: Please do keep in mind Linux is GPL, we are BSD. We cannot copy from Linux and I don't think posting Linux source code here is a good thing to do.
|
Simon Goldschmidt <goldsimon> |
Fri 03 Mar 2017 06:41:20 PM UTC, comment #6: Here's what Linux's tcp_cong.c is doing:
|
Joel Cunningham <jcunningham> |
Fri 03 Mar 2017 06:25:57 PM UTC, comment #5: Thanks for the heads up Ambroz. RFC 5681 references RFC 3465 in "section 3.1. Slow Start and Congestion Avoidance". I believe LwIP implements correctly most of slow start/congestion avoidance/fast recovery, but I haven't done a full audit. I've just noticed the issue with cwnd being susceptible to ACK division and performing sub-optimal when stretch ACKs are used (common case is delayed ACKs)
|
Joel Cunningham <jcunningham> |
Fri 03 Mar 2017 06:13:41 PM UTC, comment #4: Hi everyone,
|
Ambroz Bizjak <abizjak> |
Fri 03 Mar 2017 05:46:26 PM UTC, comment #3: I'd like to target this for 2.1.0 and can put together a patch
|
Joel Cunningham <jcunningham> |
Tue 23 Aug 2016 02:01:55 PM UTC, comment #2: Simon,
|
Joel Cunningham <jcunningham> |
Tue 23 Aug 2016 11:54:36 AM UTC, comment #1: Does the fact that this is assigned to you mean you're working on it? That would be good to know :-) |
Simon Goldschmidt <goldsimon> |
Mon 22 Aug 2016 04:15:28 PM UTC, original submission:
LwIP currently doesn't implement appropriate byte counting when making adjustments to the congestion window during slow start. RFC 3465 outlines both security and performance issues with the initial slow start implementation
|
Joel Cunningham <jcunningham> |
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 14 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2017-04-20 | jcunningham | Status | In Progress | Done | |
Percent Complete | 0% | 100% | |||
Open/Closed | Open | Closed | |||
2017-04-19 | jcunningham | Attached File | - | Added 0001-tcp-task-14128-Appropriate-Byte-Counting-support-NO-OPTIONS.patch, #40445 | |
2017-04-13 | jcunningham | Attached File | - | Added 0001-tcp-task-14128-Appropriate-Byte-Counting-support.patch, #40384 | |
2017-03-07 | jcunningham | Attached File | - | Added 256KB-abc-1-limit-2.txt, #39916 | |
2017-03-07 | jcunningham | Attached File | - | Added 0001-Add-ABC-support.patch, #39912 | |
Attached File | - | Added shell-stat-abc-0.txt, #39913 | |||
Attached File | - | Added shell-stat-abc-1-limit-2.txt, #39914 | |||
Attached File | - | Added 256KB-abc-0.txt, #39915 | |||
2017-03-03 | jcunningham | Status | None | In Progress | |
Planned Release | None | 2.1.0 | |||
2016-08-23 | goldsimon | Summary | Appropriate btye counting/stretch ACK support | Appropriate byte counting/stretch ACK support | |
2016-08-22 | jcunningham | Summary | Appropriate bye counting/stretch ACK support | Appropriate btye counting/stretch ACK support |
Powered by Savane 3.13-d3ae.
Corresponding source code
Committed in de90d03e483010be770d03d65e34792046b39c6d