Merge pull request #18427 from maribu/lwip/confirm_send

pkg/lwip: make use of confirm send
RIOT-OS · May 6, 2024 · 3791db3 · 3791db3
2 parents a5996e2 + 28f412f
commit 3791db3
Show file tree

Hide file tree

Showing 20 changed files with 126 additions and 26 deletions.
diff --git a/cpu/esp_common/Makefile.dep b/cpu/esp_common/Makefile.dep
@@ -93,11 +93,14 @@ ifneq (,$(filter esp_wifi,$(USEMODULE)))
 endif
 
 ifneq (,$(filter esp_wifi_any,$(USEMODULE)))
-  USEMODULE += netdev_legacy_api
   USEMODULE += netopt
   USEMODULE += ztimer_msec
 endif
 
+ifneq (,$(filter esp_eth esp_wifi esp_now,$(USEMODULE)))
+  USEMODULE += netdev_legacy_api
+endif
+
 ifneq (,$(filter esp_freertos_common,$(USEMODULE)))
   ifneq (,$(filter ztimer_msec,$(USEMODULE)))
     USEMODULE += core_thread_flags

diff --git a/cpu/native/Makefile.dep b/cpu/native/Makefile.dep
@@ -40,6 +40,10 @@ ifneq (,$(filter socket_zep,$(USEMODULE)))
   endif
 endif
 
+ifneq (,$(filter netdev_tap,$(USEMODULE)))
+  USEMODULE += netdev_legacy_api
+endif
+
 ifneq (,$(filter libc_gettimeofday,$(USEMODULE)))
   USEMODULE += ztimer64_usec
 endif

diff --git a/cpu/nrf5x_common/Makefile.dep b/cpu/nrf5x_common/Makefile.dep
@@ -14,3 +14,7 @@ FEATURES_OPTIONAL += vdd_lc_filter_reg0
 ifneq (,$(filter nimble,$(USEPKG)))
   USEPKG += nrfx
 endif
+
+ifneq (,$(filter nrfmin,$(USEMODULE)))
+  USEMODULE += netdev_legacy_api
+endif
diff --git a/cpu/nrf5x_common/radio/nrfmin/Makefile.dep b/cpu/nrf5x_common/radio/nrfmin/Makefile.dep
@@ -0,0 +1 @@
+USEMODULE += netdev_legacy_api
diff --git a/drivers/kw41zrf/Makefile.dep b/drivers/kw41zrf/Makefile.dep
@@ -1,5 +1,6 @@
 USEMODULE += ieee802154
 USEMODULE += netdev_ieee802154
+USEMODULE += netdev_legacy_api
 USEMODULE += core_thread_flags
 USEMODULE += random
 USEMODULE += mcux_xcvr_mkw41z
diff --git a/examples/benchmark_udp/Makefile b/examples/benchmark_udp/Makefile
@@ -18,7 +18,6 @@ ifeq (0,$(LWIP))
   USEMODULE += gnrc_ipv6_default
 else
   USEMODULE += lwip_ipv6
-  USEMODULE += lwip_netdev
 endif
 
 # Add also the shell, some shell commands

diff --git a/examples/gcoap/Makefile b/examples/gcoap/Makefile
@@ -24,8 +24,6 @@ ifeq (,$(filter 1, $(LWIP_IPV4) $(LWIP_IPV6)))
   # Additional networking modules that can be dropped if not needed
   USEMODULE += gnrc_icmpv6_echo
 else
-  USEMODULE += lwip_netdev
-
   ifeq (1,$(LWIP_IPV4))
     USEMODULE += ipv4_addr
 

diff --git a/examples/gcoap_dtls/Makefile b/examples/gcoap_dtls/Makefile
@@ -24,8 +24,6 @@ ifeq (,$(filter 1, $(LWIP_IPV4) $(LWIP_IPV6)))
   # Additional networking modules that can be dropped if not needed
   USEMODULE += gnrc_icmpv6_echo
 else
-  USEMODULE += lwip_netdev
-
   ifeq (1,$(LWIP_IPV4))
     USEMODULE += ipv4_addr
 

diff --git a/examples/paho-mqtt/Makefile.lwip b/examples/paho-mqtt/Makefile.lwip
@@ -20,6 +20,5 @@ ifneq (0,$(LWIP_IPV6))
 endif
 
 ifneq (0,$(USE_LWIP))
-  USEMODULE += lwip_netdev
   USEMODULE += lwip
 endif
