Skip to content
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

examples/dtls-sock: Enable non 32-bit architectures #20196

Merged

Conversation

fzi-haxel
Copy link
Contributor

@fzi-haxel fzi-haxel commented Dec 19, 2023

Contribution description

Minor change:
Remove the unnecessary FEATURES_REQUIRED += arch_32bit filter from the dtls-sock example.
(Which the similar dtls-echo example also does not have.)

Testing procedure

Test the dtls-sock example with non 32-bit boards.
DISCLAIMER: I only compiled the example for all boards. I don't have an 8-bit board with enough memory to test this. If someone could test this, I would appreciate it.

@github-actions github-actions bot added the Area: examples Area: Example Applications label Dec 19, 2023
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 19, 2023
@riot-ci
Copy link

riot-ci commented Dec 19, 2023

Murdock results

✔️ PASSED

181f965 examples/dtls-sock: Bump stack size even more on AVR

Success Failures Total Runtime
16 0 17 01m:19s

Artifacts

fzi-haxel added a commit to fzi-haxel/RIOT that referenced this pull request Dec 19, 2023
@fzi-haxel
Copy link
Contributor Author

These are all 8-bit boards with enough memory that could be used with the dtls-sock example

  • atmega256rfr2-xpro
  • atxmega-a1u-xpro
  • atxmega-a1-xplained
  • avr-rss2
  • derfmega256

@OlegHahm
Copy link
Member

Does any of it come with a (supported) transceiver?

@fzi-haxel
Copy link
Contributor Author

fzi-haxel commented Dec 21, 2023

At least the atmega256rfr2-xpro, avr-rss2, and derfmega256 should have an at86rf2xx.
I think atxmega-a1u-xpro and atxmega-a1-xplained don't have an onboard transceiver.

This example should be similar to the dtls-echo example. Do we know if this has been tested for 8-bit and with what?

fzi-haxel added a commit to fzi-haxel/RIOT that referenced this pull request Dec 21, 2023
OlegHahm pushed a commit to OlegHahm/RIOT that referenced this pull request Dec 21, 2023
@benpicco
Copy link
Contributor

benpicco commented Dec 21, 2023

Well it always comes down to stack size. With the defaults on avr-rss2 I get

2023-12-21 22:48:32,683 # > dtlsc fe80::204:2519:1801:c8c5 test
2023-12-21 22:48:32,737 # scheduler(): stack overflow detected, pid=2
2023-12-21 22:48:32,740 # *** RIOT kernel panic:
2023-12-21 22:48:32,740 # STACK OVERFLOW
2023-12-21 22:48:32,740 # 
2023-12-21 22:48:32,740 # *** halted.

When I bump the stack size it's working:

2023-12-21 22:49:58,449 # > dtlsc fe80::204:2519:1801:c8c5 test
2023-12-21 22:49:58,541 # From [fe80::204:2519:1801:c8c5%6]:20220
2023-12-21 22:49:59,235 # Client got hint: Connection to server successful
2023-12-21 22:49:59,242 # Sent DTLS message
2023-12-21 22:49:59,263 # Received 4 bytes: "Terminating
--- a/examples/dtls-sock/Makefile
+++ b/examples/dtls-sock/Makefile
@@ -49,7 +46,7 @@ CFLAGS += -DDTLS_DEFAULT_PORT=$(DTLS_PORT)
 # CFLAGS += -DCONFIG_DTLS_DEBUG
 
 # FIXME: This is a temporary patch
-CFLAGS += -DTHREAD_STACKSIZE_MAIN=\(2*THREAD_STACKSIZE_LARGE\)
+CFLAGS += -DTHREAD_STACKSIZE_MAIN=\(3*THREAD_STACKSIZE_LARGE\)
 
 # Comment this out to disable code in RIOT that does safety checking
 # which is not needed in a production environment but helps in the

This is because on AVR THREAD_STACKSIZE_LARGE is just 1024 instead of 2048 bytes as on other architectures.

With 3072 bytes for main we actually still have plenty space left:

