lwIP - A Lightweight TCP/IP stack - Bugs: bug #20287, tcp_output_nagle sends too early
You are not allowed to post comments on this tracker with your current authentication level.
bug #20287: tcp_output_nagle sends too early
Submitter: | Simon Goldschmidt <goldsimon> | ||
Submitted: | Thu 28 Jun 2007 10:40:41 AM UTC | ||
Category: | TCP | Severity: | 4 - Important |
Item Group: | Faulty Behaviour | Status: | Fixed |
Privacy: | Public | Assigned to: | goldsimon |
Open/Closed: | Closed | Planned Release: | None |
lwIP version: | None |
Jump to the original submission
Wed 21 Nov 2007 06:39:55 PM UTC, comment #28: |
Simon Goldschmidt <goldsimon> |
Wed 21 Nov 2007 05:59:37 PM UTC, comment #27: I chose to check it in like that: it's stable for me and I have the feeling it only gets tested by others after checking in. |
Simon Goldschmidt <goldsimon> |
Mon 12 Nov 2007 08:09:18 PM UTC, comment #26:
|
Simon Goldschmidt <goldsimon> |
Mon 12 Nov 2007 12:43:03 PM UTC, comment #25: If in tcp_enqueue, the number of enqueued pbufs has reached the maximum limit, TCP has to wait for data to be acknowledged to enqueue more. If this limit is reached at a point where nagle would wait and not send (e.g. the only unsent segment is half full; at least one segment in unacked), lwIP is stuck until the remote side acknowledges unacked data. This typically leads to an ACK being sent by a remote TCP timer because of the delayed ACK strategy.
|
Simon Goldschmidt <goldsimon> |
Mon 12 Nov 2007 12:27:20 PM UTC, comment #24: I'm not sure I fully understand why we need a special case for when there's been a memory error.
|
Kieran Mansley <kieranm> |
Sun 11 Nov 2007 09:10:04 PM UTC, comment #23: Is everyone OK with using the last free bit of tcp_pcb->flags for remembering a memory error in tcp_enqueue?
|
Simon Goldschmidt <goldsimon> |
Sun 11 Nov 2007 08:33:39 PM UTC, comment #22: Don't know where my head was... TCP_NODELAY obviously doesn't work for that, we'd have to create another flag. |
Simon Goldschmidt <goldsimon> |
Sun 11 Nov 2007 05:47:22 PM UTC, comment #21: Just a little status update for anyone who's interested:
|
Simon Goldschmidt <goldsimon> |
Fri 09 Nov 2007 01:10:38 PM UTC, comment #20: Hmm, it's been a while but I think the status was 'works for me'. I only wanted some feedback from others since this really a crucial part of the stack's TCP implementation...
|
Simon Goldschmidt <goldsimon> |
Fri 09 Nov 2007 01:05:20 PM UTC, comment #19: Any status update on this? Sounds like it's done, and just needs checking in. |
Kieran Mansley <kieranm> |
Fri 05 Oct 2007 03:14:38 PM UTC, comment #18: This has taken some time, but finally, I found some free time to work on it ;-)
|
Simon Goldschmidt <goldsimon> |
Wed 29 Aug 2007 09:25:45 AM UTC, comment #17: No, not yet. I plan to have more time to spend on this next month :-( |
Simon Goldschmidt <goldsimon> |
Wed 29 Aug 2007 09:12:55 AM UTC, comment #16: Is this done? |
Kieran Mansley <kieranm> |
Mon 09 Jul 2007 01:11:53 PM UTC, comment #15:
|
Simon Goldschmidt <goldsimon> |
Mon 09 Jul 2007 01:09:10 PM UTC, comment #14: I'd prefer not to add the call to tcp_output from tcp_write as the API would lose some flexibility.
|
Kieran Mansley <kieranm> |
Mon 09 Jul 2007 01:07:16 PM UTC, comment #13: I'd favour moving all the nagle code into tcp_output. This would add one function call per tcp_write call, but it's more like what people suppose TCP does, I think.
|
Simon Goldschmidt <goldsimon> |
Mon 09 Jul 2007 01:03:04 PM UTC, comment #12: Or perhaps have a flag to tcp_output to say "send as much as possible" or "just send one segment", and leave the nagle code as is? Haven't thought this through, but might be simpler. |
Kieran Mansley <kieranm> |
Mon 09 Jul 2007 01:01:25 PM UTC, comment #11: OK, I see what you mean.
|
Kieran Mansley <kieranm> |
Mon 09 Jul 2007 12:50:27 PM UTC, comment #10: Imagine you are sending 500 byte chunks, have no unsent queue and some segments in unacked (with TCP_MSS of 1460):
|
Simon Goldschmidt <goldsimon> |
Mon 09 Jul 2007 12:43:55 PM UTC, comment #9: Can you give a little more information (to save me having to think about it too hard!) to why this extra nagle code is necessary in tcp_output? Is it that we enqueue code when not nagle constrained, and then try and output it when we should be nagle constrained? |
Kieran Mansley <kieranm> |
Fri 29 Jun 2007 07:17:32 PM UTC, comment #8: Seems ok for me...
|
Frédéric Bernon <fbernon> |
Fri 29 Jun 2007 05:49:10 PM UTC, comment #7: - As a first fix, I've added a check that snd_queuelen doesn't overflow. I'm still checking if it needs to be converted to u16_t.
|
Simon Goldschmidt <goldsimon> |
Thu 28 Jun 2007 07:55:38 PM UTC, comment #6: The new algorithm performs much better.
|
Simon Goldschmidt <goldsimon> |
Thu 28 Jun 2007 12:11:04 PM UTC, comment #5: Sorry, unsent of course! (in the patch, it's correct!)
|
Simon Goldschmidt <goldsimon> |
Thu 28 Jun 2007 11:20:15 AM UTC, comment #4: I would understand it if you wanted to send if pcb->unsent contains more than one segment, but I don't understand the significance of pcb->unacked having more than one segment.
|
Kieran Mansley <kieranm> |
Thu 28 Jun 2007 11:07:18 AM UTC, comment #3: Sorry, my phrase was a little easy to misunderstand.
|
Simon Goldschmidt <goldsimon> |
Thu 28 Jun 2007 10:50:44 AM UTC, comment #2: From your description I think you've slightly misunderstood the nagle algorithm (or made a typo in your description!) I've not looked at the patch to see which it is, but as I understand nagle can be summarised as:
|
Kieran Mansley <kieranm> |
Thu 28 Jun 2007 10:42:04 AM UTC, comment #1: Attaching a simple patch as proposed. |
Simon Goldschmidt <goldsimon> |
Thu 28 Jun 2007 10:40:41 AM UTC, original submission:
Our implementation TCP nagle algorithm (tcp_output_nagle, previously included in netconn_write/do_write), sends if pcb->snd_queuelen > 1.
|
Simon Goldschmidt <goldsimon> |
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 6 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2007-11-21 | goldsimon | Status | None | Fixed | |
Open/Closed | Open | Closed | |||
2007-11-11 | goldsimon | Attached File | - | Added tcp_nagle2.patch, #14360 | |
2007-10-05 | goldsimon | Attached File | - | Added tcp_nagle.patch, #14094 | |
2007-07-13 | kieranm | Planned Release | 1.3.0 | ||
2007-06-28 | goldsimon | Attached File | - | Added tcp_output_nagle.patch, #13193 |
Powered by Savane 3.13-4448.
Corresponding source code
I regard this as fixed, please don't hesitate to reopen it if you find errors/problems!