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.

Sign up for GitHub

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

Jump to bottom

libschc: initial import as package #18515

Merged
merged 2 commits into from
Mar 2, 2023
Merged

libschc: initial import as package #18515

merged 2 commits into from
Mar 2, 2023

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Aug 25, 2022 •
edited
Loading

Contribution description

This is part 1 of my takeover of #15040. The PR only contains the import of the pkg, not the GNRC module, which will be provided in part 2 as a separate PR.

I mostly updated to both the current version of libSCHC and RIOT, added tests, and adapted the overall style to be more RIOT-like and following our coding conventions.

Testing already revealed some usage problems (or maybe even bugs) with libSCHC. I will have a call with @bartmoons next week to discuss these.

TODOs
  • Adapt pre-configured rule in pkg/libschc/include/rules set more for general RIOT+LoRaWAN use cases
  • Increase test coverage of tests
  • Clean-up rule set for test application

Testing procedure

Run the provided tests with

make -C tests/pkg_libschc/ flash test

on a platform of your choice.

Issues/PRs references

Partial alternative to #15040

@miri64 miri64 added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Aug 25, 2022
@github-actions github-actions bot added Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports Area: tests Area: tests and testing framework labels Aug 25, 2022
@miri64 miri64 mentioned this pull request Aug 25, 2022
Closed
@miri64 miri64 force-pushed the pkg/feat/libschc branch 3 times, most recently from a6972ef to 0f80bc5 Compare August 26, 2022 08:57
@github-actions github-actions bot added Area: doc Area: Documentation Area: CI Area: Continuous Integration of RIOT components labels Aug 26, 2022
@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 26, 2022
@miri64 miri64 force-pushed the pkg/feat/libschc branch 3 times, most recently from 1dd4af8 to a12018f Compare August 26, 2022 09:24
@miri64 miri64 added the CI: run tests If set, CI server will run tests on hardware for the labeled PR label Aug 26, 2022
@miri64 miri64 force-pushed the pkg/feat/libschc branch 2 times, most recently from 0f439c4 to b643136 Compare August 26, 2022 09:44
@github-actions github-actions bot added the Area: build system Area: Build system label Aug 26, 2022
@miri64 miri64 force-pushed the pkg/feat/libschc branch 2 times, most recently from 12604d1 to a75398d Compare August 26, 2022 09:56
@miri64 miri64 added CI: disable test cache If set, CI will always run all tests regardless of whether they have been run successfully before CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 26, 2022
@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 26, 2022
@kaspar030 kaspar030 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 28, 2022
@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 29, 2022
@miri64 miri64 force-pushed the pkg/feat/libschc branch 2 times, most recently from f26066f to 1fa9646 Compare February 15, 2023 12:34
@miri64 @bartmoons
libschc: initial import
566066a 
Co-Authored-By: boortmans <bart.moons@gmail.com>
@miri64
Copy link
Member Author

miri64 commented Feb 22, 2023

This PR is ready for review in my opinion and required for me to seriously proceed on #18700.

@miri64
Copy link
Member Author

miri64 commented Feb 22, 2023

The libSCHC version may need some more bumps, but I would rather prefer to do that in master, than keeping track of a moving target here.

jia200x
jia200x approved these changes Mar 2, 2023
View reviewed changes
Copy link
Member

@jia200x jia200x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK.
I tested this branch through #18700 and everything works as expected. Since this is still a (kind of) experimental feature, I think we should get it ASAP and continue improving from there.

Nice work btw!

@miri64 miri64 force-pushed the pkg/feat/libschc branch from d6133a9 to 9459e29 Compare March 2, 2023 10:31
@github-actions github-actions bot removed the Area: CI Area: Continuous Integration of RIOT components label Mar 2, 2023
@miri64
Copy link
Member Author

miri64 commented Mar 2, 2023

Squashed and removed the temporary commit that restricted Murdock to the test.

@miri64
Copy link
Member Author

miri64 commented Mar 2, 2023

bors try

bors bot added a commit that referenced this pull request Mar 2, 2023
@bors
Try #18515:
d3893b3
@miri64
Copy link
Member Author

miri64 commented Mar 2, 2023

bors cancel
bors merge

bors bot added a commit that referenced this pull request Mar 2, 2023
@bors @miri64 @chrysn
Merge #18515 #19331
0a44316 
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>
@miri64
Copy link
Member Author

miri64 commented Mar 2, 2023

bors t-

@miri64
Copy link
Member Author

miri64 commented Mar 2, 2023

bors try-

@miri64
Copy link
Member Author

miri64 commented Mar 2, 2023

Or for globs sake, bors try- already!

@bors
Copy link
Contributor

bors bot commented Mar 2, 2023

Build failed (retrying...):

@bors
Copy link
Contributor

bors bot commented Mar 2, 2023

Build succeeded:

@bors bors bot merged commit 8d800e9 into RIOT-OS:master Mar 2, 2023
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jia200x jia200x jia200x approved these changes

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss MrKevinWeiss is a code owner

@smlng smlng Awaiting requested review from smlng

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@kaspar030 kaspar030 Awaiting requested review from kaspar030

Assignees
No one assigned
Labels
Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Milestone
Release 2023.04
Development

Successfully merging this pull request may close these issues.
6 participants
@miri64 @MrKevinWeiss @riot-ci @jia200x @kaspar030 @leandrolanzieri