pkg/tinydtls: Adjust defaults #19331

chrysn · 2023-02-27T21:19:25Z

Contribution description

This adjusts two defaults in tinydtls:

Default verbosity is set to warning. At the info level, this module produces way more output (several lines per new connection, and even per message) than is common in RIOT.
If gcoap is used, the buffer size is adjusted to the gcoap buffer size plus overhead. Otherwise, CoAP-over-DTLS works fine until one happens to request larger resources.

Testing procedure

Run examples/gcoap_dtls
Send a CoAP request from outside, eg. with aiocoap-client 'coaps://[fe80::3c63:beff:fe85:ca96%tapbr0]/.well-known/core' --credentials testserver.json (where testserver.json is {"coaps://[fe80::3c63:beff:fe85:ca96%tapbr0]/*": {"dtls": {"psk": {"ascii": "secretPSK"}, "client-identity": {"ascii": "Client_identity"}}}}).

Before, there are messages shown for every request; now there are none.

Modify examples/gcoap/server.c as follows:

diff --git a/examples/gcoap/server.c b/examples/gcoap/server.c
index bf2315cd01..28e1faac27 100644
--- a/examples/gcoap/server.c
+++ b/examples/gcoap/server.c
@@ -68,7 +68,7 @@ static const coap_resource_t _resources[] = {
 };
 
 static const char *_link_params[] = {
-    ";ct=0;rt=\"count\";obs",
+    ";ct=0;rt=\"count\";obs;looooooooooooooooooooooong-attribute=\"loooooooooooooooooooooooooooooong\"",
     NULL
 };

The request passes; without this patch, it is stuck in retransmissions until "Network error: Retransmissions exceeded".

Issues/PRs references

This contributes to making #19289 usable with a minimum level of security. (That module fills up the gcoap buffer to the brim). While the module handles the verbosity as well as it can (occasionally admitting that it lost bytes of output), the previous verbosity produces an infinite stream of stdout data. (But the default should be quiet immaterial of that particular PR).

riot-ci · 2023-02-27T21:53:09Z

Murdock results

✔️ PASSED

f2d5928 dtls, tinydtls: Raise default number of connections

Success	Failures	Total	Runtime
6902	0	6902	13m:29s

Artifacts

Documentation preview

sys/include/net/dtls.h

chrysn · 2023-02-28T15:12:13Z

I've added a third change: The default number of DTLS sessions is raised from 1 to 2.

