lwIP - A Lightweight TCP/IP stack - Tasks: task #6861, Pimp ip_frag.c
You are not allowed to post comments on this tracker with your current authentication level.
task #6861: Pimp ip_frag.c
Submitter: | Simon Goldschmidt <goldsimon> | ||
Submitted: | Wed 09 May 2007 07:27:13 AM UTC | ||
Category: | None | Should Start On: | Wed 09 May 2007 12:00:00 AM UTC |
Should be Finished on: | Sun 05 Aug 2007 12:00:00 AM UTC | Priority: | 1 - Later |
Status: | In Progress | Privacy: | Public |
Assigned to: | taranowski | Percent Complete: | 100% |
Open/Closed: | Closed | Planned Release: | 1.3.0 |
Effort: | 0.00 |
Jump to the original submission
Thu 18 Oct 2007 08:14:34 PM UTC, comment #41: |
Simon Goldschmidt <goldsimon> |
Wed 17 Oct 2007 08:19:11 PM UTC, comment #40: Thanks for checking in those modifications Simon. I don't have check-in permissions, hence the post-the-patch approach. I should have some time in a week or two to test out Simon's modifications.
|
Thomas Taranowski <taranowski> |
Thu 11 Oct 2007 07:51:47 PM UTC, comment #39: I have included code to free the oldest packets (not the currently assembled, however) until enough pbufs are freed to enqueue the new fragment. This is also done if MEMP_REASSDATA is empty.
|
Simon Goldschmidt <goldsimon> |
Wed 10 Oct 2007 01:17:43 PM UTC, comment #38:
|
Simon Goldschmidt <goldsimon> |
Wed 10 Oct 2007 12:09:00 PM UTC, comment #37:
|
Jonathan Larmour <jifl> |
Tue 09 Oct 2007 07:34:05 PM UTC, comment #36:
|
Simon Goldschmidt <goldsimon> |
Tue 09 Oct 2007 06:15:39 AM UTC, comment #35:
|
Simon Goldschmidt <goldsimon> |
Mon 08 Oct 2007 02:37:27 PM UTC, comment #34: Hmm, Thomas marked it as done, but I don't know what his status on this task is...
|
Simon Goldschmidt <goldsimon> |
Mon 08 Oct 2007 01:57:00 PM UTC, comment #33: Thanks for this Simon. I see the solution to using IP_HLEN or not is to just drop it if there are options - that's fair enough :). At least it's obvious what's going on.
|
Jonathan Larmour <jifl> |
Mon 08 Oct 2007 07:40:54 AM UTC, comment #32: I add a default value in opt.h for MEMP_NUM_REASSDATA (but it need to be comment and tune), so, now, I can build. But I got this warning :
|
Frédéric Bernon <fbernon> |
Mon 08 Oct 2007 07:28:33 AM UTC, comment #31: It seems there is a build problem with last changes:
|
Frédéric Bernon <fbernon> |
Sun 07 Oct 2007 11:54:43 AM UTC, comment #30: I just have added checks for overlapping and duplicate fragments. These are thrown away to prevent blocking of PBUF_POOL pbufs when they aren't needed. The checks are not big (4 if-statements), but still can be ifdef'ed out until we decide we really want them in. |
Simon Goldschmidt <goldsimon> |
Sat 06 Oct 2007 03:31:19 PM UTC, comment #29: OH, I forgot: if noone has any comments or change requests for this, I'd consider it as really done now and would close it! |
Simon Goldschmidt <goldsimon> |
Sat 06 Oct 2007 03:29:48 PM UTC, comment #28: After waiting for some time now, I ported Thomas' patch to the newest CVS HEAD and checked it in with some changes:
|
Simon Goldschmidt <goldsimon> |
Thu 09 Aug 2007 02:30:36 PM UTC, comment #27: I didn't get round to replying to Thomas either. First, thanks for your further efforts on this Thomas.
|
Jonathan Larmour <jifl> |
Thu 09 Aug 2007 02:13:39 PM UTC, comment #26: Since I'm currently only checking my emails (not at home yet), I can't really look at the code. But some comments anyway:
|
Simon Goldschmidt <goldsimon> |
Fri 03 Aug 2007 08:10:48 PM UTC, comment #25: Thanks for the careful review Jonathan...
|
Thomas Taranowski <taranowski> |
Fri 03 Aug 2007 03:45:58 PM UTC, comment #24: I should just mention that I was commenting on the state of the file now, not only Thomas' changes - some of these comments apply to what was there before it was p1mp3d. Thanks for being a p1mp, Thonmas ;). |
Jonathan Larmour <jifl> |
Fri 03 Aug 2007 03:41:11 PM UTC, comment #23: Thanks Thomas.
|
Jonathan Larmour <jifl> |
Fri 03 Aug 2007 06:52:24 AM UTC, comment #22: I've finished debugging/reworking Simon's patch, and it be p1mp3d. I've done some testing, and it works reliably. I attached the patchfile for the ip_frag.c file. The other parts of Simon's original patch are unmodified. |
Thomas Taranowski <taranowski> |
Thu 26 Jul 2007 03:15:23 PM UTC, comment #21: I think this is something we have to cope with. Just waiting for a retransmission is no good as the retransmitted one may also overlap (there's a good chance of this). We'd throw that one away too, and all future retransmissions, and so never make any progress.
|
Kieran Mansley <kieranm> |
Thu 26 Jul 2007 02:46:47 PM UTC, comment #20: Re comment #19:
|
Simon Goldschmidt <goldsimon> |
Thu 26 Jul 2007 02:17:56 PM UTC, comment #19: From just visually looking at the code, it looks good. Just two minor thoughts though:
|
Jonathan Larmour <jifl> |
Thu 26 Jul 2007 01:58:26 PM UTC, comment #18:
|
Jonathan Larmour <jifl> |
Mon 23 Jul 2007 09:31:08 PM UTC, comment #17: Thanks Simon. I'll perform some testing on the patch this week. |
Thomas Taranowski <taranowski> |
Mon 23 Jul 2007 09:03:08 PM UTC, comment #16: I'm attaching a patch and the file ip_frag.c with my changes.
|
Simon Goldschmidt <goldsimon> |
Mon 23 Jul 2007 02:58:22 PM UTC, comment #15: I plan to be working the ip reassembly code pretty heavy this week, as I need some performance improvements for the short term. However, At the moment, I'm not interesting reinventing the wheel if Simon already has done some work on this. What's the current status on the design/implementation? |
Thomas Taranowski <taranowski> |
Fri 20 Jul 2007 05:44:18 PM UTC, comment #14:
|
Simon Goldschmidt <goldsimon> |
Fri 20 Jul 2007 03:16:31 PM UTC, comment #13: Hi folks. Sorry I've been out of touch - I was off on vacation for 3 weeks or so, and then returned to work to be dumped into major firefighting. I'm only now starting on the lwIP backlog that built up in the time. Anyway.....
|
Jonathan Larmour <jifl> |
Fri 13 Jul 2007 12:40:51 PM UTC, comment #12:
|
Simon Goldschmidt <goldsimon> |
Fri 13 Jul 2007 12:06:18 PM UTC, comment #11: Any update on this? I'm tempted to tag it as required for the 1.3.0 release, as it seems mostly done, but would like to make sure that's feasible before I do. |
Kieran Mansley <kieranm> |
Tue 26 Jun 2007 03:46:31 PM UTC, comment #10: There are 2 patches:
|
Simon Goldschmidt <goldsimon> |
Tue 26 Jun 2007 03:36:52 PM UTC, comment #9: Do you mean use a single PBUF_RAM as a static buffer for the whole reassembled packet? I thought that this patch was trying to move away from a single static buffer.
|
Kieran Mansley <kieranm> |
Tue 26 Jun 2007 03:28:10 PM UTC, comment #8: Why can't we allocat a PBUF_RAM in ip_reass_init() (or ip_frag_init :) and use that as the static buffer? That way we wouldn't have to allocate a PBUF_REF (for data and IP header) and a PBUF_RAM (for link header) but simply could reuse the single PBUF_RAM all the time.
|
Simon Goldschmidt <goldsimon> |
Sun 03 Jun 2007 06:37:34 PM UTC, comment #7: Jonathan, I tried to integrate your patch and get a compiler warning that iphdr is accessed uninitialized on line 334 (for your version based on v1.1.1).
|
Simon Goldschmidt <goldsimon> |
Sat 02 Jun 2007 05:15:27 PM UTC, comment #6: I've just 'stumbled' accross ip_frag_init(). Since we have different options for frag and reass, and ip_frag_init() really initialzes the reassembly variables, shouldn't it be named ip_reass_init()???
|
Simon Goldschmidt <goldsimon> |
Wed 09 May 2007 11:50:21 AM UTC, comment #5: Good, I got permission. I'm attaching the fragmentation patch against 1.1. It does not presently apply cleanly against CVS and I haven't updated it, but you're going to be changing even more stuff in this area anyway.
*/
|
Jonathan Larmour <jifl> |
Wed 09 May 2007 09:55:10 AM UTC, comment #4: My suggestion would be to hold PBUF_POOLs temporarily to reassemble them later. Let's say I'd allow a maximum of 10 packets consisting of PBUF_POOLs (of tot_len = maximum MTU). I would then allocate those pbuf chains in ip_frag_init() and free one each time I put a new packet in the reassembly-list. That way I guarantee the amount of pbufs in the pool stays the same while still avoiding 2*memcpy() for each reassembled packet (which, in my opinion, fits even better to the name "lwIP" :) |
Simon Goldschmidt <goldsimon> |
Wed 09 May 2007 09:46:59 AM UTC, comment #3: I'm quite concerned about the risk of running out of PBUFs. Swapping a problem of it not performing well (because it can't do much reassembly, and so more retransmissions are required) for one where the stack grinds to a halt because its run out of buffers wouldn't be a good idea. If we can ensure that doesn't happen, I'd be happier to see this go in.
|
Kieran Mansley <kieranm> |
Wed 09 May 2007 09:43:06 AM UTC, comment #2:
|
Simon Goldschmidt <goldsimon> |
Wed 09 May 2007 09:33:06 AM UTC, comment #1: I have a patch against lwIP 1.1 to allow ip_frag to either use a static buffer (like the current implementation) or create a sequence of PBUF_REFs pointing into the payload of the original buffer.
|
Jonathan Larmour <jifl> |
Wed 09 May 2007 07:27:13 AM UTC, original submission:
- segments could be "queued" as pbufs so no copy is needed (memcpy to reassbuf could be avoided)
|
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 16 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2007-10-18 | goldsimon | Percent Complete | 80% | 100% | |
Open/Closed | Open | Closed | |||
2007-10-11 | goldsimon | Percent Complete | 60% | 80% | |
2007-10-09 | goldsimon | Status | Done | In Progress | |
Percent Complete | 0% | 60% | |||
2007-08-03 | taranowski | Attached File | - | Added ip_frag.c.detabbed_and_static, #13574 | |
2007-08-03 | taranowski | Status | Ready For Test | Done | |
2007-08-03 | taranowski | Attached File | - | Added ip_frag.c.patch_redeux, #13563 | |
Attached File | - | Added ip_frag.c, #13564 | |||
2007-07-27 | taranowski | Should be Finished on | 2007-05-09 | 2007-08-05 | |
2007-07-27 | taranowski | Assigned to | None | taranowski | |
2007-07-23 | goldsimon | Attached File | - | Added ip_frag.c, #13444 | |
Attached File | - | Added ipreass_queue.patch, #13445 | |||
2007-07-13 | goldsimon | Planned Release | None | 1.3.0 | |
2007-06-03 | goldsimon | Status | None | Ready For Test | |
2007-05-09 | jifl | Attached File | - | Added lwip-ipfrag-pbuf.patch, #12728 |
Powered by Savane 3.13-d3ae.
Corresponding source code
Oh, OK! I didn't know that. Didn't know you could assign a task but don't have check-in permissions.
Anyway, since v1.43 I think it's 100% complete so could be marked as done. Only didn't update this task, yet.
So I'll close it as done. Reopen it if you find any problems.