-
Notifications
You must be signed in to change notification settings - Fork 2.1k
sys/net/gcoap: Increase default GCOAP_PDU_BUF_SIZE #16377
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
Conversation
The number is from RFC7252 Secion 4.6. It is increased to ensure that any to-be-expected-sized message can be received, and to avoid arbitrary limits. Applications can still downsize the buffer if they run into constraints, provided they know that their applications will never send such large requests. Mitigates: RIOT-OS#14167
Why don't you just put May be the cleaner solution would be to ask gcoap for one of its resend_buffers (as they are already buffers to be used for sending and some messages are copied there anyway) |
Ping @chrysn? |
Going for |
I guess if it is well documented in the code, it is ok. Better show new contributors how to properly allocate buffers instead of having to tweak the stack size every time we need a bigger payload ;-). |
…ocation examples/gcoap: Allocate the send buffer statically
A static in the example is fine with me; pushed as a squash commit. (Failing check-commits checks can be resolved at rebase time). I do have a different proposal for resolving #14167 too, but this here is the straightforward one, and the other (which doesn't even have a draft PR yet) can still demo how to use CoAP with smaller buffer sizes by tuning it down -- but unless users implement handlers carefully and with awareness of the limitations, a full message size is the IMO the better default. |
Mhhhh I am guessing that other proposal could tap into #16715's potential? ^^ |
Kind of. The other proposal would introduce a new memo state ("you got a message, but it's truncated"), which a full #16715 solution would probably need to do proper auto-slicing. But it also depends on sock_udp_recv_buf and that's still experimental. |
Correction: The other proposal does have a PR, #16378. Github just doesn't manage to find PRs based on the branch name facepalm. (I know gerrit isn't perfect, but). There has been quite some activity I've failed to follow up on as well, continuing the sprint day there... |
I'm withdrawing this on grounds of #16378 being merged. A small buffer will usually be sufficient, applications can now work provided all involved CoAP peers are well-behaved, and increasing this has become purely a performance trade-off thing. (And I personally prefer to have things that break early when the peer is not top notch so it can be fixed, rather than having things fall apart subtly when performance parameters are tuned). |
Contribution description
This bumps the default gcoap receive and retransmit buffer size from 128 to about 1k; questions about this come in again and again, quoting IRC:
In most cases, GCOAP_PDU_BUF_SIZE buffers are only ever allocated statically. The one exception is the gcoap shell application where there's a stack allocated buffer as well, and the default stack size has been increased there.
Testing procedure
aiocoap-client 'coap://[fe80::3c63:beff:fe85:ca96%tapbr0]/' -m PUT --payload @README.rst
; before this, it'll just retransmit and time out; after, it'll just return 4.04 as it should.This demonstrates how Gcoap drops long packages instead of gracefully erring out #14167 is worked around.
coap get -c 2a01:4f8:190:3064::5 5683 /.well-known/core
(But primarily, I'm interested if any builds now overflow the RAM in the CI tests)
Issues/PRs references
While this does not resolve #14167, it mitigates it -- we can now receive reasonably sized packages, ie. those that are sent by a client that does not do path MTU discovery.
It doesn't look like any other resolution is imminent, and the behavior resulting from large requests and small buffers is confusing to newcomers.
Open questions