lwIP - A Lightweight TCP/IP stack - Bugs: bug #28054, Two segments with FIN flag on the...
You are not allowed to post comments on this tracker with your current authentication level.
bug #28054: Two segments with FIN flag on the out-of-sequence queue
Submitter: | Oleg Tyshev <olegreen> | ||
Submitted: | Thu 19 Nov 2009 04:55:13 PM UTC | ||
Category: | TCP | Severity: | 3 - Normal |
Item Group: | Faulty Behaviour | Status: | Fixed |
Privacy: | Public | Assigned to: | goldsimon |
Open/Closed: | Closed | Planned Release: | None |
lwIP version: | CVS Head |
Jump to the original submission
Sun 29 Nov 2009 01:25:09 PM UTC, comment #17: |
Simon Goldschmidt <goldsimon> |
Fri 27 Nov 2009 08:05:25 AM UTC, comment #16: The unit tests I just checked in seem to confirm the patch works (although not all clauses of the code addin ooseq segments are covered, yet).
|
Simon Goldschmidt <goldsimon> |
Mon 23 Nov 2009 05:40:45 PM UTC, comment #15: Patch again corrected.
|
Oleg Tyshev <olegreen> |
Mon 23 Nov 2009 03:11:15 PM UTC, comment #14: I'll take a look at the patch, and if I think it's safe I'll try and get it in to 1.3.2 |
Kieran Mansley <kieranm> |
Mon 23 Nov 2009 03:08:03 PM UTC, comment #13: The patch looks good to me. I also like the separation into smaller functions, it would do the rest of the code good, too, since it gets more readable. And at least MSVC and gcc seem to inline static functions quite well, which means that code size isn't an argument here.
|
Simon Goldschmidt <goldsimon> |
Mon 23 Nov 2009 01:48:03 PM UTC, comment #12: In this patch common part for insertion packet into
|
Oleg Tyshev <olegreen> |
Fri 20 Nov 2009 06:27:45 PM UTC, comment #11: There are 4 almost the same come fragments:
|
Oleg Tyshev <olegreen> |
Fri 20 Nov 2009 05:55:14 PM UTC, comment #10: :)Patch |
Oleg Tyshev <olegreen> |
Fri 20 Nov 2009 05:54:35 PM UTC, comment #9: Sorry, I've made a lot of errors in deallocation.
|
Oleg Tyshev <olegreen> |
Fri 20 Nov 2009 05:05:00 PM UTC, comment #8: Selected lines do this work
|
Oleg Tyshev <olegreen> |
Fri 20 Nov 2009 04:59:34 PM UTC, comment #7: I'm guessing Oleg can test, and as it's clear that we've got a bug in this situation we can only make it better! |
Kieran Mansley <kieranm> |
Fri 20 Nov 2009 04:50:43 PM UTC, comment #6:
|
Simon Goldschmidt <goldsimon> |
Fri 20 Nov 2009 04:41:08 PM UTC, comment #5: If we add to the oos-queue packet with FIN flag,
|
Oleg Tyshev <olegreen> |
Fri 20 Nov 2009 04:23:15 PM UTC, comment #4: Why does the patch test for FIN? I think this problem would also be solved if we would always try to remove a second segment instead of trimming the first (if the last seqno is the same).
|
Simon Goldschmidt <goldsimon> |
Fri 20 Nov 2009 04:12:04 PM UTC, comment #3: No, we shouldn't remove FIN flag.
|
Oleg Tyshev <olegreen> |
Fri 20 Nov 2009 03:42:25 PM UTC, comment #2: From looking at the code it looks like we do the right thing when the second segment is in order - there is lots of checking of TCP flags there - but ignore them completely when trimming if the second segment is also out of order. Whenever we trim a segment in that section (tcp_in.c:1244 -> 1363) we need to check the flags and removing any FIN if present. |
Kieran Mansley <kieranm> |
Fri 20 Nov 2009 03:36:30 PM UTC, comment #1: If anyone can propose a simple fix for this in the next couple of days this could go into 1.3.2 |
Kieran Mansley <kieranm> |
Thu 19 Nov 2009 04:55:13 PM UTC, original submission:
Suppouse we have on the oos_queue one segment
|
Oleg Tyshev <olegreen> |
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 7 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2009-11-29 | goldsimon | Open/Closed | Open | Closed | |
2009-11-29 | goldsimon | Assigned to | None | goldsimon | |
Status | None | Fixed | |||
2009-11-23 | olegreen | Attached File | - | Added oos_fin.patch, #19118 | |
2009-11-23 | olegreen | Attached File | - | Added oos_fin.patch, #19117 | |
2009-11-20 | olegreen | Attached File | - | Added oos_fin.patch, #19095 | |
2009-11-20 | olegreen | Attached File | - | Added oos_fin.patch, #19092 |
Powered by Savane 3.13-caa5.
Corresponding source code
I reviewed the patch and adapted the out-of-sequence unit tests to cover nearly all the out-of-sequence code.
After these are working successfully (found another bug there where segments were freed but pbufs weren't), I've checked in the patch.
Kieran, I hope you don't mind me taking over here (because of comment #14) :-)