From 38798cb877ae95b51980685cfc38c171cd7e8bad Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Mon, 16 Aug 2021 15:36:33 +0200 Subject: [PATCH 1/5] gnrc_netif: use events instead of msg queue for ISR offload --- makefiles/pseudomodules.inc.mk | 1 - sys/include/net/gnrc/netif.h | 2 - sys/include/net/gnrc/netif/internal.h | 5 -- sys/net/gnrc/Makefile.dep | 7 +- sys/net/gnrc/netif/gnrc_netif.c | 100 ++++++-------------------- 5 files changed, 25 insertions(+), 90 deletions(-) diff --git a/makefiles/pseudomodules.inc.mk b/makefiles/pseudomodules.inc.mk index e28145130963..42c0df02a0b7 100644 --- a/makefiles/pseudomodules.inc.mk +++ b/makefiles/pseudomodules.inc.mk @@ -86,7 +86,6 @@ PSEUDOMODULES += gnrc_neterr PSEUDOMODULES += gnrc_netapi_callbacks PSEUDOMODULES += gnrc_netapi_mbox PSEUDOMODULES += gnrc_netif_bus -PSEUDOMODULES += gnrc_netif_events PSEUDOMODULES += gnrc_netif_timestamp PSEUDOMODULES += gnrc_pktbuf_cmd PSEUDOMODULES += gnrc_netif_6lo diff --git a/sys/include/net/gnrc/netif.h b/sys/include/net/gnrc/netif.h index d30a8a1c8cf4..01b5b2617002 100644 --- a/sys/include/net/gnrc/netif.h +++ b/sys/include/net/gnrc/netif.h @@ -139,7 +139,6 @@ typedef struct { * @see net_gnrc_netif_flags */ uint32_t flags; -#if IS_USED(MODULE_GNRC_NETIF_EVENTS) || defined(DOXYGEN) /** * @brief Event queue for asynchronous events */ @@ -148,7 +147,6 @@ typedef struct { * @brief ISR event for the network device */ event_t event_isr; -#endif /* MODULE_GNRC_NETIF_EVENTS */ #if (GNRC_NETIF_L2ADDR_MAXLEN > 0) || DOXYGEN /** * @brief The link-layer address currently used as the source address diff --git a/sys/include/net/gnrc/netif/internal.h b/sys/include/net/gnrc/netif/internal.h index efa4adc50778..4933985af747 100644 --- a/sys/include/net/gnrc/netif/internal.h +++ b/sys/include/net/gnrc/netif/internal.h @@ -40,11 +40,6 @@ extern "C" { */ #define GNRC_NETIF_PKTQ_DEQUEUE_MSG (0x1233) -/** - * @brief Message type for @ref netdev_event_t "netdev events" - */ -#define NETDEV_MSG_TYPE_EVENT (0x1234) - /** * @brief Acquires exclusive access to the interface * diff --git a/sys/net/gnrc/Makefile.dep b/sys/net/gnrc/Makefile.dep index b22bc9070740..2a60fc2aa14f 100644 --- a/sys/net/gnrc/Makefile.dep +++ b/sys/net/gnrc/Makefile.dep @@ -163,11 +163,6 @@ ifneq (,$(filter gnrc_netif_bus,$(USEMODULE))) USEMODULE += core_msg_bus endif -ifneq (,$(filter gnrc_netif_events,$(USEMODULE))) - USEMODULE += core_thread_flags - USEMODULE += event -endif - ifneq (,$(filter ieee802154 nrfmin esp_now cc110x gnrc_sixloenc,$(USEMODULE))) ifneq (,$(filter gnrc_ipv6, $(USEMODULE))) USEMODULE += gnrc_sixlowpan @@ -466,6 +461,8 @@ endif ifneq (,$(filter gnrc_netif_%,$(USEMODULE))) USEMODULE += gnrc_netif + USEMODULE += core_thread_flags + USEMODULE += event endif ifneq (,$(filter gnrc_netif_pktq,$(USEMODULE))) diff --git a/sys/net/gnrc/netif/gnrc_netif.c b/sys/net/gnrc/netif/gnrc_netif.c index 2eed989ff580..a1420fc1f9c6 100644 --- a/sys/net/gnrc/netif/gnrc_netif.c +++ b/sys/net/gnrc/netif/gnrc_netif.c @@ -1605,7 +1605,6 @@ int gnrc_netif_default_init(gnrc_netif_t *netif) static void _send(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt, bool push_back); -#if IS_USED(MODULE_GNRC_NETIF_EVENTS) /** * @brief Call the ISR handler from an event * @@ -1616,16 +1615,6 @@ static void _event_handler_isr(event_t *evp) gnrc_netif_t *netif = container_of(evp, gnrc_netif_t, event_isr); netif->dev->driver->isr(netif->dev); } -#endif - -static inline void _event_post(gnrc_netif_t *netif) -{ -#if IS_USED(MODULE_GNRC_NETIF_EVENTS) - event_post(&netif->evq, &netif->event_isr); -#else - (void)netif; -#endif -} static void _process_receive_stats(gnrc_netif_t *netdev, gnrc_pktsnip_t *pkt) { @@ -1648,24 +1637,6 @@ static void _process_receive_stats(gnrc_netif_t *netdev, gnrc_pktsnip_t *pkt) netstats_nb_update_rx(&netdev->netif, src, src_len, hdr->rssi, hdr->lqi); } -/** - * @brief Retrieve the netif event queue if enabled - * - * @param[in] netif gnrc_netif instance to operate on - * - * @return NULL if MODULE_GNRC_NETIF_EVENTS is not enabled - * @return gnrc_netif_t::evq if MODULE_GNRC_NETIF_EVENTS is enabled - */ -static inline event_queue_t *_get_evq(gnrc_netif_t *netif) -{ -#ifdef MODULE_GNRC_NETIF_EVENTS - return &netif->evq; -#else - (void)netif; - return NULL; -#endif -} - /** * @brief Process any pending events and wait for IPC messages * @@ -1679,37 +1650,30 @@ static inline event_queue_t *_get_evq(gnrc_netif_t *netif) */ static void _process_events_await_msg(gnrc_netif_t *netif, msg_t *msg) { - if (IS_USED(MODULE_GNRC_NETIF_EVENTS)) { - while (1) { - /* Using messages for external IPC, and events for internal events */ - - /* First drain the queues before blocking the thread */ - /* Events will be handled before messages */ - DEBUG("gnrc_netif: handling events\n"); - event_t *evp; - /* We can not use event_loop() or event_wait() because then we would not - * wake up when a message arrives */ - event_queue_t *evq = _get_evq(netif); - while ((evp = event_get(evq))) { - DEBUG("gnrc_netif: event %p\n", (void *)evp); - if (evp->handler) { - evp->handler(evp); - } - } - /* non-blocking msg check */ - int msg_waiting = msg_try_receive(msg); - if (msg_waiting > 0) { - return; + while (1) { + /* Using messages for external IPC, and events for internal events */ + + /* First drain the queues before blocking the thread */ + /* Events will be handled before messages */ + DEBUG("gnrc_netif: handling events\n"); + event_t *evp; + /* We can not use event_loop() or event_wait() because then we would not + * wake up when a message arrives */ + event_queue_t *evq = &netif->evq; + while ((evp = event_get(evq))) { + DEBUG("gnrc_netif: event %p\n", (void *)evp); + if (evp->handler) { + evp->handler(evp); } - DEBUG("gnrc_netif: waiting for events\n"); - /* Block the thread until something interesting happens */ - thread_flags_wait_any(THREAD_FLAG_MSG_WAITING | THREAD_FLAG_EVENT); } - } - else { - /* Only messages used for event handling */ - DEBUG("gnrc_netif: waiting for incoming messages\n"); - msg_receive(msg); + /* non-blocking msg check */ + int msg_waiting = msg_try_receive(msg); + if (msg_waiting > 0) { + return; + } + DEBUG("gnrc_netif: waiting for events\n"); + /* Block the thread until something interesting happens */ + thread_flags_wait_any(THREAD_FLAG_MSG_WAITING | THREAD_FLAG_EVENT); } } @@ -1831,7 +1795,6 @@ static void *_gnrc_netif_thread(void *args) _netif_ctx_t *ctx = args; gnrc_netapi_opt_t *opt; gnrc_netif_t *netif; - netdev_t *dev; int res; msg_t reply = { .type = GNRC_NETAPI_MSG_TYPE_ACK }; msg_t msg_queue[GNRC_NETIF_MSG_QUEUE_SIZE]; @@ -1841,11 +1804,9 @@ static void *_gnrc_netif_thread(void *args) gnrc_netif_acquire(netif); netif->pid = thread_getpid(); -#if IS_USED(MODULE_GNRC_NETIF_EVENTS) netif->event_isr.handler = _event_handler_isr, /* set up the event queue */ event_queue_init(&netif->evq); -#endif /* MODULE_GNRC_NETIF_EVENTS */ /* setup the link-layer's message queue */ msg_init_queue(msg_queue, GNRC_NETIF_MSG_QUEUE_SIZE); @@ -1857,7 +1818,6 @@ static void *_gnrc_netif_thread(void *args) LOG_ERROR("gnrc_netif: init failed: %d\n", ctx->result); return NULL; } - dev = netif->dev; #if DEVELHELP assert(options_tested); #endif @@ -1885,10 +1845,6 @@ static void *_gnrc_netif_thread(void *args) _send_queued_pkt(netif); break; #endif /* IS_USED(MODULE_GNRC_NETIF_PKTQ) */ - case NETDEV_MSG_TYPE_EVENT: - DEBUG("gnrc_netif: GNRC_NETDEV_MSG_TYPE_EVENT received\n"); - dev->driver->isr(dev); - break; case GNRC_NETAPI_MSG_TYPE_SND: DEBUG("gnrc_netif: GNRC_NETDEV_MSG_TYPE_SND received\n"); _send(netif, msg.content.ptr, false); @@ -1966,17 +1922,7 @@ static void _event_cb(netdev_t *dev, netdev_event_t event) gnrc_netif_t *netif = (gnrc_netif_t *) dev->context; if (event == NETDEV_EVENT_ISR) { - if (IS_USED(MODULE_GNRC_NETIF_EVENTS)) { - _event_post(netif); - } - else { - msg_t msg = { .type = NETDEV_MSG_TYPE_EVENT, - .content = { .ptr = netif } }; - - if (msg_send(&msg, netif->pid) <= 0) { - puts("gnrc_netif: possibly lost interrupt."); - } - } + event_post(&netif->evq, &netif->event_isr); } else { DEBUG("gnrc_netif: event triggered -> %i\n", event); From 081dfe0e64d3300453d8abf7bca86d6b3dfee4cb Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Mon, 16 Aug 2021 15:36:57 +0200 Subject: [PATCH 2/5] gnrc_netif: use event queue for ISR offload --- sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c b/sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c index 0bc1d1d9f9d0..d0cf4df17e7e 100644 --- a/sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c +++ b/sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c @@ -200,12 +200,7 @@ static void _driver_cb(netdev_t *dev, netdev_event_t event) gnrc_lorawan_t *mac = &netif->lorawan.mac; if (event == NETDEV_EVENT_ISR) { - msg_t msg = { .type = NETDEV_MSG_TYPE_EVENT, - .content = { .ptr = netif } }; - - if (msg_send(&msg, netif->pid) <= 0) { - DEBUG("gnrc_netif: possibly lost interrupt.\n"); - } + event_post(&netif->evq, &netif->event_isr); } else { DEBUG("gnrc_netif: event triggered -> %i\n", event); From 6040747f33316eedad78f713d134f69b0bdd9ab9 Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Mon, 16 Aug 2021 15:37:16 +0200 Subject: [PATCH 3/5] link_layer/gomach: use event queue for ISR offload --- sys/net/gnrc/link_layer/gomach/gomach.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/sys/net/gnrc/link_layer/gomach/gomach.c b/sys/net/gnrc/link_layer/gomach/gomach.c index cf2364a2c297..7dd51eed554c 100644 --- a/sys/net/gnrc/link_layer/gomach/gomach.c +++ b/sys/net/gnrc/link_layer/gomach/gomach.c @@ -23,6 +23,7 @@ #include #include +#include "event.h" #include "random.h" #include "timex.h" #include "periph/rtt.h" @@ -2031,14 +2032,7 @@ static void _gomach_event_cb(netdev_t *dev, netdev_event_t event) gnrc_netif_t *netif = (gnrc_netif_t *) dev->context; if (event == NETDEV_EVENT_ISR) { - msg_t msg; - - msg.type = NETDEV_MSG_TYPE_EVENT; - msg.content.ptr = (void *) netif; - - if (msg_send(&msg, netif->pid) <= 0) { - DEBUG("[GOMACH] gnrc_netdev: possibly lost interrupt.\n"); - } + event_post(&netif->evq, &netif->event_isr); } else { DEBUG("gnrc_netdev: event triggered -> %i\n", event); From 869c5f37a7027463575bbde984089b774b4f00ea Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Mon, 16 Aug 2021 15:37:27 +0200 Subject: [PATCH 4/5] link_layer/lwmac: use event queue for ISR offload --- sys/net/gnrc/link_layer/lwmac/lwmac.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/sys/net/gnrc/link_layer/lwmac/lwmac.c b/sys/net/gnrc/link_layer/lwmac/lwmac.c index cf5876a392ab..9393fbad561e 100644 --- a/sys/net/gnrc/link_layer/lwmac/lwmac.c +++ b/sys/net/gnrc/link_layer/lwmac/lwmac.c @@ -25,6 +25,7 @@ #include #include +#include "event.h" #include "od.h" #include "timex.h" #include "random.h" @@ -796,14 +797,7 @@ static void _lwmac_event_cb(netdev_t *dev, netdev_event_t event) gnrc_netif_t *netif = (gnrc_netif_t *) dev->context; if (event == NETDEV_EVENT_ISR) { - msg_t msg; - - msg.type = NETDEV_MSG_TYPE_EVENT; - msg.content.ptr = (void *) netif; - - if (msg_send(&msg, netif->pid) <= 0) { - LOG_WARNING("WARNING: [LWMAC] gnrc_netdev: possibly lost interrupt.\n"); - } + event_post(&netif->evq, &netif->event_isr); } else { DEBUG("gnrc_netdev: event triggered -> %i\n", event); From 67f90490441adaf7350f31302d4cfddb764b670d Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Tue, 10 May 2022 22:18:54 +0200 Subject: [PATCH 5/5] tests/gnrc_netif: remove cast from test_netif_get_name() --- tests/gnrc_netif/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/gnrc_netif/main.c b/tests/gnrc_netif/main.c index 96f47b7f159b..34fff1e905a0 100644 --- a/tests/gnrc_netif/main.c +++ b/tests/gnrc_netif/main.c @@ -1298,7 +1298,7 @@ static void test_netif_get_name(void) TEST_ASSERT_NOT_NULL(netif); res = netif_get_name(netif, name); - sprintf(exp_name, "%d", (int) ((gnrc_netif_t *)netif)->pid); + sprintf(exp_name, "%d", netif_get_id(netif)); TEST_ASSERT_EQUAL_INT(strlen(exp_name), res); TEST_ASSERT_EQUAL_STRING(&exp_name[0], &name[0]); }