Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
19618: cpu/stm32: fix riotboot settings for L4 and WB r=benpicco a=gschorcht

### Contribution description

This PR fixes the `riotboot` configuration for L4 and WB.

The family is not called `stm32l4` or `stm32wb` but `l4` and `wb`. That is, the `riotboot` configuration didn't work at all. Furthermore, a minimum `RIOTBOOT_LEN` of `0x2000` is required for L4.

Found when investigating the compilation errors for `bootloaders/riotboot_serial` in PR #19576.

### Testing procedure

1. Green CI.
2. Use the following commands:
    ```
    BOARD=nucleo-l496zg make -C tests/riotboot info-debug-variable-RIOTBOOT_HDR_LEN
    BOARD=p-nucleo-wb55 make -C tests/riotboot info-debug-variable-RIOTBOOT_HDR_LEN
    ```
    In master these commands give
    ```
    0x400
    ```
    With this PR these commands give
    ```
    0x200
    ```
    as expected.
3. Use the following commands:
    ```
    BOARD=nucleo-l496zg make -C tests/riotboot info-debug-variable-RIOTBOOT_LEN
    BOARD=p-nucleo-wb55 make -C tests/riotboot info-debug-variable-RIOTBOOT_LEN
    ```
    In master these commands give
    ```
    0x1000
    ```
    With this PR these commands give
    ```
    0x2000
    ```
    as expected.

### Issues/PRs references


19639: tests/net/gnrc_mac_timeout: add automated test r=aabadie a=aabadie



19644: gnrc_ipv6_nib: include RIO with all subnets in downstream RA r=benpicco a=benpicco



19649: gnrc_sixlowpan_iphc: prefix bits outside context must be zero r=benpicco a=benpicco



19656: gnrc/ipv6_auto_subnets: allow to configure minimal prefix length r=benpicco a=benpicco



Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
Co-authored-by: Alexandre Abadie <alexandre.abadie@inria.fr>
Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
  • Loading branch information
4 people committed May 23, 2023
6 parents 3469fce + e6c1aec + da67b11 + 771e9e9 + 7e9308d + 2a76911 commit a272abb
Show file tree
Hide file tree
Showing 7 changed files with 206 additions and 38 deletions.
13 changes: 13 additions & 0 deletions core/lib/include/compiler_hints.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,19 @@ extern "C" {
#define assume(cond) assert(cond)
#endif

/**
* @brief Wrapper function to silence "comparison is always false due to limited
* range of data type" type of warning when the warning is caused by a
* preprocessor configuration value that may be zero.
*
* @param[in] n Variable that may be zero
* @return The same variable @p n
*/
static inline unsigned may_be_zero(unsigned n)
{
return n;
}

#ifdef __cplusplus
}
#endif
Expand Down
15 changes: 10 additions & 5 deletions cpu/stm32/stm32_riotboot.mk
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,26 @@ ifneq (,$(filter $(CPU_FAM),f2 f4 f7))
# flash, to get evenly sized and distributed slots.
SLOT0_LEN ?= $(shell printf "0x%x" $$((($(ROM_LEN:%K=%*1024)-2*$(RIOTBOOT_LEN)) / $(NUM_SLOTS))))
SLOT1_LEN ?= $(SLOT0_LEN)
else ifeq (stm32l4,$(CPU_FAM))
else ifeq (l4,$(CPU_FAM))
# "The Vector table must be naturally aligned to a power of two whose alignment
# value is greater than or equal to number of Exceptions supported x 4"
# CPU_IRQ_NUMOFF for stm32l4 boards is < 91+16 so (107*4 bytes = 428 bytes ~= 0x200)
# CPU_IRQ_NUMOFF for stm32l4 boards is < 95+16 so (111*4 bytes = 444 bytes ~= 0x200)
# RIOTBOOT_HDR_LEN can be set to 0x200
RIOTBOOT_HDR_LEN ?= 0x200
else ifeq (stm32wb,$(CPU_FAM))
else ifeq (wb,$(CPU_FAM))
# "The Vector table must be naturally aligned to a power of two whose alignment
# value is greater than or equal to number of Exceptions supported x 4"
# CPU_IRQ_NUMOFF for stm32l4 boards is < 91+16 so (107*4 bytes = 428 bytes ~= 0x200)
# CPU_IRQ_NUMOFF for stm32wb boards is < 63+16 so (79*4 bytes = 316 bytes ~= 0x200)
# RIOTBOOT_HDR_LEN can be set to 0x200
RIOTBOOT_HDR_LEN ?= 0x200