diff --git a/examples/telnet_server/Makefile b/examples/telnet_server/Makefile
@@ -24,7 +24,6 @@ ifeq (0,$(LWIP))
   USEMODULE += gnrc_icmpv6_echo
 else
   USEMODULE += lwip_ipv6
-  USEMODULE += lwip_netdev
 endif
 
 USEMODULE += netutils

diff --git a/pkg/lwip/Makefile.dep b/pkg/lwip/Makefile.dep
@@ -103,6 +103,11 @@ ifneq (,$(filter lwip_dhcp_auto,$(USEMODULE)))
   USEMODULE += lwip_dhcp
 endif
 
+# if an actual netdev is used, we need lwip_netdev to integrate it
+ifneq (,$(filter lwip_ethernet lwip_sixlowpan,$(USEMODULE)))
+  USEMODULE += lwip_netdev
+endif
+
 ifneq (,$(filter netif,$(USEMODULE)))
   USEMODULE += fmt
 endif
diff --git a/pkg/lwip/contrib/netdev/lwip_netdev.c b/pkg/lwip/contrib/netdev/lwip_netdev.c
@@ -36,14 +36,16 @@
 #include "netif/etharp.h"
 #include "netif/lowpan6.h"
 #include "thread.h"
+#include "thread_flags.h"
+#include "utlist.h"
 
 #define ENABLE_DEBUG                0
 #include "debug.h"
 
 #define LWIP_NETDEV_NAME            "lwip_netdev_mux"
 #define LWIP_NETDEV_PRIO            (THREAD_PRIORITY_MAIN - 4)
 #define LWIP_NETDEV_STACKSIZE       (THREAD_STACKSIZE_DEFAULT)
-#define LWIP_NETDEV_MSG_TYPE_EVENT 0x1235
+#define LWIP_NETDEV_MSG_TYPE_EVENT  0x1235
 
 #define ETHERNET_IFNAME1 'E'
 #define ETHERNET_IFNAME2 'T'
@@ -52,6 +54,7 @@
 #define WPAN_IFNAME2 'P'
 
 event_queue_t lwip_event_queue = { 0 };
+#define THREAD_FLAG_LWIP_TX_DONE    (1U << 11)
 
 static kernel_pid_t _pid = KERNEL_PID_UNDEF;
 static WORD_ALIGNED char _stack[LWIP_NETDEV_STACKSIZE];
@@ -76,6 +79,19 @@ static void _event_cb(netdev_t *dev, netdev_event_t event);
 static void *_event_loop(void *arg);
 static void _isr(event_t *ev);
 
+bool is_netdev_legacy_api(netdev_t *netdev)
+{
+    static_assert(IS_USED(MODULE_NETDEV_NEW_API) || IS_USED(MODULE_NETDEV_LEGACY_API),
+                  "used netdev misses dependency to netdev_legacy_api");
+    if (!IS_USED(MODULE_NETDEV_NEW_API)) {
+        return true;
+    }
+    if (!IS_USED(MODULE_NETDEV_LEGACY_API)) {
+        return false;
+    }
+    return (netdev->driver->confirm_send == NULL);
+}
+
 err_t lwip_netdev_init(struct netif *netif)
 {
     LWIP_ASSERT("netif != NULL", (netif != NULL));
@@ -254,6 +270,65 @@ err_t lwip_netdev_init(struct netif *netif)
     return res;
 }
 
+#if (IS_USED(MODULE_NETDEV_NEW_API))
+static err_t _common_link_output(struct netif *netif, netdev_t *netdev, iolist_t *iolist)
+{
+    lwip_netif_dev_acquire(netif);
+
+    if (is_netdev_legacy_api(netdev)) {
+        err_t res = (netdev->driver->send(netdev, iolist) > 0) ? ERR_OK : ERR_BUF;
+        lwip_netif_dev_release(netif);
+        return res;
+    }
+
+    unsigned irq_state;
+    lwip_netif_t *compat_netif = container_of(netif, lwip_netif_t, lwip_netif);
+
+    irq_state = irq_disable();
+    compat_netif->thread_doing_tx = thread_get_active();
+    irq_restore(irq_state);
+
+    if (netdev->driver->send(netdev, iolist) < 0) {
+        lwip_netif_dev_release(netif);
+        irq_state = irq_disable();
+        compat_netif->thread_doing_tx = NULL;
+        irq_restore(irq_state);
+        return ERR_IF;
+    }
+
+    /* block until TX completion is signaled from IRQ */
+    thread_flags_wait_any(THREAD_FLAG_LWIP_TX_DONE);
+
+    irq_state = irq_disable();
+    compat_netif->thread_doing_tx = NULL;
+    irq_restore(irq_state);
+
+    int retval;
+    while (-EAGAIN == (retval = netdev->driver->confirm_send(netdev, NULL))) {
+        /* this should not happen, as the driver really only should emit the
+         * TX done event when it is actually done. But better be safe than
+         * sorry */
+        DEBUG_PUTS("[lwip_netdev] confirm_send() returned -EAGAIN\n");
+    }
+
+    lwip_netif_dev_release(netif);
+
+    if (retval < 0) {
+        return ERR_IF;
+    }
+
+    return ERR_OK;
+}
+#else /* only old API */
+static err_t _common_link_output(struct netif *netif, netdev_t *netdev, iolist_t *iolist)
+{
+    lwip_netif_dev_acquire(netif);
+    err_t res = (netdev->driver->send(netdev, iolist) > 0) ? ERR_OK : ERR_BUF;
+    lwip_netif_dev_release(netif);
+    return res;
+}
+#endif
+
 #ifdef MODULE_NETDEV_ETH
 static err_t _eth_link_output(struct netif *netif, struct pbuf *p)
 {
@@ -282,10 +357,7 @@ static err_t _eth_link_output(struct netif *netif, struct pbuf *p)
 #if ETH_PAD_SIZE
     pbuf_header(p, ETH_PAD_SIZE); /* reclaim the padding word */
 #endif
-    lwip_netif_dev_acquire(netif);
-    err_t res = (netdev->driver->send(netdev, iolist) >= 0) ? ERR_OK : ERR_BUF;
-    lwip_netif_dev_release(netif);
-    return res;
+    return _common_link_output(netif, netdev, iolist);
 }
 #endif
 
@@ -299,10 +371,7 @@ static err_t _ieee802154_link_output(struct netif *netif, struct pbuf *p)
         .iol_len = (p->len - IEEE802154_FCS_LEN),   /* FCS is written by driver */
     };
 
-    lwip_netif_dev_acquire(netif);
-    err_t res = (netdev->driver->send(netdev, &pkt) >= 0) ? ERR_OK : ERR_BUF;
-    lwip_netif_dev_release(netif);
-    return res;
+    return _common_link_output(netif, netdev, &pkt);
 }
 #endif
 
@@ -331,10 +400,7 @@ static err_t _slip_link_output(struct netif *netif, struct pbuf *p)
         .iol_len = p->len,
     };
 
-    lwip_netif_dev_acquire(netif);
-    err_t res = (netdev->driver->send(netdev, &pkt) >= 0) ? ERR_OK : ERR_BUF;
-    lwip_netif_dev_release(netif);
-    return res;
+    return _common_link_output(netif, netdev, &pkt);
 }
 #endif
 
