kaspar/RIOT
1
0
Fork 0
mirror of https://github.com/RIOT-OS/RIOT.git synced 2025-12-24 22:13:52 +01:00
Code Releases Activity

sys/net/gnrc/rpl: sync access to netstats

Browse Source 
Synchronize the RPL thread updating the RPL netstats with the RPL
shell command reading it by disabling IRQs. This will prevent printing
corrupted data on non-32bit platforms as well as printing inconsistent
data (e.g. TX count of old state in conjunction with TX bytes of new
state) for all platforms.

Co-authored-by: Martine Lenders <mail@martine-lenders.eu>
This commit is contained in:
Marian Buschsieweke 2022-06-28 18:09:51 +02:00
parent 93d8bade8e
commit 5a47d6a9b8
No known key found for this signature in database
GPG Key ID: CB8E3238CE715A94
3 changed files with 73 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
sys/include/net/gnrc/rpl.h
View File

@ -540,7 +540,17 @@ extern const ipv6_addr_t ipv6_addr_all_rpl_nodes;
#ifdef MODULE_NETSTATS_RPL
/**
* @brief Statistics for RPL control messages
* @brief Statistics for RPL control messages
* @warning Access to this structure need to be synchronized with the RPL
* thread to avoid reading corrupted valued. The RPL thread will
* disable IRQs while updating the fields. A reader that disables
* IRQs while reading values from here will read back correct and
* consistent values.
* @details Note that on 32 bit platforms reading individual fields usually is
* safe without locking, as a 32 bit read typically is an atomic
* operation. However, reasoning about e.g. the relation between TX
* packets and TX bytes will still require IRQs disabled, as the stats
* could be updated between the two reads even on 32 bit platforms.
*/
extern netstats_rpl_t gnrc_rpl_netstats;
#endif

32
sys/net/gnrc/routing/rpl/gnrc_rpl_internal/netstats.h
View File