# Slot size is determined by "((total_flash_size - RIOTBOOT_LEN) / 2)".
# If RIOTBOOT_LEN uses an odd number of flashpages, the remainder of the
# flash cannot be divided by two slots while staying FLASHPAGE_SIZE aligned.
RIOTBOOT_LEN ?= 0x2000
# 8KB are currently enough, set it to 16KB if USB-DFU or tinyUSB DFU is used
ifneq (,$(filter usbus_dfu tinyusb_dfu,$(USEMODULE)))
RIOTBOOT_LEN ?= 0x4000
else
RIOTBOOT_LEN ?= 0x2000
endif
endif
16 changes: 16 additions & 0 deletions sys/include/net/gnrc/ipv6/nib/conf.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,22 @@ extern "C" {
#endif
#endif

/**
* @brief Include a Route Information Option for subnets
* on other interfaces in normal Router Advertisements
* generated by @ref gnrc_ipv6_nib_change_rtr_adv_iface
*
* This is only needed if your node is an upstream router,
* but not the default router, but you want to propagate
* the information that the custom subnets it knows about
* should be routed through it instead of the default route.
*
* Requires the `gnrc_ipv6_nib_rio` module.
*/
#ifndef CONFIG_GNRC_IPV6_NIB_ADD_RIO_IN_RA
#define CONFIG_GNRC_IPV6_NIB_ADD_RIO_IN_RA 0
#endif

/**
* @brief Include a Route Information Option for subnets
* on other interfaces in the last Router Advertisement
Expand Down
113 changes: 81 additions & 32 deletions sys/net/gnrc/network_layer/ipv6/nib/_nib-router.c
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,78 @@ static inline uint16_t _nib_abr_entry_valid_offset(const _nib_abr_entry_t *abr)
}
#endif

/* check if a different prefix already includes the prefix of the entry */
static bool _has_better_match(const _nib_offl_entry_t *entry)
{
_nib_offl_entry_t *candidate = NULL;

while ((candidate = _nib_offl_iter(candidate))) {

if (candidate == entry) {
continue;
}

if (!(entry->mode & (_PL | _FT))) {
continue;
}

if (candidate->pfx_len >= entry->pfx_len) {
continue;
}

if (ipv6_addr_match_prefix(&entry->pfx, &candidate->pfx) == candidate->pfx_len) {
return true;
}
}

return false;
}

static gnrc_pktsnip_t *_add_rio(gnrc_netif_t *netif, gnrc_pktsnip_t *ext_opts, bool offl)
{
_nib_offl_entry_t *entry = NULL;
uint32_t now = evtimer_now_msec();

while ((entry = _nib_offl_iter(entry))) {

unsigned id = netif->pid;
if (_nib_onl_get_if(entry->next_hop) == id) {
continue;
}

if (entry->pfx_len == 0) {
continue;
}

if (offl && _has_better_match(entry)) {
continue;
}

bool local_pfx = (entry->mode & _PL) && (entry->flags & _PFX_ON_LINK);
bool routed_pfx = (entry->mode & _FT);

if (local_pfx || (offl && routed_pfx)) {

DEBUG("nib: adding downstream subnet to RA\n");
uint32_t valid_ltime = (entry->valid_until == UINT32_MAX) ? UINT32_MAX :
((entry->valid_until - now) / MS_PER_SEC);
gnrc_pktsnip_t *snip = gnrc_ndp_opt_ri_build(&entry->pfx,
entry->pfx_len,
valid_ltime,
NDP_OPT_RI_FLAGS_PRF_ZERO,
ext_opts);
if (snip != NULL) {
ext_opts = snip;
} else {
DEBUG_PUTS("nib: can't add RIO to RA - out of memory");
break;
}
}
}

return ext_opts;
}

static gnrc_pktsnip_t *_build_ext_opts(gnrc_netif_t *netif,
_nib_abr_entry_t *abr)
{
Expand Down Expand Up @@ -232,6 +304,12 @@ static gnrc_pktsnip_t *_build_ext_opts(gnrc_netif_t *netif,
}
}

/* advertise route to off-link subnets */
if (CONFIG_GNRC_IPV6_NIB_ADD_RIO_IN_RA) {
DEBUG("nib: add RIO to RA on interface %u\n", netif->pid);
ext_opts = _add_rio(netif, ext_opts, true);
}

return ext_opts;
}

