-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use platform alloc & intrusive lists for managing MinMdns responders #16181
Use platform alloc & intrusive lists for managing MinMdns responders #16181
Conversation
PR #16181: Size comparison from 6e76729 to 9b32c96 Increases (8 builds for esp32, linux, mbed, p6)
Decreases (8 builds for esp32, linux, mbed, p6)
Full report (18 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
PR #16181: Size comparison from e5c5a9e to b726abc Increases above 0.2%:
Increases (19 builds for esp32, linux, mbed, nrfconnect, p6, telink)
Decreases (17 builds for esp32, linux, mbed, p6)
Full report (27 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
PR #16181: Size comparison from fea74e6 to 1bdfa49 Increases above 0.2%:
Increases (17 builds for esp32, linux, mbed, p6)
Decreases (17 builds for esp32, linux, mbed, p6)
Full report (27 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
Would you mind just opening a quick issue to track the discussion on whether a static/dynamic configurable pool would be workable here? It might not, but if we could replace the platform New with an option to have static allocation it might be beneficial for some platforms so they don't have to worry about running out of heap and being unable to support the required number of fabrics. |
This reverts commit c4d3f6c.
PR #16181: Size comparison from fea74e6 to 7d12029 Increases above 0.2%:
Increases (27 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (17 builds for esp32, linux, mbed, p6)
Full report (27 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
…roject-chip#16181) * Initial version: use an intrusive list for query responders * Updated some tests * Mark no upper limit on advertisement * Remove TODO comment - I did it * Simplified iterators - shorter code * Add log and return error if query responder addition fails * Fix back the test: out of memory is from the advertiser not allocators * Updated comment * Remove LogErrorOnFailure * Review comments applied - typo/spacing fixed * Enable DNSSD on ipv4 if broadcast is available * Ensure minmdns cleans up its lists at shutdown * Revert "Enable DNSSD on ipv4 if broadcast is available" This reverts commit c4d3f6c.
Problem
We place an upper limit on number of fabrics supported for operational functionality by statically allocating some responders. this has several drawbacks:
Note that Responders are still limited and changing that will be done in a separate PR.
Part of #8000, however note that altough advertisers themselves can be created now, they will not be registered for 'response' until a followup PR is made.
Change overview
Create a intrusive list holder for advertiser allocators, use that instead.
Testing
ran minmdns server/client demo and it advertised
updated and ran unit tests
CI will heavily test commissioning and connecting side which exercises minmdns on linux.