Skip to content

Commit

Permalink
Merge #16158 #19331
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
3 people authored Feb 28, 2023
3 parents 4ccf0af + ecdc64e + 9e48b6e commit 2a73f1b
Show file tree
Hide file tree
Showing 12 changed files with 469 additions and 1 deletion.
6 changes: 6 additions & 0 deletions makefiles/pseudomodules.inc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,12 @@ PSEUDOMODULES += gnrc_sixlowpan_frag_sfr_stats
## @{
##
PSEUDOMODULES += gnrc_sixlowpan_frag_sfr_congure
## @defgroup net_gnrc_sixlowpan_frag_sfr_congure_sfr gnrc_sixlowpan_frag_sfr_congure_sfr: Appendix C
## @brief Basic congestion control for 6LoWPAN SFR as proposed in Appendix C of RFC 8931
## @see [RFC 8931, Appendix C](https://tools.ietf.org/html/rfc8931#section-appendix.c)
## @{
PSEUDOMODULES += gnrc_sixlowpan_frag_sfr_congure_sfr
## @}
## @}
PSEUDOMODULES += gnrc_sixlowpan_iphc_nhc
PSEUDOMODULES += gnrc_sixlowpan_nd_border_router
Expand Down
1 change: 1 addition & 0 deletions pkg/tinydtls/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ config DTLS_CONTEXT_MAX

config DTLS_PEER_MAX
int "Max number of peers"
default 2 if KCONFIG_USEMODULE_GCOAP_DTLS
default 1
help
The maximum number of DTLS peers.
Expand Down
26 changes: 26 additions & 0 deletions pkg/tinydtls/Makefile.include
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,35 @@ endif
PEER_MAX := $(or $(CONFIG_DTLS_PEER_MAX),$(patsubst -DCONFIG_DTLS_PEER_MAX=%,%,$(filter -DCONFIG_DTLS_PEER_MAX=%,$(CFLAGS))))
ifneq (,$(PEER_MAX))
CFLAGS += -DDTLS_PEER_MAX=$(PEER_MAX)
else ifneq (,$(filter gcoap_dtls,$(USEMODULE)))
# The default value in sys/include/net/dtls.h for CONFIG_DTLS_PEER_MAX is 2
# when gcoap_dtls is active, otherwise 1. As the default in tinydtls is 1,
# we need to set it explicitly if the dtls.h default value deviates from
# the tinydtls default.
CFLAGS += -DDTLS_PEER_MAX=2
endif

HANDSHAKE_MAX := $(or $(CONFIG_DTLS_HANDSHAKE_MAX),$(patsubst -DCONFIG_DTLS_HANDSHAKE_MAX=%,%,$(filter -DCONFIG_DTLS_HANDSHAKE_MAX=%,$(CFLAGS))))
ifneq (,$(HANDSHAKE_MAX))
CFLAGS += -DDTLS_HANDSHAKE_MAX=$(HANDSHAKE_MAX)
endif

ifneq (,$(filter gcoap,$(USEMODULE)))
# Configuring the buffer large enough that a full Gcoap packet can be
# encrypted or decrypted.

# This is the default in gcoap.h, which we don't have access to, so it is copied over.
CONFIG_GCOAP_PDU_BUF_SIZE := $(or $(CONFIG_GCOAP_PDU_BUF_SIZE),128)

# If there were another way to set up DTLS_MAX_BUF, we'd need to set the
# maximum of these here.
#
# 29 bytes are the overhead measured with Wireshark on packets exchanged in
# default configuration; adding some to be safe against variable size fields.
CFLAGS += "-DDTLS_MAX_BUF=($(CONFIG_GCOAP_PDU_BUF_SIZE) + 36)"
endif

# 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)
6 changes: 6 additions & 0 deletions sys/include/net/dtls.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
#ifndef NET_DTLS_H
#define NET_DTLS_H

#include "modules.h"

