From 32bb9743e4bc5d7dc120ce7e8f66d1185f79db47 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Wed, 20 Jul 2022 13:51:15 +0200 Subject: [PATCH 1/4] sys/net/gnrc/pktbuf_static: make use of alignas() Since we are now using C11, we can make use of `alignas()` provided by `` to make the alignment code easier to read. --- sys/net/gnrc/pktbuf/include/pktbuf_internal.h | 13 ++++--------- .../gnrc/pktbuf_static/gnrc_pktbuf_static.c | 19 +++++++++---------- .../pktbuf_static/include/pktbuf_static.h | 12 ++++++++++++ 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/sys/net/gnrc/pktbuf/include/pktbuf_internal.h b/sys/net/gnrc/pktbuf/include/pktbuf_internal.h index c48db7b82d..ddc3462311 100644 --- a/sys/net/gnrc/pktbuf/include/pktbuf_internal.h +++ b/sys/net/gnrc/pktbuf/include/pktbuf_internal.h @@ -28,6 +28,10 @@ #include "mutex.h" +#if IS_USED(MODULE_GNRC_PKTBUF_STATIC) +#include "pktbuf_static.h" +#endif + #ifdef __cplusplus extern "C" { #endif @@ -41,15 +45,6 @@ extern "C" { */ extern mutex_t gnrc_pktbuf_mutex; -#if IS_USED(MODULE_GNRC_PKTBUF_STATIC) || DOXYGEN -/** - * @brief The actual static buffer used when module gnrc_pktbuf_static is used - * - * @warning This is an internal buffer and should not be touched by external code - */ -extern uint8_t *gnrc_pktbuf_static_buf; -#endif - /** * @brief Check if the given pointer is indeed part of the packet buffer * diff --git a/sys/net/gnrc/pktbuf_static/gnrc_pktbuf_static.c b/sys/net/gnrc/pktbuf_static/gnrc_pktbuf_static.c index 041d2e9a12..6e981e740b 100644 --- a/sys/net/gnrc/pktbuf_static/gnrc_pktbuf_static.c +++ b/sys/net/gnrc/pktbuf_static/gnrc_pktbuf_static.c @@ -44,12 +44,9 @@ #endif #define CANARY 0x55 - -/* The static buffer needs to be aligned to word size, so that its start - * address can be casted to `_unused_t *` safely. Just allocating an array of - * (word sized) uintptr_t is a trivial way to do this */ -static uintptr_t _pktbuf_buf[CONFIG_GNRC_PKTBUF_SIZE / sizeof(uintptr_t)]; -uint8_t *gnrc_pktbuf_static_buf = (uint8_t *)_pktbuf_buf; +alignas(sizeof(_unused_t)) uint8_t gnrc_pktbuf_static_buf[CONFIG_GNRC_PKTBUF_SIZE]; +static_assert((CONFIG_GNRC_PKTBUF_SIZE % sizeof(_unused_t)) == 0, + "CONFIG_GNRC_PKTBUF_SIZE has to be a multiple of 8"); static _unused_t *_first_unused; #ifdef DEVELHELP @@ -91,11 +88,13 @@ void gnrc_pktbuf_init(void) { mutex_lock(&gnrc_pktbuf_mutex); if (CONFIG_GNRC_PKTBUF_CHECK_USE_AFTER_FREE) { - memset(_pktbuf_buf, CANARY, sizeof(_pktbuf_buf)); + memset(gnrc_pktbuf_static_buf, CANARY, sizeof(gnrc_pktbuf_static_buf)); } - _first_unused = (_unused_t *)_pktbuf_buf; + /* Silence false -Wcast-align: gnrc_pktbuf_static_buf has qualifier + * `alignas(_unused_t)`, so it is guaranteed to be safe */ + _first_unused = (_unused_t *)(uintptr_t)gnrc_pktbuf_static_buf; _first_unused->next = NULL; - _first_unused->size = sizeof(_pktbuf_buf); + _first_unused->size = sizeof(gnrc_pktbuf_static_buf); mutex_unlock(&gnrc_pktbuf_mutex); } @@ -326,7 +325,7 @@ void gnrc_pktbuf_stats(void) bool gnrc_pktbuf_is_empty(void) { return ((uintptr_t)_first_unused == (uintptr_t)gnrc_pktbuf_static_buf) && - (_first_unused->size == sizeof(_pktbuf_buf)); + (_first_unused->size == sizeof(gnrc_pktbuf_static_buf)); } bool gnrc_pktbuf_is_sane(void) diff --git a/sys/net/gnrc/pktbuf_static/include/pktbuf_static.h b/sys/net/gnrc/pktbuf_static/include/pktbuf_static.h index dce6f8736e..7f55b040d5 100644 --- a/sys/net/gnrc/pktbuf_static/include/pktbuf_static.h +++ b/sys/net/gnrc/pktbuf_static/include/pktbuf_static.h @@ -22,6 +22,7 @@ #define PKTBUF_STATIC_H #include +#include #ifdef __cplusplus extern "C" { @@ -40,6 +41,17 @@ typedef struct _unused { unsigned int size; /**< the size of the unused section */ } _unused_t; + +/** + * @brief The actual static buffer used when module gnrc_pktbuf_static is used + * + * @warning This is an internal buffer and should not be touched by external + * code + * + * @details This buffer is aligned to boundaries of `sizeof(_unused_t)` + */ +extern alignas(sizeof(_unused_t)) uint8_t gnrc_pktbuf_static_buf[CONFIG_GNRC_PKTBUF_SIZE]; + /** * @brief Calculates the required space of a number of bytes including * alignment to the size of @ref _unused_t From 3a80878848a2b1fb3bcce64be22cf6c951d10a36 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Thu, 21 Jul 2022 08:50:07 +0200 Subject: [PATCH 2/4] sys/net/gnrc_pktbuf: make static buffer internal --- sys/net/gnrc/pktbuf/include/pktbuf_internal.h | 15 +----- .../gnrc/pktbuf_malloc/gnrc_pktbuf_malloc.c | 8 +++ .../gnrc/pktbuf_static/gnrc_pktbuf_static.c | 52 +++++++++++-------- .../pktbuf_static/include/pktbuf_static.h | 12 ----- 4 files changed, 40 insertions(+), 47 deletions(-) diff --git a/sys/net/gnrc/pktbuf/include/pktbuf_internal.h b/sys/net/gnrc/pktbuf/include/pktbuf_internal.h index ddc3462311..029c39ee56 100644 --- a/sys/net/gnrc/pktbuf/include/pktbuf_internal.h +++ b/sys/net/gnrc/pktbuf/include/pktbuf_internal.h @@ -28,10 +28,6 @@ #include "mutex.h" -#if IS_USED(MODULE_GNRC_PKTBUF_STATIC) -#include "pktbuf_static.h" -#endif - #ifdef __cplusplus extern "C" { #endif @@ -56,16 +52,7 @@ extern mutex_t gnrc_pktbuf_mutex; * the packet buffer * @retval false @p ptr does not point to data in the packet buffer */ -static inline bool gnrc_pktbuf_contains(void *ptr) -{ -#if IS_USED(MODULE_GNRC_PKTBUF_STATIC) - return ((uintptr_t)ptr >= (uintptr_t)gnrc_pktbuf_static_buf) && - ((uintptr_t)ptr < (uintptr_t)gnrc_pktbuf_static_buf + CONFIG_GNRC_PKTBUF_SIZE); -#else - (void)ptr; - return true; -#endif -} +bool gnrc_pktbuf_contains(void *ptr); /** * @brief Release an internal buffer diff --git a/sys/net/gnrc/pktbuf_malloc/gnrc_pktbuf_malloc.c b/sys/net/gnrc/pktbuf_malloc/gnrc_pktbuf_malloc.c index 8decd931c5..dd5e38fe9f 100644 --- a/sys/net/gnrc/pktbuf_malloc/gnrc_pktbuf_malloc.c +++ b/sys/net/gnrc/pktbuf_malloc/gnrc_pktbuf_malloc.c @@ -288,4 +288,12 @@ void gnrc_pktbuf_free_internal(void *data, size_t size) _free(data); } +bool gnrc_pktbuf_contains(void *ptr) +{ + (void)ptr; + /* tracking the memory areas malloced is to expensive, so this function + * only is useful with gnrc_pktbuf_static */ + return true; +} + /** @} */ diff --git a/sys/net/gnrc/pktbuf_static/gnrc_pktbuf_static.c b/sys/net/gnrc/pktbuf_static/gnrc_pktbuf_static.c index 6e981e740b..5ac2708886 100644 --- a/sys/net/gnrc/pktbuf_static/gnrc_pktbuf_static.c +++ b/sys/net/gnrc/pktbuf_static/gnrc_pktbuf_static.c @@ -18,9 +18,10 @@ #include #include #include +#include #include -#include #include +#include #include #include "mutex.h" @@ -44,7 +45,8 @@ #endif #define CANARY 0x55 -alignas(sizeof(_unused_t)) uint8_t gnrc_pktbuf_static_buf[CONFIG_GNRC_PKTBUF_SIZE]; + +static alignas(sizeof(_unused_t)) uint8_t _static_buf[CONFIG_GNRC_PKTBUF_SIZE]; static_assert((CONFIG_GNRC_PKTBUF_SIZE % sizeof(_unused_t)) == 0, "CONFIG_GNRC_PKTBUF_SIZE has to be a multiple of 8"); static _unused_t *_first_unused; @@ -88,13 +90,13 @@ void gnrc_pktbuf_init(void) { mutex_lock(&gnrc_pktbuf_mutex); if (CONFIG_GNRC_PKTBUF_CHECK_USE_AFTER_FREE) { - memset(gnrc_pktbuf_static_buf, CANARY, sizeof(gnrc_pktbuf_static_buf)); + memset(_static_buf, CANARY, sizeof(_static_buf)); } - /* Silence false -Wcast-align: gnrc_pktbuf_static_buf has qualifier + /* Silence false -Wcast-align: _static_buf has qualifier * `alignas(_unused_t)`, so it is guaranteed to be safe */ - _first_unused = (_unused_t *)(uintptr_t)gnrc_pktbuf_static_buf; + _first_unused = (_unused_t *)(uintptr_t)_static_buf; _first_unused->next = NULL; - _first_unused->size = sizeof(gnrc_pktbuf_static_buf); + _first_unused->size = sizeof(_static_buf); mutex_unlock(&gnrc_pktbuf_mutex); } @@ -281,12 +283,12 @@ void gnrc_pktbuf_stats(void) { #ifdef MODULE_OD _unused_t *ptr = _first_unused; - uint8_t *chunk = &gnrc_pktbuf_static_buf[0]; + uint8_t *chunk = &_static_buf[0]; int count = 0; printf("packet buffer: first byte: %p, last byte: %p (size: %u)\n", - (void *)&gnrc_pktbuf_static_buf[0], - (void *)&gnrc_pktbuf_static_buf[CONFIG_GNRC_PKTBUF_SIZE], + (void *)&_static_buf[0], + (void *)&_static_buf[CONFIG_GNRC_PKTBUF_SIZE], CONFIG_GNRC_PKTBUF_SIZE); printf(" position of last byte used: %" PRIu16 "\n", max_byte_count); if (ptr == NULL) { /* packet buffer is completely full */ @@ -312,8 +314,8 @@ void gnrc_pktbuf_stats(void) ptr = ptr->next; } - if (chunk <= &gnrc_pktbuf_static_buf[CONFIG_GNRC_PKTBUF_SIZE - 1]) { - _print_chunk(chunk, &gnrc_pktbuf_static_buf[CONFIG_GNRC_PKTBUF_SIZE] - chunk, count); + if (chunk <= &_static_buf[CONFIG_GNRC_PKTBUF_SIZE - 1]) { + _print_chunk(chunk, &_static_buf[CONFIG_GNRC_PKTBUF_SIZE] - chunk, count); } #else DEBUG("pktbuf: needs od module\n"); @@ -324,8 +326,8 @@ void gnrc_pktbuf_stats(void) #ifdef TEST_SUITES bool gnrc_pktbuf_is_empty(void) { - return ((uintptr_t)_first_unused == (uintptr_t)gnrc_pktbuf_static_buf) && - (_first_unused->size == sizeof(gnrc_pktbuf_static_buf)); + return ((uintptr_t)_first_unused == (uintptr_t)_static_buf) && + (_first_unused->size == sizeof(_static_buf)); } bool gnrc_pktbuf_is_sane(void) @@ -335,8 +337,8 @@ bool gnrc_pktbuf_is_sane(void) /* Invariants of this implementation: * - the head of _unused_t list is _first_unused * - if _unused_t list is empty the packet buffer is full and _first_unused is NULL - * - forall ptr_in _unused_t list: &gnrc_pktbuf_static_buf[0] < ptr - * && ptr < &gnrc_pktbuf_static_buf[CONFIG_GNRC_PKTBUF_SIZE] + * - forall ptr_in _unused_t list: &_static_buf[0] < ptr + * && ptr < &_static_buf[CONFIG_GNRC_PKTBUF_SIZE] * - forall ptr in _unused_t list: ptr->next == NULL || ptr < ptr->next * - forall ptr in _unused_t list: (ptr->next != NULL && ptr->size <= (ptr->next - ptr)) || * (ptr->next == NULL @@ -344,14 +346,14 @@ bool gnrc_pktbuf_is_sane(void) */ while (ptr) { - if ((&gnrc_pktbuf_static_buf[0] >= (uint8_t *)ptr) - && ((uint8_t *)ptr >= &gnrc_pktbuf_static_buf[CONFIG_GNRC_PKTBUF_SIZE])) { + if ((&_static_buf[0] >= (uint8_t *)ptr) + && ((uint8_t *)ptr >= &_static_buf[CONFIG_GNRC_PKTBUF_SIZE])) { return false; } if ((ptr->next != NULL) && (ptr >= ptr->next)) { return false; } - size_t pos_in_buf = (uint8_t *)ptr - &gnrc_pktbuf_static_buf[0]; + size_t pos_in_buf = (uint8_t *)ptr - &_static_buf[0]; if (((ptr->next == NULL) || (ptr->size > (size_t)((uint8_t *)(ptr->next) - (uint8_t *)ptr))) && ((ptr->next != NULL) || (ptr->size != CONFIG_GNRC_PKTBUF_SIZE - pos_in_buf))) { return false; @@ -415,7 +417,7 @@ static void *_pktbuf_alloc(size_t size) * We cast to uintptr_t as intermediate step to silence -Wcast-align */ _unused_t *new = (_unused_t *)((uintptr_t)ptr + size); - if (((((uint8_t *)new) - &(gnrc_pktbuf_static_buf[0])) + sizeof(_unused_t)) + if (((((uint8_t *)new) - &(_static_buf[0])) + sizeof(_unused_t)) > CONFIG_GNRC_PKTBUF_SIZE) { /* content of new would exceed packet buffer size so set to NULL */ _first_unused = NULL; @@ -430,7 +432,7 @@ static void *_pktbuf_alloc(size_t size) new->size = ptr->size - size; } #ifdef DEVELHELP - uint16_t last_byte = (uint16_t)((((uint8_t *)ptr) + size) - &(gnrc_pktbuf_static_buf[0])); + uint16_t last_byte = (uint16_t)((((uint8_t *)ptr) + size) - &(_static_buf[0])); if (last_byte > max_byte_count) { max_byte_count = last_byte; } @@ -488,7 +490,7 @@ void gnrc_pktbuf_free_internal(void *data, size_t size) new->size = _align(size); /* calculate number of bytes between new _unused_t chunk and end of packet * buffer */ - bytes_at_end = ((&gnrc_pktbuf_static_buf[0] + CONFIG_GNRC_PKTBUF_SIZE) + bytes_at_end = ((&_static_buf[0] + CONFIG_GNRC_PKTBUF_SIZE) - (((uint8_t *)new) + new->size)); if (bytes_at_end < sizeof(_unused_t)) { /* new is very last segment and there is a little bit of memory left @@ -509,4 +511,12 @@ void gnrc_pktbuf_free_internal(void *data, size_t size) } } +bool gnrc_pktbuf_contains(void *ptr) +{ + const uintptr_t start = (uintptr_t)_static_buf; + const uintptr_t end = start + sizeof(_static_buf); + uintptr_t pos = (uintptr_t)ptr; + return ((pos >= start) && (pos < end)); +} + /** @} */ diff --git a/sys/net/gnrc/pktbuf_static/include/pktbuf_static.h b/sys/net/gnrc/pktbuf_static/include/pktbuf_static.h index 7f55b040d5..dce6f8736e 100644 --- a/sys/net/gnrc/pktbuf_static/include/pktbuf_static.h +++ b/sys/net/gnrc/pktbuf_static/include/pktbuf_static.h @@ -22,7 +22,6 @@ #define PKTBUF_STATIC_H #include -#include #ifdef __cplusplus extern "C" { @@ -41,17 +40,6 @@ typedef struct _unused { unsigned int size; /**< the size of the unused section */ } _unused_t; - -/** - * @brief The actual static buffer used when module gnrc_pktbuf_static is used - * - * @warning This is an internal buffer and should not be touched by external - * code - * - * @details This buffer is aligned to boundaries of `sizeof(_unused_t)` - */ -extern alignas(sizeof(_unused_t)) uint8_t gnrc_pktbuf_static_buf[CONFIG_GNRC_PKTBUF_SIZE]; - /** * @brief Calculates the required space of a number of bytes including * alignment to the size of @ref _unused_t From bb084612f5e2ea23b408efaa7663899e2c2ae3c6 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Mon, 14 Mar 2022 02:40:20 +0100 Subject: [PATCH 3/4] drivers/slipdev: implement sleep states --- drivers/include/slipdev.h | 8 ++++ drivers/slipdev/slipdev.c | 87 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/drivers/include/slipdev.h b/drivers/include/slipdev.h index 9c9c6984b6..6740216b49 100644 --- a/drivers/include/slipdev.h +++ b/drivers/include/slipdev.h @@ -72,6 +72,14 @@ enum { * @brief Device writes received data to stdin */ SLIPDEV_STATE_STDIN, + /** + * @brief Device is in standby, will wake up when sending data + */ + SLIPDEV_STATE_STANDBY, + /** + * @brief Device is in sleep mode + */ + SLIPDEV_STATE_SLEEP, }; /** @} */ diff --git a/drivers/slipdev/slipdev.c b/drivers/slipdev/slipdev.c index 6cb7cd78a0..6849457171 100644 --- a/drivers/slipdev/slipdev.c +++ b/drivers/slipdev/slipdev.c @@ -32,6 +32,8 @@ #include "mutex.h" #include "stdio_uart.h" +static int _check_state(slipdev_t *dev); + static inline void slipdev_lock(void) { if (IS_USED(MODULE_SLIPDEV_STDIO)) { @@ -73,6 +75,23 @@ check_end: } } +static void _poweron(slipdev_t *dev) +{ + if ((dev->state != SLIPDEV_STATE_STANDBY) || + (dev->state != SLIPDEV_STATE_SLEEP)) { + return; + } + + dev->state = 0; + uart_init(dev->config.uart, dev->config.baudrate, _slip_rx_cb, dev); +} + +static inline void _poweroff(slipdev_t *dev, uint8_t state) +{ + uart_poweroff(dev->config.uart); + dev->state = state; +} + static int _init(netdev_t *netdev) { slipdev_t *dev = (slipdev_t *)netdev; @@ -145,10 +164,33 @@ unsigned slipdev_unstuff_readbyte(uint8_t *buf, uint8_t byte, bool *escaped) return res; } +static int _check_state(slipdev_t *dev) +{ + /* power states not supported when multiplexing stdio */ + if (IS_USED(MODULE_SLIPDEV_STDIO)) { + return 0; + } + + /* discard data when interface is in SLEEP mode */ + if (dev->state == SLIPDEV_STATE_SLEEP) { + return -ENETDOWN; + } + + /* sending data wakes the interface from STANDBY */ + if (dev->state == SLIPDEV_STATE_STANDBY) { + _poweron(dev); + } + + return 0; +} + static int _send(netdev_t *netdev, const iolist_t *iolist) { slipdev_t *dev = (slipdev_t *)netdev; - int bytes = 0; + int bytes = _check_state(dev); + if (bytes) { + return bytes; + } DEBUG("slipdev: sending iolist\n"); slipdev_lock(); @@ -220,6 +262,45 @@ static void _isr(netdev_t *netdev) } } +#if !IS_USED(MODULE_SLIPDEV_STDIO) +static int _set_state(slipdev_t *dev, netopt_state_t state) +{ + if (IS_USED(MODULE_SLIPDEV_STDIO)) { + return -ENOTSUP; + } + + switch (state) { + case NETOPT_STATE_STANDBY: + _poweroff(dev, SLIPDEV_STATE_STANDBY); + break; + case NETOPT_STATE_SLEEP: + _poweroff(dev, SLIPDEV_STATE_SLEEP); + break; + case NETOPT_STATE_IDLE: + _poweron(dev); + break; + default: + return -ENOTSUP; + } + + return sizeof(netopt_state_t); +} + +static int _set(netdev_t *netdev, netopt_t opt, const void *value, size_t max_len) +{ + (void)max_len; + + slipdev_t *dev = (slipdev_t *)netdev; + switch (opt) { + case NETOPT_STATE: + assert(max_len <= sizeof(netopt_state_t)); + return _set_state(dev, *((const netopt_state_t *)value)); + default: + return -ENOTSUP; + } +} +#endif /* !MODULE_SLIPDEV_STDIO */ + static int _get(netdev_t *netdev, netopt_t opt, void *value, size_t max_len) { (void)netdev; @@ -249,7 +330,11 @@ static const netdev_driver_t slip_driver = { .init = _init, .isr = _isr, .get = _get, +#if IS_USED(MODULE_SLIPDEV_STDIO) .set = netdev_set_notsup, +#else + .set = _set, +#endif }; void slipdev_setup(slipdev_t *dev, const slipdev_params_t *params, uint8_t index) From 1e29dd3267dcf35eab0f2ad5e3e110ccf0149ee8 Mon Sep 17 00:00:00 2001 From: Kaspar Schleiser Date: Tue, 10 Jan 2023 12:42:40 +0100 Subject: [PATCH 4/4] CI: seperate check-labels and check-commits workflows --- .../workflows/{check-pr.yml => check-commits.yml} | 14 ++------------ .github/workflows/check-labels.yml | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 12 deletions(-) rename .github/workflows/{check-pr.yml => check-commits.yml} (59%) create mode 100644 .github/workflows/check-labels.yml diff --git a/.github/workflows/check-pr.yml b/.github/workflows/check-commits.yml similarity index 59% rename from .github/workflows/check-pr.yml rename to .github/workflows/check-commits.yml index 1c7569b2f0..d294f37836 100644 --- a/.github/workflows/check-pr.yml +++ b/.github/workflows/check-commits.yml @@ -1,18 +1,8 @@ -name: check-pr +name: check-commits on: pull_request: - types: [opened, reopened, labeled, unlabeled, synchronize] - pull_request_review: - types: [submitted, dismissed] + types: [opened, reopened, synchronize] jobs: - check-labels: - runs-on: ubuntu-latest - steps: - - uses: RIOT-OS/check-labels-action@v1.0.0 - with: - access_token: ${{ secrets.GITHUB_TOKEN }} - unset_labels: 'CI: needs squashing,State: waiting for CI update,State: waiting for other PR,Process: blocked by feature freeze' - cond_labels: '(Process: needs >1 ACK,review.approvals>1),(Area: RDM,review.approvals>2)' check-commits: runs-on: ubuntu-latest if: ${{ github.base_ref }} diff --git a/.github/workflows/check-labels.yml b/.github/workflows/check-labels.yml new file mode 100644 index 0000000000..37d9dcd3ee --- /dev/null +++ b/.github/workflows/check-labels.yml @@ -0,0 +1,15 @@ +name: check-labels +on: + pull_request: + types: [opened, reopened, labeled, unlabeled] + pull_request_review: + types: [submitted, dismissed] +jobs: + check-labels: + runs-on: ubuntu-latest + steps: + - uses: RIOT-OS/check-labels-action@v1.0.0 + with: + access_token: ${{ secrets.GITHUB_TOKEN }} + unset_labels: 'CI: needs squashing,State: waiting for CI update,State: waiting for other PR,Process: blocked by feature freeze' + cond_labels: '(Process: needs >1 ACK,review.approvals>1),(Area: RDM,review.approvals>2)'