As long as this is 1 and Gcoap is used (OK, maybe we'd limit the default to 2 for gcoap), long-running DTLS sessions are impossible otherwise -- and the DTLS handshake is relatively heavy in terms of traffic and energy.

[edit: conditionally on using GCOAP_DTLS, see #19331 (comment) ]

chrysn · 2023-02-28T15:57:18Z

The peer-max thing is not catching it without Kconfig, as that parameter (while defined with a default in dtls.h) needs to be passed through the Makefile to take effect ... ugh. Let's see if I can fix that.

pkg/tinydtls/Makefile.include

chrysn · 2023-02-28T18:30:58Z

Can I squash? (I'd reduce to three commits for the individual topics).

miri64

ACK, trusting your testing and mostly this should be problematic in compilation :-).

miri64 · 2023-02-28T19:06:04Z

bors merge

bors · 2023-02-28T19:06:08Z

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

miri64 · 2023-02-28T19:16:59Z

bors cancel
bors merge

bors · 2023-02-28T19:17:02Z

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

miri64 · 2023-02-28T19:18:56Z

Ah, Murdock did not rerun yet (but is also not shown in the statuses for some reason...)

benpicco · 2023-02-28T20:06:20Z

bors merge

miri64 · 2023-02-28T20:06:35Z

bors cancel
bors merge

bors · 2023-02-28T20:06:38Z

Canceled.

16158: gnrc_sixlowpan_frag_sfr_congure_sfr: initial import r=miri64 a=miri64 19331: pkg/tinydtls: Adjust defaults r=miri64 a=chrysn ### Contribution description This adjusts two defaults in tinydtls: * Default verbosity is set to warning. At the info level, this module produces way more output (several lines per new connection, and even per message) than is common in RIOT. * If gcoap is used, the buffer size is adjusted to the gcoap buffer size plus overhead. Otherwise, CoAP-over-DTLS works fine until one happens to request larger resources. ### Testing procedure * Run examples/gcoap_dtls * Send a CoAP request from outside, eg. with `aiocoap-client 'coaps://[fe80::3c63:beff:fe85:ca96%tapbr0]/.well-known/core' --credentials testserver.json` (where testserver.json is `{"coaps://[fe80::3c63:beff:fe85:ca96%tapbr0]/*": {"dtls": {"psk": {"ascii": "secretPSK"}, "client-identity": {"ascii": "Client_identity"}}}}`). Before, there are messages shown for every request; now there are none. Modify `examples/gcoap/server.c` as follows: ```patch diff --git a/examples/gcoap/server.c b/examples/gcoap/server.c index bf2315cd01..28e1faac27 100644 --- a/examples/gcoap/server.c +++ b/examples/gcoap/server.c `@@` -68,7 +68,7 `@@` static const coap_resource_t _resources[] = { }; static const char *_link_params[] = { - ";ct=0;rt=\"count\";obs", + ";ct=0;rt=\"count\";obs;looooooooooooooooooooooong-attribute=\"loooooooooooooooooooooooooooooong\"", NULL }; ``` The request passes; without this patch, it is stuck in retransmissions until "Network error: Retransmissions exceeded". ### Issues/PRs references This contributes to making #19289 usable with a minimum level of security. (That module fills up the gcoap buffer to the brim). While the module handles the verbosity as well as it can (occasionally admitting that it lost bytes of output), the previous verbosity produces an infinite stream of stdout data. (But the default should be quiet immaterial of that particular PR). Co-authored-by: Martine Lenders <m.lenders@fu-berlin.de> Co-authored-by: chrysn <chrysn@fsfe.org>

bors · 2023-02-28T20:44:07Z

Build failed (retrying...):

Murdock

benpicco · 2023-02-28T20:50:16Z

pkg/tinydtls/Makefile.include

+# TinyDTLS emits several messages during connection establishment at the info
+# level; this is way more verbose than common in RIOT.
+TINYDTLS_LOG_LEVEL ?= LOG_WARNING
+CFLAGS += -DLOG_LEVEL=$(TINYDTLS_LOG_LEVEL)


You might have to set this on the pkg Makefile instead so it’s only set for this pkg

Good point, thanks.

bors cancel

I've moved all newly introduced sections from Makefile.include to Makefile (affecting default verbosity and buffer size selection).

Running the tests again confirms that both are still picked up (ie. the relevant information about the gcoap module is also around there).

bors · 2023-02-28T20:52:00Z

Canceled.

benpicco · 2023-03-02T12:13:01Z

bors merge

19331: pkg/tinydtls: Adjust defaults r=benpicco a=chrysn ### Contribution description This adjusts two defaults in tinydtls: * Default verbosity is set to warning. At the info level, this module produces way more output (several lines per new connection, and even per message) than is common in RIOT. * If gcoap is used, the buffer size is adjusted to the gcoap buffer size plus overhead. Otherwise, CoAP-over-DTLS works fine until one happens to request larger resources. ### Testing procedure * Run examples/gcoap_dtls * Send a CoAP request from outside, eg. with `aiocoap-client 'coaps://[fe80::3c63:beff:fe85:ca96%tapbr0]/.well-known/core' --credentials testserver.json` (where testserver.json is `{"coaps://[fe80::3c63:beff:fe85:ca96%tapbr0]/*": {"dtls": {"psk": {"ascii": "secretPSK"}, "client-identity": {"ascii": "Client_identity"}}}}`). Before, there are messages shown for every request; now there are none. Modify `examples/gcoap/server.c` as follows: ```patch diff --git a/examples/gcoap/server.c b/examples/gcoap/server.c index bf2315cd01..28e1faac27 100644 --- a/examples/gcoap/server.c +++ b/examples/gcoap/server.c `@@` -68,7 +68,7 `@@` static const coap_resource_t _resources[] = { }; static const char *_link_params[] = { - ";ct=0;rt=\"count\";obs", + ";ct=0;rt=\"count\";obs;looooooooooooooooooooooong-attribute=\"loooooooooooooooooooooooooooooong\"", NULL }; ``` The request passes; without this patch, it is stuck in retransmissions until "Network error: Retransmissions exceeded". ### Issues/PRs references This contributes to making #19289 usable with a minimum level of security. (That module fills up the gcoap buffer to the brim). While the module handles the verbosity as well as it can (occasionally admitting that it lost bytes of output), the previous verbosity produces an infinite stream of stdout data. (But the default should be quiet immaterial of that particular PR). Co-authored-by: chrysn <chrysn@fsfe.org>

miri64 · 2023-03-02T12:23:07Z

bors cancel
bors merge

bors · 2023-03-02T12:23:09Z

Canceled.

18515: libschc: initial import as package r=miri64 a=miri64 19331: pkg/tinydtls: Adjust defaults r=miri64 a=chrysn ### Contribution description This adjusts two defaults in tinydtls: * Default verbosity is set to warning. At the info level, this module produces way more output (several lines per new connection, and even per message) than is common in RIOT. * If gcoap is used, the buffer size is adjusted to the gcoap buffer size plus overhead. Otherwise, CoAP-over-DTLS works fine until one happens to request larger resources. ### Testing procedure * Run examples/gcoap_dtls * Send a CoAP request from outside, eg. with `aiocoap-client 'coaps://[fe80::3c63:beff:fe85:ca96%tapbr0]/.well-known/core' --credentials testserver.json` (where testserver.json is `{"coaps://[fe80::3c63:beff:fe85:ca96%tapbr0]/*": {"dtls": {"psk": {"ascii": "secretPSK"}, "client-identity": {"ascii": "Client_identity"}}}}`). Before, there are messages shown for every request; now there are none. Modify `examples/gcoap/server.c` as follows: ```patch diff --git a/examples/gcoap/server.c b/examples/gcoap/server.c index bf2315cd01..28e1faac27 100644 --- a/examples/gcoap/server.c +++ b/examples/gcoap/server.c `@@` -68,7 +68,7 `@@` static const coap_resource_t _resources[] = { }; static const char *_link_params[] = { - ";ct=0;rt=\"count\";obs", + ";ct=0;rt=\"count\";obs;looooooooooooooooooooooong-attribute=\"loooooooooooooooooooooooooooooong\"", NULL }; ``` The request passes; without this patch, it is stuck in retransmissions until "Network error: Retransmissions exceeded". ### Issues/PRs references This contributes to making #19289 usable with a minimum level of security. (That module fills up the gcoap buffer to the brim). While the module handles the verbosity as well as it can (occasionally admitting that it lost bytes of output), the previous verbosity produces an infinite stream of stdout data. (But the default should be quiet immaterial of that particular PR). Co-authored-by: Martine Lenders <m.lenders@fu-berlin.de> Co-authored-by: chrysn <chrysn@fsfe.org>

bors · 2023-03-02T12:39:17Z

Build failed (retrying...):

Murdock

bors · 2023-03-02T18:37:29Z

Build succeeded:

chrysn requested review from benpicco, aabadie and HendrikVE February 27, 2023 21:19

chrysn requested a review from leandrolanzieri as a code owner February 27, 2023 21:19

chrysn force-pushed the tinydtls-defaults branch from 97b428f to 8e3a266 Compare February 27, 2023 21:19

github-actions bot added the Area: pkg Area: External package ports label Feb 27, 2023

chrysn added Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: pkg Area: External package ports and removed Area: pkg Area: External package ports labels Feb 27, 2023

github-actions bot removed the Area: network Area: Networking label Feb 27, 2023

chrysn added the Area: network Area: Networking label Feb 27, 2023

benpicco requested a review from miri64 February 27, 2023 22:18

chrysn requested review from jia200x and MrKevinWeiss as code owners February 28, 2023 15:09

github-actions bot added Area: Kconfig Area: Kconfig integration Area: sys Area: System labels Feb 28, 2023

benpicco reviewed Feb 28, 2023

View reviewed changes

sys/include/net/dtls.h Show resolved Hide resolved

chrysn force-pushed the tinydtls-defaults branch from 72da1ff to f0e68aa Compare February 28, 2023 15:44

benpicco reviewed Feb 28, 2023

View reviewed changes

pkg/tinydtls/Makefile.include Outdated Show resolved Hide resolved

miri64 reviewed Feb 28, 2023

View reviewed changes

pkg/tinydtls/Makefile.include Outdated Show resolved Hide resolved

chrysn force-pushed the tinydtls-defaults branch from 42905b4 to 9e48b6e Compare February 28, 2023 18:52

miri64 approved these changes Feb 28, 2023

View reviewed changes

benpicco reviewed Feb 28, 2023

View reviewed changes

chrysn added 3 commits March 2, 2023 09:15

pkg/tinydtls: Set buffer size as required for gcoap

5d741b9

tinydtls: Reduce default verbosity

f7a83a7

dtls, tinydtls: Raise default number of connections

f2d5928

chrysn force-pushed the tinydtls-defaults branch from 9e48b6e to f2d5928 Compare March 2, 2023 08:17

bors bot merged commit b062640 into RIOT-OS:master Mar 2, 2023

MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pkg/tinydtls: Adjust defaults #19331

pkg/tinydtls: Adjust defaults #19331

chrysn commented Feb 27, 2023

riot-ci commented Feb 27, 2023 •

edited

Loading

chrysn commented Feb 28, 2023 •

edited

Loading

chrysn commented Feb 28, 2023

chrysn commented Feb 28, 2023

miri64 left a comment

miri64 commented Feb 28, 2023

bors bot commented Feb 28, 2023

miri64 commented Feb 28, 2023

bors bot commented Feb 28, 2023

miri64 commented Feb 28, 2023

benpicco commented Feb 28, 2023

miri64 commented Feb 28, 2023

bors bot commented Feb 28, 2023

bors bot commented Feb 28, 2023

benpicco Feb 28, 2023

chrysn Feb 28, 2023

chrysn Mar 2, 2023

bors bot commented Feb 28, 2023

benpicco commented Mar 2, 2023

miri64 commented Mar 2, 2023

bors bot commented Mar 2, 2023

bors bot commented Mar 2, 2023

bors bot commented Mar 2, 2023

pkg/tinydtls: Adjust defaults #19331

pkg/tinydtls: Adjust defaults #19331

Conversation

chrysn commented Feb 27, 2023

Contribution description

Testing procedure

Issues/PRs references

riot-ci commented Feb 27, 2023 • edited Loading

Murdock results

Artifacts

chrysn commented Feb 28, 2023 • edited Loading

chrysn commented Feb 28, 2023

chrysn commented Feb 28, 2023

miri64 left a comment

Choose a reason for hiding this comment

miri64 commented Feb 28, 2023

bors bot commented Feb 28, 2023

miri64 commented Feb 28, 2023

bors bot commented Feb 28, 2023

miri64 commented Feb 28, 2023

benpicco commented Feb 28, 2023

miri64 commented Feb 28, 2023

bors bot commented Feb 28, 2023

bors bot commented Feb 28, 2023

benpicco Feb 28, 2023

Choose a reason for hiding this comment

chrysn Feb 28, 2023

Choose a reason for hiding this comment

chrysn Mar 2, 2023

Choose a reason for hiding this comment

bors bot commented Feb 28, 2023

benpicco commented Mar 2, 2023

miri64 commented Mar 2, 2023

bors bot commented Mar 2, 2023

bors bot commented Mar 2, 2023

bors bot commented Mar 2, 2023

riot-ci commented Feb 27, 2023 •

edited

Loading

chrysn commented Feb 28, 2023 •

edited

Loading