#ifdef __cplusplus
extern "C" {
#endif
Expand All @@ -44,8 +46,12 @@ extern "C" {
* @brief The maximum number DTLS peers (i.e. sessions)
*/
#ifndef CONFIG_DTLS_PEER_MAX
#if IS_USED(MODULE_GCOAP_DTLS)
#define CONFIG_DTLS_PEER_MAX (2)
#else
#define CONFIG_DTLS_PEER_MAX (1)
#endif
#endif

#ifdef __cplusplus
}
Expand Down
2 changes: 1 addition & 1 deletion sys/include/net/gnrc/sixlowpan/frag/sfr/congure.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* When included, this module enables congestion control for 6LoWPAN Selective Fragment Recovery
* (SFR). The flavor of congestion control can be selected using the following sub-modules:
*
* - TBD
* - @ref net_gnrc_sixlowpan_frag_sfr_congure_sfr (the default)
* @{
*
* @file
Expand Down
4 changes: 4 additions & 0 deletions sys/net/gnrc/Makefile.dep
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,10 @@ endif

ifneq (,$(filter gnrc_sixlowpan_frag_sfr_congure,$(USEMODULE)))
USEMODULE += gnrc_sixlowpan_frag_sfr
ifeq (,$(filter gnrc_sixlowpan_frag_sfr_congure_% congure_mock,$(USEMODULE)))
# pick Appendix C as default congestion control
USEMODULE += gnrc_sixlowpan_frag_sfr_congure_sfr
endif
endif

ifneq (,$(filter gnrc_sixlowpan_frag_sfr_ecn_%,$(USEMODULE)))
Expand Down
115 changes: 115 additions & 0 deletions sys/net/gnrc/network_layer/sixlowpan/frag/sfr/congure_sfr.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/*
* Copyright (C) 2021 Freie Universität Berlin
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @{
*
* @file
* @author Martine Lenders <m.lenders@fu-berlin.de>
*/

#include "kernel_defines.h"
#include "congure.h"
#include "net/gnrc/sixlowpan/config.h"

#include "net/gnrc/sixlowpan/frag/sfr/congure.h"

/* This implements a very simple SFR as suggested in
* https://datatracker.ietf.org/doc/html/rfc8931#appendix-C */

typedef congure_snd_t congure_sfr_snd_t;

static void _snd_init(congure_snd_t *cong, void *ctx);
static int32_t _snd_inter_msg_interval(congure_snd_t *cong, unsigned msg_size);
static void _snd_report_msg_sent(congure_snd_t *cong, unsigned sent_size);
static void _snd_report_msg_discarded(congure_snd_t *cong, unsigned msg_size);
static void _snd_report_msgs_lost(congure_snd_t *cong, congure_snd_msg_t *msgs);
static void _snd_report_msgs_timeout(congure_snd_t *cong, congure_snd_msg_t *msgs);
static void _snd_report_msg_acked(congure_snd_t *cong, congure_snd_msg_t *msg,
congure_snd_ack_t *ack);
static void _snd_report_ecn_ce(congure_snd_t *cong, ztimer_now_t time);

static congure_sfr_snd_t _sfr_congures_sfr[CONFIG_GNRC_SIXLOWPAN_FRAG_FB_SIZE];
static const congure_snd_driver_t _driver = {
.init = _snd_init,
.inter_msg_interval = _snd_inter_msg_interval,
.report_msg_sent = _snd_report_msg_sent,
.report_msg_discarded = _snd_report_msg_discarded,
.report_msgs_timeout = _snd_report_msgs_timeout,
.report_msgs_lost = _snd_report_msgs_lost,
.report_msg_acked = _snd_report_msg_acked,
.report_ecn_ce = _snd_report_ecn_ce,
};

congure_snd_t *gnrc_sixlowpan_frag_sfr_congure_snd_get(void)
{
for (unsigned i = 0; i < ARRAY_SIZE(_sfr_congures_sfr); i++) {
if (_sfr_congures_sfr[i].driver == NULL) {
_sfr_congures_sfr[i].driver = &_driver;
return &_sfr_congures_sfr[i];
}
}
return NULL;
}

static void _snd_init(congure_snd_t *cong, void *ctx)
{
(void)ctx;
cong->cwnd = CONFIG_GNRC_SIXLOWPAN_SFR_OPT_WIN_SIZE;
}

static int32_t _snd_inter_msg_interval(congure_snd_t *cong, unsigned msg_size)
{
(void)cong;
(void)msg_size;
return CONFIG_GNRC_SIXLOWPAN_SFR_INTER_FRAME_GAP_US;
}

static void _snd_report_msg_sent(congure_snd_t *cong, unsigned sent_size)
{
(void)cong;
(void)sent_size;
}

static void _snd_report_msg_discarded(congure_snd_t *cong, unsigned msg_size)
{
(void)cong;
(void)msg_size;
}

static void _snd_report_msgs_lost(congure_snd_t *cong, congure_snd_msg_t *msgs)
{
/* Appendix C defines loss as timeout, so this does nothing */
(void)cong;
(void)msgs;
}

static void _snd_report_msgs_timeout(congure_snd_t *cong,
congure_snd_msg_t *msgs)
{
(void)msgs;
if (cong->cwnd > 1U) {
cong->cwnd /= 2U;
}
}

static void _snd_report_msg_acked(congure_snd_t *cong, congure_snd_msg_t *msg,
congure_snd_ack_t *ack)
{
(void)cong;
(void)msg;
(void)ack;
}

static void _snd_report_ecn_ce(congure_snd_t *cong, ztimer_now_t time)
{
(void)time;
if (cong->cwnd > 1U) {
cong->cwnd--;
}
}
49 changes: 49 additions & 0 deletions tests/gnrc_sixlowpan_frag_sfr_congure_impl/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
include ../Makefile.tests_common

USEMODULE += auto_init_gnrc_netif
USEMODULE += gnrc_ipv6_router_default
USEMODULE += gnrc_sixlowpan_frag_sfr
USEMODULE += gnrc_udp
USEMODULE += gnrc_rpl
USEMODULE += auto_init_gnrc_rpl
USEMODULE += gnrc_pktdump
USEMODULE += gnrc_icmpv6_echo
USEMODULE += shell
USEMODULE += shell_cmds_default
USEMODULE += shell_cmd_gnrc_pktbuf
USEMODULE += shell_cmd_gnrc_udp
USEMODULE += ps
USEMODULE += netstats_l2
USEMODULE += netstats_ipv6
USEMODULE += netstats_rpl

ifeq (native, $(BOARD))
USEMODULE += socket_zep
USEMODULE += socket_zep_hello
USEMODULE += netdev
TERMFLAGS = -z 127.0.0.1:17754 # Murdock has no IPv6 support
else
USEMODULE += netdev_default
# automated test only works on native
TESTS=
endif

CONGURE_IMPL ?= congure_sfr

ifeq (congure_sfr,$(CONGURE_IMPL))
USEMODULE += gnrc_sixlowpan_frag_sfr_congure_sfr
else
$(error "Unknown CongURE implementation `$(CONGURE_IMPL)`")
endif

.PHONY: zep_dispatch

zep_dispatch:
$(Q)env -u CC -u CFLAGS $(MAKE) -C $(RIOTTOOLS) $@

TERMDEPS += zep_dispatch

include $(RIOTBASE)/Makefile.include

# Set a custom channel if needed
include $(RIOTMAKE)/default-radio-settings.inc.mk
56 changes: 56 additions & 0 deletions tests/gnrc_sixlowpan_frag_sfr_congure_impl/Makefile.ci
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
BOARD_INSUFFICIENT_MEMORY := \
airfy-beacon \
arduino-duemilanove \
arduino-leonardo \
arduino-mega2560 \
arduino-nano \
arduino-uno \
atmega328p \
atmega328p-xplained-mini \
atxmega-a3bu-xplained \
blackpill-stm32f103c8 \
blackpill-stm32f103cb \
bluepill-stm32f030c8 \
bluepill-stm32f103c8 \
bluepill-stm32f103cb \
calliope-mini \
derfmega128 \
hifive1 \
hifive1b \
i-nucleo-lrwan1 \
im880b \
lsn50 \
microbit \
microduino-corerf \
msb-430 \
msb-430h \
nrf51dongle \
nrf6310 \
nucleo-f030r8 \
nucleo-f031k6 \
nucleo-f042k6 \
nucleo-f070rb \
nucleo-f072rb \
nucleo-f302r8 \
nucleo-f303k8 \
nucleo-f334r8 \
nucleo-l011k4 \
nucleo-l031k6 \
nucleo-l053r8 \
samd10-xmini \
saml10-xpro \
saml11-xpro \
slstk3400a \
stk3200 \
stm32f030f4-demo \
stm32f0discovery \
stm32f7508-dk \
stm32g0316-disco \
stm32l0538-disco \
stm32mp157c-dk2 \
telosb \
waspmote-pro \
yunjia-nrf51822 \
z1 \
zigduino \
#
8 changes: 8 additions & 0 deletions tests/gnrc_sixlowpan_frag_sfr_congure_impl/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
SFR CongURE implementation test
===============================
This test is largely based on the [`gnrc_networking`][1] example but is changed
so SFR with different CongURE implementations can be tested. When `CONGURE_IMPL`
is not set in the environment, `gnrc_sixlowpan_frag_sfr_congure_sfr` is used,
other implementations can be used with `congure_<impl>`.

[1]: https://github.com/RIOT-OS/RIOT/tree/master/examples/gnrc_networking
39 changes: 39 additions & 0 deletions tests/gnrc_sixlowpan_frag_sfr_congure_impl/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright (C) 2015-21 Freie Universität Berlin
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @ingroup tests
* @{
*
* @file
*
* @author Hauke Petersen <hauke.petersen@fu-berlin.de>
* @author Martine Lenders <m.lenders@fu-berlin.de>
*
* @}
*/

#include <stdio.h>

#include "fmt.h"
#include "shell.h"
#include "msg.h"

#define MAIN_QUEUE_SIZE (8)
static msg_t _main_msg_queue[MAIN_QUEUE_SIZE];

int main(void)
{
msg_init_queue(_main_msg_queue, MAIN_QUEUE_SIZE);

char line_buf[SHELL_DEFAULT_BUFSIZE];
shell_run(NULL, line_buf, SHELL_DEFAULT_BUFSIZE);

/* should be never reached */
return 0;
}
Loading

0 comments on commit 2a73f1b

Please sign in to comment.