@@ -369,9 +435,24 @@ static void _event_cb(netdev_t *dev, netdev_event_t event)
 
     switch (event) {
     case NETDEV_EVENT_ISR:
+        DEBUG_PUTS("[lwip_netdev] NETDEV_EVENT_ISR");
         event_post(&lwip_event_queue, &compat_netif->ev_isr);
         break;
+#if (IS_USED(MODULE_NETDEV_NEW_API))
+    case NETDEV_EVENT_TX_COMPLETE:
+        DEBUG_PUTS("[lwip_netdev] NETDEV_EVENT_TX_COMPLETE");
+        {
+            unsigned irq_state = irq_disable();
+            thread_t *target = compat_netif->thread_doing_tx;
+            irq_restore(irq_state);
+            if (target) {
+                thread_flags_set(target, THREAD_FLAG_LWIP_TX_DONE);
+            }
+        }
+        break;
+#endif
     case NETDEV_EVENT_RX_COMPLETE:
+        DEBUG_PUTS("[lwip_netdev] NETDEV_EVENT_RX_COMPLETE");
         {
             struct pbuf *p = _get_recv_pkt(dev);
             if (p == NULL) {
@@ -385,10 +466,12 @@ static void _event_cb(netdev_t *dev, netdev_event_t event)
         }
         break;
     case NETDEV_EVENT_LINK_UP:
+        DEBUG_PUTS("[lwip_netdev] NETDEV_EVENT_LINK_UP");
         /* Will wake up DHCP state machine */
         netifapi_netif_set_link_up(netif);
         break;
     case NETDEV_EVENT_LINK_DOWN:
+        DEBUG_PUTS("[lwip_netdev] NETDEV_EVENT_LINK_DOWN");
         netifapi_netif_set_link_down(netif);
         break;
     default:

diff --git a/pkg/lwip/contrib/sys_arch.c b/pkg/lwip/contrib/sys_arch.c
@@ -223,7 +223,7 @@ sys_thread_t sys_thread_new(const char *name, lwip_thread_fn thread, void *arg,
     return res;
 }
 
-static kernel_pid_t lwip_tcpip_thread = KERNEL_PID_UNDEF;
+kernel_pid_t lwip_tcpip_thread = KERNEL_PID_UNDEF;
 static kernel_pid_t lwip_lock_thread;
 
 void sys_mark_tcpip_thread(void) {

diff --git a/pkg/lwip/include/arch/sys_arch.h b/pkg/lwip/include/arch/sys_arch.h
@@ -117,6 +117,8 @@ static inline void sys_mbox_set_invalid(sys_mbox_t *mbox)
 
 typedef kernel_pid_t sys_thread_t;      /**< Platform specific thread type */
 
+extern kernel_pid_t lwip_tcpip_thread;  /**< PID of the lwIP TCP/IP thread */
+
 /**
  * @name    Functions for locking/unlocking core to assure thread safety.
  * @{

diff --git a/pkg/lwip/include/lwip/netif/compat.h b/pkg/lwip/include/lwip/netif/compat.h
@@ -40,6 +40,9 @@ typedef struct {
 #ifdef MODULE_BHP_EVENT
     bhp_event_t bhp;                    /**< IPC Bottom Half Processor */
 #endif
+#if (IS_USED(MODULE_NETDEV_NEW_API))
+    thread_t *thread_doing_tx;          /**< The thread currently doing TX */
+#endif
 } lwip_netif_t;
 
 /**

diff --git a/sys/Makefile.dep b/sys/Makefile.dep
@@ -169,6 +169,10 @@ ifneq (,$(filter netdev_tap,$(USEMODULE)))
   USEMODULE += iolist
 endif
 
+ifneq (,$(filter netdev_test,$(USEMODULE)))
+  USEMODULE += netdev_legacy_api
+endif
+
 ifneq (,$(filter eui_provider,$(USEMODULE)))
   USEMODULE += luid
 endif

diff --git a/tests/pkg/lwip/Makefile b/tests/pkg/lwip/Makefile
@@ -18,7 +18,7 @@ ifneq (0, $(LWIP_IPV6))
 endif
 
 # including lwip_ipv6_mld would currently break this test on at86rf2xx radios
-USEMODULE += lwip lwip_netdev
+USEMODULE += lwip
 USEMODULE += lwip_udp
 USEMODULE += lwip_tcp
 USEMODULE += sock_async_event

diff --git a/tests/pkg/lwip_sock_ip/Makefile b/tests/pkg/lwip_sock_ip/Makefile
@@ -22,7 +22,6 @@ endif
 
 USEMODULE += inet_csum
 USEMODULE += l2util
-USEMODULE += lwip_netdev
 USEMODULE += netdev_eth
 USEMODULE += netdev_test
 USEMODULE += ps

diff --git a/tests/pkg/lwip_sock_tcp/Makefile b/tests/pkg/lwip_sock_tcp/Makefile
@@ -16,7 +16,6 @@ ifneq (0, $(LWIP_IPV6))
 endif
 
 USEMODULE += inet_csum
-USEMODULE += lwip_netdev
 USEMODULE += netdev_eth
 USEMODULE += netdev_test
 USEMODULE += ps

diff --git a/tests/pkg/lwip_sock_udp/Makefile b/tests/pkg/lwip_sock_udp/Makefile
@@ -22,7 +22,6 @@ endif
 
 USEMODULE += inet_csum
 USEMODULE += l2util
-USEMODULE += lwip_netdev
 USEMODULE += netdev_eth
 USEMODULE += netdev_test
 USEMODULE += ps
-Original file line number
+Diff line change
@@ Expand Up / @@ -117,6 +117,8 @@ static inline void sys_mbox_set_invalid(sys_mbox_t *mbox) @@
     typedef kernel_pid_t sys_thread_t;      /**< Platform specific thread type */
+    extern kernel_pid_t lwip_tcpip_thread;  /**< PID of the lwIP TCP/IP thread */
     /**
      * @name    Functions for locking/unlocking core to assure thread safety.
      * @{
@@ Expand Down @@