2023-12-21 22:53:53,275 # > ps
2023-12-21 22:53:53,284 # 	pid | name                 | state    Q | pri | stack  ( used) ( free) | base addr  | current     
2023-12-21 22:53:53,293 # 	  1 | idle                 | pending  Q |  15 |    192 (  134) (   58) |     0x2ef6 |     0x2f58 
2023-12-21 22:53:53,301 # 	  2 | main                 | running  Q |   7 |   3072 ( 2516) (  556) |     0x2fb6 |     0x3a9e 
2023-12-21 22:53:53,310 # 	  3 | 6lo                  | bl rx    _ |   3 |    448 (  190) (  258) |     0x5f32 |     0x6087 
2023-12-21 22:53:53,319 # 	  4 | ipv6                 | bl rx    _ |   4 |    448 (  256) (  192) |     0x3c8a |     0x3dc2 
2023-12-21 22:53:53,327 # 	  5 | udp                  | bl rx    _ |   5 |    192 (  150) (   42) |     0x6134 |     0x6189 
2023-12-21 22:53:53,336 # 	  6 | at86rf2xx            | bl anyfl _ |   2 |    520 (  224) (  296) |     0x4030 |     0x41cd 
2023-12-21 22:53:53,343 # 	    | SUM                  |            |     |   4872 ( 3470) ( 1402)

@fzi-haxel
Copy link
Contributor Author

Well it always comes down to stack size. With the defaults on avr-rss2 I get
...
When I bump the stack size it's working:

Thanks for testing. This explains the Makefile of the dtls-echo example, which contains

# FIXME: This is a temporary patch
# TinyDTLS <= 0.8.6 requires around 426 bytes in RAM.
CFLAGS += -DTHREAD_STACKSIZE_MAIN=\(3*THREAD_STACKSIZE_DEFAULT\)

Should I just increase the stack size to 3*THREAD_STACKSIZE_DEFAULT or do you have a better recommendation?

@maribu
Copy link
Member

maribu commented Dec 27, 2023

I used this to bump the stack size:

From c7c84f8a7d8d08e23895a550470314b1b04969f8 Mon Sep 17 00:00:00 2001
From: Marian Buschsieweke <marian.buschsieweke@posteo.net>
Date: Wed, 27 Dec 2023 21:40:31 +0100
Subject: [PATCH] examples/dtls-sock: Bump stack size even more on AVR

---
 examples/dtls-sock/Makefile | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/examples/dtls-sock/Makefile b/examples/dtls-sock/Makefile
index 3033e445b6..2337fe6dd2 100644
--- a/examples/dtls-sock/Makefile
+++ b/examples/dtls-sock/Makefile
@@ -45,9 +45,6 @@ CFLAGS += -DDTLS_DEFAULT_PORT=$(DTLS_PORT)
 # Uncomment to enable debug logs
 # CFLAGS += -DCONFIG_DTLS_DEBUG
 
-# FIXME: This is a temporary patch
-CFLAGS += -DTHREAD_STACKSIZE_MAIN=\(2*THREAD_STACKSIZE_LARGE\)
-
 # Comment this out to disable code in RIOT that does safety checking
 # which is not needed in a production environment but helps in the
 # development process:
@@ -61,3 +58,10 @@ include $(RIOTBASE)/Makefile.include
 # server and client, we need four slots in total. The default (2) would be
 # sufficient if only client or server is ever run.
 CFLAGS += -DCONFIG_CREDMAN_MAX_CREDENTIALS=4
+
+# FIXME: This is a temporary patch
+ifeq (,$(filter arch_avr8,$(FEATURES_USED)))
+  CFLAGS += -DTHREAD_STACKSIZE_MAIN=\(2*THREAD_STACKSIZE_LARGE\)
+else
+  CFLAGS += -DTHREAD_STACKSIZE_MAIN=\(4*THREAD_STACKSIZE_LARGE\)
+endif
-- 
2.43.0

Quite interestingly: With my local toolchain, this app goes straight into a bootloop on the ATxmega board. I'm not sure if this is an issue with my toolchain, or our ATxmega part relying on some optimization not being implemented in the GCC version used.

It seems this app never supported testing with both DTLS server and client running on the same board, as the credman capacity was too low for this. I was unable to successfully perform a handshake against ::1 (after starting the server). And my ATxmega board has no network interface other than loopback to test with.

@fzi-haxel
Copy link
Contributor Author

@maribu I've added your patch to the PR.
With your changes, did the loopback test still fail?

@maribu
Copy link
Member

maribu commented Dec 30, 2023

Since it worked for @benpicco, that doesn't seem to be an 8-bit compatibility issue. While that issue remains, it is unrelated and should not block the PR.

@maribu maribu enabled auto-merge December 30, 2023 00:17
@maribu maribu added this pull request to the merge queue Dec 30, 2023
Merged via the queue into RIOT-OS:master with commit 75512d5 Dec 30, 2023
26 checks passed
@fzi-haxel fzi-haxel deleted the pr/enable_dtls_sock_example_for_non_32 branch January 8, 2024 13:05
@MrKevinWeiss MrKevinWeiss added this to the Release 2024.01 milestone Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: examples Area: Example Applications CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants