From a86411b81e4a1bdf03df4610d4207158286591e2 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Wed, 20 Nov 2024 16:40:36 +0100 Subject: [PATCH] drivers/netdev: allow .send() to return > 0 to signal synchronos send --- drivers/include/net/netdev.h | 8 +++++++- pkg/lwip/contrib/netdev/lwip_netdev.c | 22 +++++++++++++--------- sys/net/gnrc/netif/gnrc_netif.c | 2 +- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/drivers/include/net/netdev.h b/drivers/include/net/netdev.h index 66ebc37c65..30fdaf9aa1 100644 --- a/drivers/include/net/netdev.h +++ b/drivers/include/net/netdev.h @@ -440,7 +440,7 @@ typedef struct netdev_driver { * @retval -ENETDOWN Device is powered down * @retval <0 Other error * @retval 0 Transmission successfully started - * @retval >0 Number of bytes transmitted (deprecated!) + * @retval >0 Number of bytes transmitted (transmission already complete) * * This function will cause the driver to start the transmission in an * async fashion. The driver will "own" the `iolist` until a subsequent @@ -448,6 +448,12 @@ typedef struct netdev_driver { * than `-EAGAIN`. The driver must signal completion using the * NETDEV_EVENT_TX_COMPLETE event, regardless of success or failure. * + * If the driver implements blocking send (e.g. because it writes out the + * frame byte-by-byte over a serial line) it can also return the number of bytes + * transmitted here directly. In this case it MUST NOT emit a NETDEV_EVENT_TX_COMPLETE + * event, netdev_driver_t::confirm_send will never be called but should still be + * implemented to signal conformance to the new API. + * * Old drivers might not be ported to the new API and have * netdev_driver_t::confirm_send set to `NULL`. In that case the driver * will return the number of bytes transmitted on success (instead of `0`) diff --git a/pkg/lwip/contrib/netdev/lwip_netdev.c b/pkg/lwip/contrib/netdev/lwip_netdev.c index dde6e631a9..592e6e59b8 100644 --- a/pkg/lwip/contrib/netdev/lwip_netdev.c +++ b/pkg/lwip/contrib/netdev/lwip_netdev.c @@ -275,8 +275,9 @@ static err_t _common_link_output(struct netif *netif, netdev_t *netdev, iolist_t { lwip_netif_dev_acquire(netif); + err_t res; if (is_netdev_legacy_api(netdev)) { - err_t res = (netdev->driver->send(netdev, iolist) > 0) ? ERR_OK : ERR_BUF; + res = (netdev->driver->send(netdev, iolist) > 0) ? ERR_OK : ERR_BUF; lwip_netif_dev_release(netif); return res; } @@ -288,7 +289,8 @@ static err_t _common_link_output(struct netif *netif, netdev_t *netdev, iolist_t compat_netif->thread_doing_tx = thread_get_active(); irq_restore(irq_state); - if (netdev->driver->send(netdev, iolist) < 0) { + res = netdev->driver->send(netdev, iolist); + if (res < 0) { lwip_netif_dev_release(netif); irq_state = irq_disable(); compat_netif->thread_doing_tx = NULL; @@ -303,17 +305,19 @@ static err_t _common_link_output(struct netif *netif, netdev_t *netdev, iolist_t 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"); + if (res == 0) { + /* async send */ + while (-EAGAIN == (res = 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) { + if (res < 0) { return ERR_IF; } diff --git a/sys/net/gnrc/netif/gnrc_netif.c b/sys/net/gnrc/netif/gnrc_netif.c index 541de5cf29..aa6cd4020f 100644 --- a/sys/net/gnrc/netif/gnrc_netif.c +++ b/sys/net/gnrc/netif/gnrc_netif.c @@ -1915,7 +1915,7 @@ static void _send(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt, bool push_back) * completed. For new netdevs (with confirm_send), TX is async. It is only * done if TX failed right away (res < 0). */ - if (gnrc_netif_netdev_legacy_api(netif) || (res < 0)) { + if (gnrc_netif_netdev_legacy_api(netif) || (res != 0)) { _tx_done(netif, pkt, tx_sync, res, push_back); } #if IS_USED(MODULE_NETDEV_NEW_API)