lwIP - A Lightweight TCP/IP stack - Bugs: bug #52763, mdns: move data off the stack and...
You are not allowed to post comments on this tracker with your current authentication level.
bug #52763: mdns: move data off the stack and dynamically allocate data.
Submitter: | Douglas <ourairquality> | ||
Submitted: | Sat 30 Dec 2017 03:25:45 AM UTC | ||
Category: | DNS | Severity: | 3 - Normal |
Item Group: | None | Status: | None |
Privacy: | Public | Assigned to: | None |
Open/Closed: | Closed | Planned Release: | None |
lwIP version: | git head |
Jump to the original submission
Sun 07 Jan 2018 01:26:25 PM UTC, comment #16: |
Douglas <ourairquality> |
Thu 04 Jan 2018 07:04:22 AM UTC, comment #15: This is more or less a duplicate to bug #48953.
|
Simon Goldschmidt <goldsimon> |
Wed 03 Jan 2018 06:57:52 PM UTC, comment #14: Reading the lasts posts, we could probably close this bug as duplicate as we already have bug #48953 to remind us that stack usage in mdns is too high.
|
Simon Goldschmidt <goldsimon> |
Wed 03 Jan 2018 04:00:50 PM UTC, comment #13:
|
Douglas <ourairquality> |
Wed 03 Jan 2018 03:45:32 PM UTC, comment #12:
|
Douglas <ourairquality> |
Wed 03 Jan 2018 03:21:24 PM UTC, comment #11: Adding even more unrelated code into the same patchset is not the way forward. Our time matters as well, and we want to make sure that the new code works. At some point it will be easier for me to just do the changes myself than to review this.
|
Erik Ekman <yarrick> |
Wed 03 Jan 2018 02:44:16 PM UTC, comment #10: Up to now, worst case calculation was deliberately used in lwIP to prevent "random" allocation failures when the heap gets empty. That's why we have pools. And you can use the heap to emulate the pools if you want to. However, I'm not comfortable with using mem_malloc all over the place. We have to let people configure the allocation scheme. You're the best example for that. But you can't enforce your allocaton scheme to all users of lwip. We have to find a way that fits all.
|
Simon Goldschmidt <goldsimon> |
Wed 03 Jan 2018 02:10:46 PM UTC, comment #9: Thank you for the feedback, some of the issues caught are good to have fixed. The rebased patch combines all the fixes, even the reverted support for delaying responses. I doubt there are the resources to slice this all up sorry, so could we try to move forward.
|
Douglas <ourairquality> |
Wed 03 Jan 2018 12:42:59 PM UTC, comment #8: Sorry, we reverted the async send commit, so your patch is out of date again.
|
Erik Ekman <yarrick> |
Wed 03 Jan 2018 03:16:18 AM UTC, comment #7: Rebased around the other improvements, and thank you for the help.
|
Douglas <ourairquality> |
Sun 31 Dec 2017 03:03:16 PM UTC, comment #6:
|
Douglas <ourairquality> |
Sun 31 Dec 2017 01:19:09 PM UTC, comment #5:
|
Simon Goldschmidt <goldsimon> |
Sun 31 Dec 2017 12:38:51 PM UTC, comment #4: Nice to see progress. Some more comments:
|
Erik Ekman <yarrick> |
Sun 31 Dec 2017 10:09:16 AM UTC, comment #3: Thank you for all the feedback, and an attempt has been made to address it all.
|
Douglas <ourairquality> |
Sat 30 Dec 2017 03:13:15 PM UTC, comment #2: In mdns_domain_alloc(), mem_calloc already clears the memory so the memset is not required.
|
Erik Ekman <yarrick> |
Sat 30 Dec 2017 03:07:20 PM UTC, comment #1: Can we simplify the cleanup code in the build domain functions?
|
Erik Ekman <yarrick> |
Sat 30 Dec 2017 03:25:45 AM UTC, original submission:
The mdns responder uses a lot of stack for a small system, well over 1k, which is out of proportion to the general stack resource needs for a small system. This is largely due to stack allocated fixed sized 256 byte buffers for domain names. This patch largely moves data off the stack and dynamically allocates it.
|
Douglas <ourairquality> |
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 |
---|---|---|---|---|---|
2018-01-07 | ourairquality | Attached File | - | Added 0001-mdns-move-data-off-the-stack-and-dynamically-allocat.patch, #42864 | |
2018-01-04 | goldsimon | Open/Closed | Open | Closed | |
2018-01-03 | ourairquality | Attached File | - | Added 0001-mdns-move-data-off-the-stack-and-dynamically-allocat.patch, #42805 | |
2018-01-03 | ourairquality | Attached File | - | Added 0001-mdns-move-data-off-the-stack-and-dynamically-allocat.patch, #42793 | |
2017-12-31 | ourairquality | Attached File | - | Added 0001-mdns-move-data-off-the-stack-and-dynamically-allocat.patch, #42773 | |
2017-12-31 | ourairquality | Attached File | - | Added 0001-mdns-move-data-off-the-stack-and-dynamically-allocat.patch, #42771 | |
2017-12-30 | ourairquality | Attached File | - | Added 0001-mdns-move-data-off-the-stack-and-dynamically-allocat.patch, #42761 |
Powered by Savane 3.13-d3ae.
Corresponding source code
Just a small update, fwiw.
One idea that might be food for thought:
(file #42864)