Expand All @@ -243,39 +321,10 @@ static gnrc_pktsnip_t *_build_ext_opts(gnrc_netif_t *netif,
static gnrc_pktsnip_t *_build_final_ext_opts(gnrc_netif_t *netif)
{
gnrc_pktsnip_t *ext_opts = NULL;
_nib_offl_entry_t *entry = NULL;

if (!IS_USED(MODULE_GNRC_IPV6_NIB_RIO) ||
!IS_ACTIVE(CONFIG_GNRC_IPV6_NIB_ADD_RIO_IN_LAST_RA)) {
return NULL;
}

DEBUG("nib: sending final RA on interface %u\n", netif->pid);

uint32_t now = evtimer_now_msec();
while ((entry = _nib_offl_iter(entry))) {

unsigned id = netif->pid;
if (_nib_onl_get_if(entry->next_hop) == id) {
continue;
}

if ((entry->mode & _PL) && (entry->flags & _PFX_ON_LINK)) {
DEBUG("nib: adding downstream subnet to RA\n");
uint32_t valid_ltime = (entry->valid_until == UINT32_MAX) ? UINT32_MAX :
((entry->valid_until - now) / MS_PER_SEC);
gnrc_pktsnip_t *snip = gnrc_ndp_opt_ri_build(&entry->pfx,
entry->pfx_len,
valid_ltime,
NDP_OPT_RI_FLAGS_PRF_ZERO,
ext_opts);
if (snip != NULL) {
ext_opts = snip;
} else {
DEBUG_PUTS("nib: can't add RIO to RA - out of memory");
break;
}
}
if (CONFIG_GNRC_IPV6_NIB_ADD_RIO_IN_LAST_RA) {
DEBUG("nib: add RIO to final RA on interface %u\n", netif->pid);
ext_opts = _add_rio(netif, ext_opts, false);
}

return ext_opts;
Expand Down
12 changes: 12 additions & 0 deletions sys/net/gnrc/network_layer/sixlowpan/iphc/gnrc_sixlowpan_iphc.c
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@
#define NHC_IPV6_EXT_EID_MOB (0x04 << 1)
#define NHC_IPV6_EXT_EID_IPV6 (0x07 << 1)

#define SIXLOWPAN_IPHC_PREFIX_LEN (64) /**< minimum prefix length for IPHC */

/* currently only used with forwarding output, remove guard if more debug info
* is added */
#ifdef MODULE_GNRC_SIXLOWPAN_FRAG_VRB
Expand Down Expand Up @@ -1090,6 +1092,11 @@ static size_t _iphc_ipv6_encode(gnrc_pktsnip_t *pkt,
if (src_ctx && !(src_ctx->flags_id & GNRC_SIXLOWPAN_CTX_FLAGS_COMP)) {
src_ctx = NULL;
}
/* prefix bits not covered by context information must be zero */
if (src_ctx &&
ipv6_addr_match_prefix(&src_ctx->prefix, &ipv6_hdr->src) < SIXLOWPAN_IPHC_PREFIX_LEN) {
src_ctx = NULL;
}
}

if (!ipv6_addr_is_multicast(&ipv6_hdr->dst)) {
Expand All @@ -1099,6 +1106,11 @@ static size_t _iphc_ipv6_encode(gnrc_pktsnip_t *pkt,
if (dst_ctx && !(dst_ctx->flags_id & GNRC_SIXLOWPAN_CTX_FLAGS_COMP)) {
dst_ctx = NULL;
}
/* prefix bits not covered by context information must be zero */
if (dst_ctx &&
ipv6_addr_match_prefix(&dst_ctx->prefix, &ipv6_hdr->dst) < SIXLOWPAN_IPHC_PREFIX_LEN) {
dst_ctx = NULL;
}
}

/* if contexts available and both != 0 */
Expand Down
14 changes: 13 additions & 1 deletion sys/net/gnrc/routing/ipv6_auto_subnets/gnrc_ipv6_auto_subnets.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

/*
* Copyright (C) 2021 ML!PA Consulting GmbH
*
Expand Down Expand Up @@ -90,6 +89,7 @@
* @author Benjamin Valentin <benjamin.valentin@ml-pa.com>
*/

#include "compiler_hints.h"
#include "net/gnrc/ipv6.h"
#include "net/gnrc/netif.h"
#include "net/gnrc/netif/hdr.h"
Expand Down Expand Up @@ -142,6 +142,14 @@
#define CONFIG_GNRC_IPV6_AUTO_SUBNETS_PREFIX_FIX_LEN (0)
#endif

/**
* @brief Minimal length of a new prefix.
* e.g. Linux will only accept /64 prefixes for SLAAC
*/
#ifndef CONFIG_GNRC_IPV6_AUTO_SUBNETS_PREFIX_MIN_LEN
#define CONFIG_GNRC_IPV6_AUTO_SUBNETS_PREFIX_MIN_LEN (0)
#endif

/**
* @brief Number of subnets that can be configured.
*
Expand Down Expand Up @@ -370,6 +378,10 @@ static void _configure_subnets(uint8_t subnets, uint8_t start_idx, gnrc_netif_t
return;
}

if (new_prefix_len < may_be_zero(CONFIG_GNRC_IPV6_AUTO_SUBNETS_PREFIX_MIN_LEN)) {
new_prefix_len = CONFIG_GNRC_IPV6_AUTO_SUBNETS_PREFIX_MIN_LEN;
}

while ((downstream = gnrc_netif_iter(downstream))) {
gnrc_pktsnip_t *tmp;
ipv6_addr_t new_prefix;
Expand Down
61 changes: 61 additions & 0 deletions tests/net/gnrc_mac_timeout/tests/01-run.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#!/usr/bin/env python3

# Copyright (C) 2023 Inria
#
# 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.

import sys
from testrunner import run


ERROR_MS = 50 # This is large enough to pass the test on IoT-LAB nrf52dk


def testfunc(child):
child.expect(r"Testing gnrc_mac timeout module \(start time = (\d+) ms\)")
start = int(child.match.group(1))
child.expect(r"Set timeout_1, should be expired at (\d+) ms\)")
timeout_1 = int(child.match.group(1))
child.expect(r"Set timeout_2, should be expired at (\d+) ms\)")
timeout_2 = int(child.match.group(1))
child.expect(r"Set timeout_3, should be expired at (\d+) ms\)")
timeout_3 = int(child.match.group(1))
child.expect_exact("Are the reception times of all 3 msgs close to the supposed values?")

child.expect_exact("If yes, the tests were successful")
child.expect(
r"At (\d+) ms received msg 1: "
r"timeout_1 \(set at {start} ms\) expired, supposed to be {timeout_1} ms\!"
.format(start=start, timeout_1=timeout_1)
)
timeout_1_measured = int(child.match.group(1))
assert timeout_1_measured - timeout_1 < ERROR_MS
child.expect_exact(f"At {timeout_1_measured} ms: timeout_1 is not running.")
child.expect_exact(f"At {timeout_1_measured} ms: timeout_2 is running.")
child.expect_exact(f"At {timeout_1_measured} ms: timeout_3 is running.")
child.expect(
r"At (\d+) ms received msg 2: "
r"timeout_2 \(set at {start} ms\) expired, supposed to be {timeout_2} ms\!"
.format(start=start, timeout_2=timeout_2)
)
timeout_2_measured = int(child.match.group(1))
assert timeout_2_measured - timeout_2 < ERROR_MS
child.expect_exact(f"At {timeout_2_measured} ms: timeout_1 is not running.")
child.expect_exact(f"At {timeout_2_measured} ms: timeout_2 is not running.")
child.expect_exact(f"At {timeout_2_measured} ms: timeout_3 is running.")
child.expect(
r"At (\d+) ms received msg 3: "
r"timeout_3 \(set at {start} ms\) expired, supposed to be {timeout_3} ms\!"
.format(start=start, timeout_3=timeout_3)
)
timeout_3_measured = int(child.match.group(1))
assert timeout_3_measured - timeout_3 < ERROR_MS
child.expect_exact(f"At {timeout_3_measured} ms: timeout_1 is not running.")
child.expect_exact(f"At {timeout_3_measured} ms: timeout_2 is not running.")
child.expect_exact(f"At {timeout_3_measured} ms: timeout_3 is not running.")


if __name__ == "__main__":
sys.exit(run(testfunc))

0 comments on commit a272abb

Please sign in to comment.