Merge pull request #16748 from jia200x/pr/gnrc_netif/remove_netdev_event_isr_msg

gnrc_netif: use event loops by default to process ISR

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

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

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
  *

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)))

sys/net/gnrc/link_layer/gomach/gomach.c

@@ -23,6 +23,7 @@
 #include <stdint.h>
 #include <stdbool.h>
 
+#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;
+        event_post(&netif->evq, &netif->event_isr);
-
-        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");
-        }
     }
     else {
         DEBUG("gnrc_netdev: event triggered -> %i\n", event);

sys/net/gnrc/link_layer/lwmac/lwmac.c

@@ -25,6 +25,7 @@
 #include <stdint.h>
 #include <stdbool.h>
 
+#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;
+        event_post(&netif->evq, &netif->event_isr);
-
-        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");
-        }
     }
     else {
         DEBUG("gnrc_netdev: event triggered -> %i\n", event);

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) {
-        while (1) {
+        /* Using messages for external IPC, and events for internal events */
-            /* Using messages for external IPC, and events for internal events */
 
-            /* First drain the queues before blocking the thread */
+        /* First drain the queues before blocking the thread */
-            /* Events will be handled before messages */
+        /* Events will be handled before messages */
-            DEBUG("gnrc_netif: handling events\n");
+        DEBUG("gnrc_netif: handling events\n");
-            event_t *evp;
+        event_t *evp;
-            /* We can not use event_loop() or event_wait() because then we would not
+        /* We can not use event_loop() or event_wait() because then we would not
-             * wake up when a message arrives */
+         * wake up when a message arrives */
-            event_queue_t *evq = _get_evq(netif);
+        event_queue_t *evq = &netif->evq;
-            while ((evp = event_get(evq))) {
+        while ((evp = event_get(evq))) {
-                DEBUG("gnrc_netif: event %p\n", (void *)evp);
+            DEBUG("gnrc_netif: event %p\n", (void *)evp);
-                if (evp->handler) {
+            if (evp->handler) {
-                    evp->handler(evp);
+                evp->handler(evp);
-                }
             }
-            /* 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);
         }
-    }
+        /* non-blocking msg check */
-    else {
+        int msg_waiting = msg_try_receive(msg);
-        /* Only messages used for event handling */
+        if (msg_waiting > 0) {
-        DEBUG("gnrc_netif: waiting for incoming messages\n");
+            return;
-        msg_receive(msg);
+        }
+        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->evq, &netif->event_isr);
-            _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.");
-            }
-        }
     }
     else {
         DEBUG("gnrc_netif: event triggered -> %i\n", event);

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,
+        event_post(&netif->evq, &netif->event_isr);
-                      .content = { .ptr = netif } };
-
-        if (msg_send(&msg, netif->pid) <= 0) {
-            DEBUG("gnrc_netif: possibly lost interrupt.\n");
-        }
     }
     else {
         DEBUG("gnrc_netif: event triggered -> %i\n", event);

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]);
 }