@ -38,6 +38,9 @@ extern "C" {
*/
static inline void gnrc_rpl_netstats_rx_DIO(netstats_rpl_t *netstats, size_t len, int cast)
{
/* other threads (e.g. the shell thread via the rpl shell command) read
* out the data, so we have to sync accesses */
unsigned irq_state = irq_disable();
if (cast == GNRC_RPL_NETSTATS_MULTICAST) {
netstats->dio.rx_mcast_count++;
netstats->dio.rx_mcast_bytes += len;
@ -46,6 +49,7 @@ static inline void gnrc_rpl_netstats_rx_DIO(netstats_rpl_t *netstats, size_t len
netstats->dio.rx_ucast_count++;
netstats->dio.rx_ucast_bytes += len;
}
irq_restore(irq_state);
}
/**
@ -57,6 +61,9 @@ static inline void gnrc_rpl_netstats_rx_DIO(netstats_rpl_t *netstats, size_t len
*/
static inline void gnrc_rpl_netstats_tx_DIO(netstats_rpl_t *netstats, size_t len, int cast)
{
/* other threads (e.g. the shell thread via the rpl shell command) read
* out the data, so we have to sync accesses */
unsigned irq_state = irq_disable();
if (cast == GNRC_RPL_NETSTATS_MULTICAST) {
netstats->dio.tx_mcast_count++;
netstats->dio.tx_mcast_bytes += len;
@ -65,6 +72,7 @@ static inline void gnrc_rpl_netstats_tx_DIO(netstats_rpl_t *netstats, size_t len
netstats->dio.tx_ucast_count++;
netstats->dio.tx_ucast_bytes += len;
}
irq_restore(irq_state);
}
/**
@ -76,6 +84,9 @@ static inline void gnrc_rpl_netstats_tx_DIO(netstats_rpl_t *netstats, size_t len
*/
static inline void gnrc_rpl_netstats_rx_DIS(netstats_rpl_t *netstats, size_t len, int cast)
{
/* other threads (e.g. the shell thread via the rpl shell command) read
* out the data, so we have to sync accesses */
unsigned irq_state = irq_disable();
if (cast == GNRC_RPL_NETSTATS_MULTICAST) {
netstats->dis.rx_mcast_count++;
netstats->dis.rx_mcast_bytes += len;
@ -84,6 +95,7 @@ static inline void gnrc_rpl_netstats_rx_DIS(netstats_rpl_t *netstats, size_t len
netstats->dis.rx_ucast_count++;
netstats->dis.rx_ucast_bytes += len;
}
irq_restore(irq_state);
}
/**
@ -95,6 +107,9 @@ static inline void gnrc_rpl_netstats_rx_DIS(netstats_rpl_t *netstats, size_t len
*/
static inline void gnrc_rpl_netstats_tx_DIS(netstats_rpl_t *netstats, size_t len, int cast)
{
/* other threads (e.g. the shell thread via the rpl shell command) read
* out the data, so we have to sync accesses */
unsigned irq_state = irq_disable();
if (cast == GNRC_RPL_NETSTATS_MULTICAST) {
netstats->dis.tx_mcast_count++;
netstats->dis.tx_mcast_bytes += len;
@ -103,6 +118,7 @@ static inline void gnrc_rpl_netstats_tx_DIS(netstats_rpl_t *netstats, size_t len
netstats->dis.tx_ucast_count++;
netstats->dis.tx_ucast_bytes += len;
}
irq_restore(irq_state);
}
/**
@ -114,6 +130,9 @@ static inline void gnrc_rpl_netstats_tx_DIS(netstats_rpl_t *netstats, size_t len
*/
static inline void gnrc_rpl_netstats_rx_DAO(netstats_rpl_t *netstats, size_t len, int cast)
{
/* other threads (e.g. the shell thread via the rpl shell command) read
* out the data, so we have to sync accesses */
unsigned irq_state = irq_disable();
if (cast == GNRC_RPL_NETSTATS_MULTICAST) {
netstats->dao.rx_mcast_count++;
netstats->dao.rx_mcast_bytes += len;
@ -122,6 +141,7 @@ static inline void gnrc_rpl_netstats_rx_DAO(netstats_rpl_t *netstats, size_t len
netstats->dao.rx_ucast_count++;
netstats->dao.rx_ucast_bytes += len;
}
irq_restore(irq_state);
}
/**
@ -133,6 +153,9 @@ static inline void gnrc_rpl_netstats_rx_DAO(netstats_rpl_t *netstats, size_t len
*/
static inline void gnrc_rpl_netstats_tx_DAO(netstats_rpl_t *netstats, size_t len, int cast)
{
/* other threads (e.g. the shell thread via the rpl shell command) read
* out the data, so we have to sync accesses */
unsigned irq_state = irq_disable();
if (cast == GNRC_RPL_NETSTATS_MULTICAST) {
netstats->dao.tx_mcast_count++;
netstats->dao.tx_mcast_bytes += len;
@ -141,6 +164,7 @@ static inline void gnrc_rpl_netstats_tx_DAO(netstats_rpl_t *netstats, size_t len
netstats->dao.tx_ucast_count++;
netstats->dao.tx_ucast_bytes += len;
}
irq_restore(irq_state);
}
/**
@ -152,6 +176,9 @@ static inline void gnrc_rpl_netstats_tx_DAO(netstats_rpl_t *netstats, size_t len
*/
static inline void gnrc_rpl_netstats_rx_DAO_ACK(netstats_rpl_t *netstats, size_t len, int cast)
{
/* other threads (e.g. the shell thread via the rpl shell command) read
* out the data, so we have to sync accesses */
unsigned irq_state = irq_disable();
if (cast == GNRC_RPL_NETSTATS_MULTICAST) {
netstats->dao_ack.rx_mcast_count++;
netstats->dao_ack.rx_mcast_bytes += len;
@ -160,6 +187,7 @@ static inline void gnrc_rpl_netstats_rx_DAO_ACK(netstats_rpl_t *netstats, size_t
netstats->dao_ack.rx_ucast_count++;
netstats->dao_ack.rx_ucast_bytes += len;
}
irq_restore(irq_state);
}
/**
@ -171,6 +199,9 @@ static inline void gnrc_rpl_netstats_rx_DAO_ACK(netstats_rpl_t *netstats, size_t
*/
static inline void gnrc_rpl_netstats_tx_DAO_ACK(netstats_rpl_t *netstats, size_t len, int cast)
{
/* other threads (e.g. the shell thread via the rpl shell command) read
* out the data, so we have to sync accesses */
unsigned irq_state = irq_disable();
if (cast == GNRC_RPL_NETSTATS_MULTICAST) {
netstats->dao_ack.tx_mcast_count++;
netstats->dao_ack.tx_mcast_bytes += len;
@ -179,6 +210,7 @@ static inline void gnrc_rpl_netstats_tx_DAO_ACK(netstats_rpl_t *netstats, size_t
netstats->dao_ack.tx_ucast_count++;
netstats->dao_ack.tx_ucast_bytes += len;
}
irq_restore(irq_state);
}
#ifdef __cplusplus

34
sys/shell/commands/sc_gnrc_rpl.c
View File

@ -206,12 +206,38 @@ int _gnrc_rpl_send_dis(void)
#ifdef MODULE_NETSTATS_RPL
static void _print_stats_block(netstats_rpl_block_t *block, const char *name)
{
/* In the following we need to sync with the RPL thread via disabling IRQs
* to avoid reading corrupted data. The simpler strategy would be to
* disable IRQs during the whole printing (so in _stats()_, but stdio could
* be via a slow UART and, hence, have severe impact on the real time
* capabilities of the system. The second and simplest strategy would be to
* memcpy the whole netstats_rpl_t on to the stack with IRQs disabled and
* print the stack copy with IRQs re-enabled. However, that structure is
* 128 B in size, so that we would easily provoke a stack-overflow.
*
* Our strategy instead is to read the data four values at a time with
* IRQs disabled, and print them with IRQs re-enabled. The disadvantage is
* that stats may get updated while printing one group of values. However,
* the stats are grouped such that closely related values are read together.
* Hence, related metrics will always refer to the same state of the stats.
*/
unsigned irq_state = irq_disable();
uint32_t rx_ucast = block->rx_ucast_count;
uint32_t tx_ucast = block->tx_ucast_count;
uint32_t rx_mcast = block->rx_mcast_count;
uint32_t tx_mcast = block->tx_mcast_count;
irq_restore(irq_state);
printf("%7s #packets: %10" PRIu32 " / %-10" PRIu32 " %10" PRIu32 " / %-10" PRIu32 "\n",
name, block->rx_ucast_count, block->tx_ucast_count,
block->rx_mcast_count, block->tx_mcast_count);
name, rx_ucast, tx_ucast, rx_mcast, tx_mcast);
irq_state = irq_disable();
rx_ucast = block->rx_ucast_bytes;
tx_ucast = block->tx_ucast_bytes;
rx_mcast = block->rx_mcast_bytes;
tx_mcast = block->tx_mcast_bytes;
irq_restore(irq_state);
printf("%7s #bytes: %10" PRIu32 " / %-10" PRIu32 " %10" PRIu32 " / %-10" PRIu32 "\n",
name, block->rx_ucast_bytes, block->tx_ucast_bytes,
block->rx_mcast_bytes, block->tx_mcast_bytes);
name, rx_ucast, tx_ucast, rx_mcast, tx_mcast);
}
int _stats(void)