From 7c7f6671085491b5e0d34d133eed3d6698d93362 Mon Sep 17 00:00:00 2001 From: Martine Lenders Date: Mon, 25 Mar 2019 11:15:15 +0100 Subject: [PATCH 1/3] gnrc_netif: add a send queue --- Makefile.dep | 4 + sys/include/net/gnrc/netif.h | 16 +++ sys/include/net/gnrc/netif/conf.h | 24 ++++ sys/include/net/gnrc/netif/internal.h | 7 +- sys/include/net/gnrc/netif/pktq.h | 138 ++++++++++++++++++++++ sys/include/net/gnrc/netif/pktq/type.h | 50 ++++++++ sys/net/gnrc/netif/Kconfig | 13 ++ sys/net/gnrc/netif/Makefile | 3 + sys/net/gnrc/netif/gnrc_netif.c | 15 ++- sys/net/gnrc/netif/pktq/Makefile | 3 + sys/net/gnrc/netif/pktq/gnrc_netif_pktq.c | 84 +++++++++++++ 11 files changed, 355 insertions(+), 2 deletions(-) create mode 100644 sys/include/net/gnrc/netif/pktq.h create mode 100644 sys/include/net/gnrc/netif/pktq/type.h create mode 100644 sys/net/gnrc/netif/pktq/Makefile create mode 100644 sys/net/gnrc/netif/pktq/gnrc_netif_pktq.c diff --git a/Makefile.dep b/Makefile.dep index dacaa1b387..e07d4c4f08 100644 --- a/Makefile.dep +++ b/Makefile.dep @@ -673,6 +673,10 @@ ifneq (,$(filter gnrc_netif_%,$(USEMODULE))) USEMODULE += gnrc_netif endif +ifneq (,$(filter gnrc_netif_pktq,$(USEMODULE))) + USEMODULE += xtimer +endif + ifneq (,$(filter netstats_%, $(USEMODULE))) USEMODULE += netstats endif diff --git a/sys/include/net/gnrc/netif.h b/sys/include/net/gnrc/netif.h index 63bb39fb52..fe04d658a9 100644 --- a/sys/include/net/gnrc/netif.h +++ b/sys/include/net/gnrc/netif.h @@ -59,6 +59,9 @@ #if IS_USED(MODULE_GNRC_NETIF_MAC) #include "net/gnrc/netif/mac.h" #endif +#ifdef MODULE_GNRC_NETIF_PKTQ +#include "net/gnrc/pktqueue.h" +#endif #include "net/ndp.h" #include "net/netdev.h" #include "net/netopt.h" @@ -67,6 +70,9 @@ #endif #include "rmutex.h" #include "net/netif.h" +#ifdef MODULE_GNRC_NETIF_PKTQ +#include "xtimer.h" +#endif #ifdef __cplusplus extern "C" { @@ -171,6 +177,16 @@ typedef struct { #endif #if IS_USED(MODULE_GNRC_NETIF_6LO) || defined(DOXYGEN) gnrc_netif_6lo_t sixlo; /**< 6Lo component */ +#endif +#if defined(MODULE_GNRC_NETIF_PKTQ) || DOXYGEN + /** + * @brief Packet queue for sending + * + * @note Only available with @ref net_gnrc_netif_pktq. + */ + gnrc_pktqueue_t *send_queue; + msg_t dequeue_msg; /**< message for gnrc_netif_t::dequeue_timer to send */ + xtimer_t dequeue_timer; /**< timer to schedule next sending of queued packets */ #endif uint8_t cur_hl; /**< Current hop-limit for out-going packets */ uint8_t device_type; /**< Device type */ diff --git a/sys/include/net/gnrc/netif/conf.h b/sys/include/net/gnrc/netif/conf.h index 7265d1ce09..58c73ff364 100644 --- a/sys/include/net/gnrc/netif/conf.h +++ b/sys/include/net/gnrc/netif/conf.h @@ -54,6 +54,30 @@ extern "C" { #define CONFIG_GNRC_NETIF_MSG_QUEUE_SIZE_EXP (4U) #endif +/** + * @brief Packet queue pool size for all network interfaces + * + * @note With @ref net_gnrc_sixlowpan_frag the queue should fit at least + * all fragments of the minimum MTU. + * @see net_gnrc_netif_pktq + */ +#ifndef CONFIG_GNRC_NETIF_PKTQ_POOL_SIZE +#define CONFIG_GNRC_NETIF_PKTQ_POOL_SIZE (16U) +#endif + +/** + * @brief Time in microseconds for when to try send a queued packet at the + * latest + * + * Set to -1 to deactivate dequeing by timer. For this it has to be ensured that + * none of the notifications by the driver are missed! + * + * @see net_gnrc_netif_pktq + */ +#ifndef CONFIG_GNRC_NETIF_PKTQ_TIMER_US +#define CONFIG_GNRC_NETIF_PKTQ_TIMER_US (5000U) +#endif + /** * @brief Number of multicast addresses needed for @ref net_gnrc_rpl "RPL". * diff --git a/sys/include/net/gnrc/netif/internal.h b/sys/include/net/gnrc/netif/internal.h index 07ae858f2c..e69665a594 100644 --- a/sys/include/net/gnrc/netif/internal.h +++ b/sys/include/net/gnrc/netif/internal.h @@ -35,10 +35,15 @@ extern "C" { #endif +/** + * @brief Message type to send from @ref net_gnrc_netif_pktq + */ +#define GNRC_NETIF_PKTQ_DEQUEUE_MSG (0x1233) + /** * @brief Message type for @ref netdev_event_t "netdev events" */ -#define NETDEV_MSG_TYPE_EVENT (0x1234) +#define NETDEV_MSG_TYPE_EVENT (0x1234) /** * @brief Acquires exclusive access to the interface diff --git a/sys/include/net/gnrc/netif/pktq.h b/sys/include/net/gnrc/netif/pktq.h new file mode 100644 index 0000000000..582f83757a --- /dev/null +++ b/sys/include/net/gnrc/netif/pktq.h @@ -0,0 +1,138 @@ +/* + * Copyright (C) 2019-20 Freie Universität Berlin + * + * This file is subject to the terms and conditions of the GNU Lesser + * General Public License v2.1. See the file LICENSE in the top level + * directory for more details. + */ + +/** + * @defgroup net_gnrc_netif_pktq Send queue for @ref net_gnrc_netif + * @ingroup net_gnrc_netif + * @brief + * @{ + * + * @file + * @brief @ref net_gnrc_netif_pktq definitions + * + * @author Martine S. Lenders + */ +#ifndef NET_GNRC_NETIF_PKTQ_H +#define NET_GNRC_NETIF_PKTQ_H + +#include +#include + +#include "net/gnrc/netif.h" +#include "net/gnrc/netif/pktq/type.h" +#include "net/gnrc/pkt.h" + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @brief Puts a packet into the packet send queue of a network interface + * + * @pre `netif != NULL` + * @pre `pkt != NULL` + * + * @param[in] netif A network interface. May not be NULL. + * @param[in] pkt A packet. May not be NULL. + * + * @return 0 on success + * @return -1 when the pool of available gnrc_pktqueue_t entries (of size + * @ref CONFIG_GNRC_NETIF_PKTQ_POOL_SIZE) is depleted + */ +int gnrc_netif_pktq_put(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt); + +/** + * @brief Gets a packet from the packet send queue of a network interface + * + * @pre `netif != NULL` + * + * @param[in] netif A network interface. May not be NULL. + * + * @return A packet on success + * @return NULL when the queue is empty + */ +static inline gnrc_pktsnip_t *gnrc_netif_pktq_get(gnrc_netif_t *netif) +{ +#if IS_USED(MODULE_GNRC_NETIF_PKTQ) + assert(netif != NULL); + + gnrc_pktsnip_t *pkt = NULL; + gnrc_pktqueue_t *entry = gnrc_pktqueue_remove_head( + &netif->send_queue.queue + ); + + if (entry != NULL) { + pkt = entry->pkt; + entry->pkt = NULL; + } + return pkt; +#else /* IS_USED(MODULE_GNRC_NETIF_PKTQ) */ + (void)netif; + return NULL; +#endif /* IS_USED(MODULE_GNRC_NETIF_PKTQ) */ +} + +/** + * @brief Schedule a dequeue notification to network interface + * + * The notification will be scheduled in @ref CONFIG_GNRC_NETIF_PKTQ_TIMER_US + * microseconds. + * + * @pre `netif != NULL` + * + * The signaling message can be used to send the next message in + * gnrc_netif_pktq_t::queue. + * + * @param[in] netif A network interface. May not be NULL. + */ +void gnrc_netif_pktq_sched_get(gnrc_netif_t *netif); + +/** + * @brief Pushes a packet back to the head of the packet send queue of a + * network interface + * + * @pre `netif != NULL` + * @pre `pkt != NULL` + * + * @param[in] netif A network interface. May not be NULL. + * @param[in] pkt A packet. May not be NULL. + * + * @return 0 on success + * @return -1 when the pool of available gnrc_pktqueue_t entries (of size + * @ref CONFIG_GNRC_NETIF_PKTQ_POOL_SIZE) is depleted + */ +int gnrc_netif_pktq_push_back(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt); + +/** + * @brief Check if a network interface's packet send queue is empty + * + * @pre `netif != NULL` + * + * @param[in] netif A network interface. May not be NULL. + * + * @return true, when the packet send queue of @p netif is empty + * @return false, otherwise + */ +static inline bool gnrc_netif_pktq_empty(gnrc_netif_t *netif) +{ +#if IS_USED(MODULE_GNRC_NETIF_PKTQ) + assert(netif != NULL); + + return (netif->send_queue.queue == NULL); +#else /* IS_USED(MODULE_GNRC_NETIF_PKTQ) */ + (void)netif; + return false; +#endif /* IS_USED(MODULE_GNRC_NETIF_PKTQ) */ +} + +#ifdef __cplusplus +} +#endif + +#endif /* NET_GNRC_NETIF_PKTQ_H */ +/** @} */ diff --git a/sys/include/net/gnrc/netif/pktq/type.h b/sys/include/net/gnrc/netif/pktq/type.h new file mode 100644 index 0000000000..95823b78d0 --- /dev/null +++ b/sys/include/net/gnrc/netif/pktq/type.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2019-20 Freie Universität Berlin + * + * This file is subject to the terms and conditions of the GNU Lesser + * General Public License v2.1. See the file LICENSE in the top level + * directory for more details. + */ + +/** + * @addtogroup net_gnrc_netif_pktq + * @brief + * @{ + * + * @file + * @brief @ref net_gnrc_netif_pktq type definitions + * + * Contained in its own file, so the type can be included in + * @ref gnrc_netif_t while the functions in net/gnrc/netif/pktq.h can use + * @ref gnrc_netif_t as operating type. + * + * @author Martine S. Lenders + */ +#ifndef NET_GNRC_NETIF_PKTQ_TYPE_H +#define NET_GNRC_NETIF_PKTQ_TYPE_H + +#include "net/gnrc/pktqueue.h" +#include "xtimer.h" + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @brief A packet queue for @ref net_gnrc_netif with a de-queue timer + */ +typedef struct { + gnrc_pktqueue_t *queue; /**< the actual packet queue class */ +#if CONFIG_GNRC_NETIF_PKTQ_TIMER_US >= 0 + msg_t dequeue_msg; /**< message for gnrc_netif_pktq_t::dequeue_timer to send */ + xtimer_t dequeue_timer; /**< timer to schedule next sending of + * queued packets */ +#endif +} gnrc_netif_pktq_t; + +#ifdef __cplusplus +} +#endif + +#endif /* NET_GNRC_NETIF_PKTQ_TYPE_H */ +/** @} */ diff --git a/sys/net/gnrc/netif/Kconfig b/sys/net/gnrc/netif/Kconfig index c1aa3a4928..0592ccdab0 100644 --- a/sys/net/gnrc/netif/Kconfig +++ b/sys/net/gnrc/netif/Kconfig @@ -48,4 +48,17 @@ config GNRC_NETIF_NONSTANDARD_6LO_MTU This is non compliant with RFC 4944 and might not be supported by other implementations. +config GNRC_NETIF_PKTQ_POOL_SIZE + int "Packet queue pool size for all network interfaces" + depends on USEMODULE_GNRC_NETIF_PKTQ + default 16 + +config GNRC_NETIF_PKTQ_TIMER_US + int "Time in microseconds for when to try to send a queued packet at the latest" + depends on USEMODULE_GNRC_NETIF_PKTQ + default 5000 + help + Set to -1 to deactivate dequeing by timer. For this it has to be ensured + that none of the notifications by the driver are missed! + endif # KCONFIG_USEMODULE_GNRC_NETIF diff --git a/sys/net/gnrc/netif/Makefile b/sys/net/gnrc/netif/Makefile index ca2ce0eb12..bff0b18138 100644 --- a/sys/net/gnrc/netif/Makefile +++ b/sys/net/gnrc/netif/Makefile @@ -9,6 +9,9 @@ endif ifneq (,$(filter gnrc_netif_init_devs,$(USEMODULE))) DIRS += init_devs endif +ifneq (,$(filter gnrc_netif_pktq,$(USEMODULE))) + DIRS += pktq +endif ifneq (,$(filter gnrc_netif_hdr,$(USEMODULE))) DIRS += hdr endif diff --git a/sys/net/gnrc/netif/gnrc_netif.c b/sys/net/gnrc/netif/gnrc_netif.c index 5a78d089a3..7b0c0055b3 100644 --- a/sys/net/gnrc/netif/gnrc_netif.c +++ b/sys/net/gnrc/netif/gnrc_netif.c @@ -1161,7 +1161,7 @@ static void _configure_netdev(netdev_t *dev) if (res < 0) { DEBUG("gnrc_netif: enable NETOPT_RX_END_IRQ failed: %d\n", res); } -#ifdef MODULE_NETSTATS_L2 +#if defined(MODULE_NETSTATS_L2) || defined(MODULE_GNRC_NETIF_PKTQ) res = dev->driver->set(dev, NETOPT_TX_END_IRQ, &enable, sizeof(enable)); if (res < 0) { DEBUG("gnrc_netif: enable NETOPT_TX_END_IRQ failed: %d\n", res); @@ -1292,6 +1292,8 @@ void gnrc_netif_default_init(gnrc_netif_t *netif) #endif } +static void _send(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt, bool requeue); + #if IS_USED(MODULE_GNRC_NETIF_EVENTS) /** * @brief Call the ISR handler from an event @@ -1547,10 +1549,21 @@ static void _event_cb(netdev_t *dev, netdev_event_t event) switch (event) { case NETDEV_EVENT_RX_COMPLETE: pkt = netif->ops->recv(netif); + /* send packet previously queued within netif due to the lower + * layer being busy. + * Further packets will be sent on later TX_COMPLETE */ + _send_queued_pkt(netif); if (pkt) { _pass_on_packet(pkt); } break; +#if defined(MODULE_NETSTATS_L2) || defined(MODULE_GNRC_NETIF_PKTQ) + case NETDEV_EVENT_TX_COMPLETE: + /* send packet previously queued within netif due to the lower + * layer being busy. + * Further packets will be sent on later TX_COMPLETE or + * TX_MEDIUM_BUSY */ + _send_queued_pkt(netif); #ifdef MODULE_NETSTATS_L2 case NETDEV_EVENT_TX_MEDIUM_BUSY: /* we are the only ones supposed to touch this variable, diff --git a/sys/net/gnrc/netif/pktq/Makefile b/sys/net/gnrc/netif/pktq/Makefile new file mode 100644 index 0000000000..79b24af463 --- /dev/null +++ b/sys/net/gnrc/netif/pktq/Makefile @@ -0,0 +1,3 @@ +MODULE := gnrc_netif_pktq + +include $(RIOTBASE)/Makefile.base diff --git a/sys/net/gnrc/netif/pktq/gnrc_netif_pktq.c b/sys/net/gnrc/netif/pktq/gnrc_netif_pktq.c new file mode 100644 index 0000000000..a1e32c602d --- /dev/null +++ b/sys/net/gnrc/netif/pktq/gnrc_netif_pktq.c @@ -0,0 +1,84 @@ +/* + * Copyright (C) 2019 Freie Universität Berlin + * + * This file is subject to the terms and conditions of the GNU Lesser + * General Public License v2.1. See the file LICENSE in the top level + * directory for more details. + */ + +/** + * @{ + * + * @file + * @author Martine Lenders + */ + +#include "net/gnrc/pktqueue.h" +#include "net/gnrc/netif/conf.h" +#include "net/gnrc/netif/internal.h" +#include "net/gnrc/netif/pktq.h" + +static gnrc_pktqueue_t _pool[CONFIG_GNRC_NETIF_PKTQ_POOL_SIZE]; + +static gnrc_pktqueue_t *_get_free_entry(void) +{ + for (unsigned i = 0; i < CONFIG_GNRC_NETIF_PKTQ_POOL_SIZE; i++) { + if (_pool[i].pkt == NULL) { + return &_pool[i]; + } + } + return NULL; +} + +int gnrc_netif_pktq_put(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt) +{ + assert(netif != NULL); + assert(pkt != NULL); + + gnrc_pktqueue_t *entry = _get_free_entry(); + + if (entry == NULL) { + return -1; + } + entry->pkt = pkt; + gnrc_pktqueue_add(&netif->send_queue.queue, entry); + return 0; +} + +void gnrc_netif_pktq_sched_get(gnrc_netif_t *netif) +{ +#if CONFIG_GNRC_NETIF_PKTQ_TIMER_US >= 0 + assert(netif != NULL); + netif->send_queue.dequeue_msg.type = GNRC_NETIF_PKTQ_DEQUEUE_MSG; + /* Prevent timer from firing while we add this. + * Otherwise the system might crash: The timer handler sets + * netif->send_queue.dequeue_msg.sender_pid to KERNEL_PID_ISR while + * the message is added to the timer, causing the next round of the timer + * handler to try to send the message to IPC, leaving the system in an + * invalid state. */ + unsigned state = irq_disable(); + xtimer_set_msg(&netif->send_queue.dequeue_timer, + CONFIG_GNRC_NETIF_PKTQ_TIMER_US, + &netif->send_queue.dequeue_msg, netif->pid); + irq_restore(state); +#else /* CONFIG_GNRC_NETIF_PKTQ_TIMER_US >= 0 */ + (void)netif; +#endif /* CONFIG_GNRC_NETIF_PKTQ_TIMER_US >= 0 */ +} + +int gnrc_netif_pktq_push_back(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt) +{ + assert(netif != NULL); + assert(pkt != NULL); + + gnrc_pktqueue_t *entry = _get_free_entry(); + + if (entry == NULL) { + return -1; + } + entry->pkt = pkt; + LL_PREPEND(netif->send_queue.queue, entry); + return 0; +} + +/** @} */ From a72d0ef3e84e211ce06da9504151e92385ef657a Mon Sep 17 00:00:00 2001 From: Martine Lenders Date: Mon, 25 Mar 2019 12:45:09 +0100 Subject: [PATCH 2/3] gnrc_netif: add packet to queue when device is busy ... and also send on send error (i.e. when *medium* was busy) --- sys/include/net/gnrc/netif.h | 15 ++-- sys/net/gnrc/netif/gnrc_netif.c | 136 ++++++++++++++++++++++++++------ 2 files changed, 115 insertions(+), 36 deletions(-) diff --git a/sys/include/net/gnrc/netif.h b/sys/include/net/gnrc/netif.h index fe04d658a9..652e19b681 100644 --- a/sys/include/net/gnrc/netif.h +++ b/sys/include/net/gnrc/netif.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2017 Freie Universität Berlin + * Copyright (C) 2017-20 Freie Universität Berlin * * This file is subject to the terms and conditions of the GNU Lesser * General Public License v2.1. See the file LICENSE in the top level @@ -59,8 +59,8 @@ #if IS_USED(MODULE_GNRC_NETIF_MAC) #include "net/gnrc/netif/mac.h" #endif -#ifdef MODULE_GNRC_NETIF_PKTQ -#include "net/gnrc/pktqueue.h" +#if IS_USED(MODULE_GNRC_NETIF_PKTQ) +#include "net/gnrc/netif/pktq/type.h" #endif #include "net/ndp.h" #include "net/netdev.h" @@ -70,9 +70,6 @@ #endif #include "rmutex.h" #include "net/netif.h" -#ifdef MODULE_GNRC_NETIF_PKTQ -#include "xtimer.h" -#endif #ifdef __cplusplus extern "C" { @@ -178,15 +175,13 @@ typedef struct { #if IS_USED(MODULE_GNRC_NETIF_6LO) || defined(DOXYGEN) gnrc_netif_6lo_t sixlo; /**< 6Lo component */ #endif -#if defined(MODULE_GNRC_NETIF_PKTQ) || DOXYGEN +#if IS_USED(MODULE_GNRC_NETIF_PKTQ) || defined(DOXYGEN) /** * @brief Packet queue for sending * * @note Only available with @ref net_gnrc_netif_pktq. */ - gnrc_pktqueue_t *send_queue; - msg_t dequeue_msg; /**< message for gnrc_netif_t::dequeue_timer to send */ - xtimer_t dequeue_timer; /**< timer to schedule next sending of queued packets */ + gnrc_netif_pktq_t send_queue; #endif uint8_t cur_hl; /**< Current hop-limit for out-going packets */ uint8_t device_type; /**< Device type */ diff --git a/sys/net/gnrc/netif/gnrc_netif.c b/sys/net/gnrc/netif/gnrc_netif.c index 7b0c0055b3..ec20ba31e7 100644 --- a/sys/net/gnrc/netif/gnrc_netif.c +++ b/sys/net/gnrc/netif/gnrc_netif.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2014-2017 Freie Universität Berlin + * Copyright (C) 2014-20 Freie Universität Berlin * * This file is subject to the terms and conditions of the GNU Lesser * General Public License v2.1. See the file LICENSE in the top level @@ -27,6 +27,7 @@ #include "net/gnrc/ipv6/nib.h" #include "net/gnrc/ipv6.h" #endif /* MODULE_GNRC_IPV6_NIB */ +#include "net/gnrc/netif/pktq.h" #ifdef MODULE_NETSTATS #include "net/netstats.h" #endif @@ -1161,12 +1162,12 @@ static void _configure_netdev(netdev_t *dev) if (res < 0) { DEBUG("gnrc_netif: enable NETOPT_RX_END_IRQ failed: %d\n", res); } -#if defined(MODULE_NETSTATS_L2) || defined(MODULE_GNRC_NETIF_PKTQ) - res = dev->driver->set(dev, NETOPT_TX_END_IRQ, &enable, sizeof(enable)); - if (res < 0) { - DEBUG("gnrc_netif: enable NETOPT_TX_END_IRQ failed: %d\n", res); + if (IS_USED(MODULE_NETSTATS_L2) || IS_USED(MODULE_GNRC_NETIF_PKTQ)) { + res = dev->driver->set(dev, NETOPT_TX_END_IRQ, &enable, sizeof(enable)); + if (res < 0) { + DEBUG("gnrc_netif: enable NETOPT_TX_END_IRQ failed: %d\n", res); + } } -#endif } #ifdef DEVELHELP @@ -1292,7 +1293,7 @@ void gnrc_netif_default_init(gnrc_netif_t *netif) #endif } -static void _send(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt, bool requeue); +static void _send(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt, bool push_back); #if IS_USED(MODULE_GNRC_NETIF_EVENTS) /** @@ -1381,6 +1382,83 @@ static void _process_events_await_msg(gnrc_netif_t *netif, msg_t *msg) } } +static void _send_queued_pkt(gnrc_netif_t *netif) +{ + if (IS_USED(MODULE_GNRC_NETIF_PKTQ)) { + gnrc_pktsnip_t *pkt; + + if ((pkt = gnrc_netif_pktq_get(netif)) != NULL) { + _send(netif, pkt, true); + gnrc_netif_pktq_sched_get(netif); + } + } +} + +static void _send(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt, bool push_back) +{ + int res; + + if (IS_USED(MODULE_GNRC_NETIF_PKTQ)) { + /* send queued packets first to keep order */ + if (!push_back && !gnrc_netif_pktq_empty(netif)) { + int put_res; + + put_res = gnrc_netif_pktq_put(netif, pkt); + if (put_res == 0) { + DEBUG("gnrc_netif: (re-)queued pkt %p\n", (void *)pkt); + _send_queued_pkt(netif); + return; + } + else { + LOG_WARNING("gnrc_netif: can't queue packet for sending\n"); + /* try to send anyway */ + } + } + /* hold in case device was busy to not having to rewrite *all* the link + * layer implementations in case `gnrc_netif_pktq` is included */ + gnrc_pktbuf_hold(pkt, 1); + } + res = netif->ops->send(netif, pkt); + if (IS_USED(MODULE_GNRC_NETIF_PKTQ) && (res == -EBUSY)) { + int put_res; + + /* Lower layer was busy. + * Since "busy" could also mean that the lower layer is currently + * receiving, trying to wait for the device not being busy any more + * could run into the risk of overriding the received packet on send + * Rather, queue the packet within the netif now and try to send them + * again after the device completed its busy state. */ + if (push_back) { + put_res = gnrc_netif_pktq_push_back(netif, pkt); + } + else { + put_res = gnrc_netif_pktq_put(netif, pkt); + gnrc_netif_pktq_sched_get(netif); + } + if (put_res == 0) { + DEBUG("gnrc_netif: (re-)queued pkt %p\n", (void *)pkt); + return; /* early return to not release */ + } + else { + LOG_ERROR("gnrc_netif: can't queue packet for sending\n"); + } + return; + } + else if (IS_USED(MODULE_GNRC_NETIF_PKTQ)) { + /* remove previously held packet */ + gnrc_pktbuf_release(pkt); + } + if (res < 0) { + DEBUG("gnrc_netif: error sending packet %p (code: %i)\n", + (void *)pkt, res); + } +#ifdef MODULE_NETSTATS_L2 + else { + netif->stats.tx_bytes += res; + } +#endif +} + static void *_gnrc_netif_thread(void *args) { gnrc_netapi_opt_t *opt; @@ -1442,22 +1520,19 @@ static void *_gnrc_netif_thread(void *args) /* dispatch netdev, MAC and gnrc_netapi messages */ DEBUG("gnrc_netif: message %u\n", (unsigned)msg.type); switch (msg.type) { +#if IS_USED(MODULE_GNRC_NETIF_PKTQ) + case GNRC_NETIF_PKTQ_DEQUEUE_MSG: + DEBUG("gnrc_netif: send from packet send queue\n"); + _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"); - res = netif->ops->send(netif, msg.content.ptr); - if (res < 0) { - DEBUG("gnrc_netif: error sending packet %p (code: %i)\n", - msg.content.ptr, res); - } -#ifdef MODULE_NETSTATS_L2 - else { - netif->stats.tx_bytes += res; - } -#endif + _send(netif, msg.content.ptr, false); #if (CONFIG_GNRC_NETIF_MIN_WAIT_AFTER_SEND_US > 0U) xtimer_periodic_wakeup( &last_wakeup, @@ -1557,25 +1632,34 @@ static void _event_cb(netdev_t *dev, netdev_event_t event) _pass_on_packet(pkt); } break; -#if defined(MODULE_NETSTATS_L2) || defined(MODULE_GNRC_NETIF_PKTQ) +#if IS_USED(MODULE_NETSTATS_L2) || IS_USED(MODULE_GNRC_NETIF_PKTQ) case NETDEV_EVENT_TX_COMPLETE: /* send packet previously queued within netif due to the lower * layer being busy. * Further packets will be sent on later TX_COMPLETE or * TX_MEDIUM_BUSY */ _send_queued_pkt(netif); -#ifdef MODULE_NETSTATS_L2 - case NETDEV_EVENT_TX_MEDIUM_BUSY: - /* we are the only ones supposed to touch this variable, - * so no acquire necessary */ - netif->stats.tx_failed++; - break; - case NETDEV_EVENT_TX_COMPLETE: +#if IS_USED(MODULE_NETSTATS_L2) /* we are the only ones supposed to touch this variable, * so no acquire necessary */ netif->stats.tx_success++; +#endif /* IS_USED(MODULE_NETSTATS_L2) */ break; -#endif +#endif /* IS_USED(MODULE_NETSTATS_L2) || IS_USED(MODULE_GNRC_NETIF_PKTQ) */ +#if IS_USED(MODULE_NETSTATS_L2) || IS_USED(MODULE_GNRC_NETIF_PKTQ) + case NETDEV_EVENT_TX_MEDIUM_BUSY: + /* send packet previously queued within netif due to the lower + * layer being busy. + * Further packets will be sent on later TX_COMPLETE or + * TX_MEDIUM_BUSY */ + _send_queued_pkt(netif); +#if IS_USED(MODULE_NETSTATS_L2) + /* we are the only ones supposed to touch this variable, + * so no acquire necessary */ + netif->stats.tx_failed++; +#endif /* IS_USED(MODULE_NETSTATS_L2) */ + break; +#endif /* IS_USED(MODULE_NETSTATS_L2) || IS_USED(MODULE_GNRC_NETIF_PKTQ) */ default: DEBUG("gnrc_netif: warning: unhandled event %u.\n", event); } From 818097f0dc3817470db23ce2b3aaa85c2942a9ea Mon Sep 17 00:00:00 2001 From: Martine Lenders Date: Mon, 25 Mar 2019 14:33:01 +0100 Subject: [PATCH 3/3] tests: provide unittests for gnrc_netif_pktq --- .../unittests/tests-gnrc_netif_pktq/Makefile | 1 + .../tests-gnrc_netif_pktq/Makefile.include | 3 + .../tests-gnrc_netif_pktq.c | 141 ++++++++++++++++++ .../tests-gnrc_netif_pktq.h | 35 +++++ 4 files changed, 180 insertions(+) create mode 100644 tests/unittests/tests-gnrc_netif_pktq/Makefile create mode 100644 tests/unittests/tests-gnrc_netif_pktq/Makefile.include create mode 100644 tests/unittests/tests-gnrc_netif_pktq/tests-gnrc_netif_pktq.c create mode 100644 tests/unittests/tests-gnrc_netif_pktq/tests-gnrc_netif_pktq.h diff --git a/tests/unittests/tests-gnrc_netif_pktq/Makefile b/tests/unittests/tests-gnrc_netif_pktq/Makefile new file mode 100644 index 0000000000..48422e909a --- /dev/null +++ b/tests/unittests/tests-gnrc_netif_pktq/Makefile @@ -0,0 +1 @@ +include $(RIOTBASE)/Makefile.base diff --git a/tests/unittests/tests-gnrc_netif_pktq/Makefile.include b/tests/unittests/tests-gnrc_netif_pktq/Makefile.include new file mode 100644 index 0000000000..72aa5c7b7c --- /dev/null +++ b/tests/unittests/tests-gnrc_netif_pktq/Makefile.include @@ -0,0 +1,3 @@ +USEMODULE += gnrc_netif_pktq + +CFLAGS += -DCONFIG_GNRC_NETIF_PKTQ_POOL_SIZE=4 diff --git a/tests/unittests/tests-gnrc_netif_pktq/tests-gnrc_netif_pktq.c b/tests/unittests/tests-gnrc_netif_pktq/tests-gnrc_netif_pktq.c new file mode 100644 index 0000000000..dded729fe5 --- /dev/null +++ b/tests/unittests/tests-gnrc_netif_pktq/tests-gnrc_netif_pktq.c @@ -0,0 +1,141 @@ +/* + * Copyright (C) 2019 Freie Universität Berlin + * + * This file is subject to the terms and conditions of the GNU Lesser + * General Public License v2.1. See the file LICENSE in the top level + * directory for more details. + */ + +/** + * @{ + * + * @file + * @author Martine Lenders + */ + +#include "embUnit.h" + +#include "net/gnrc/netif/conf.h" +#include "net/gnrc/netif/pktq.h" + +#include "tests-gnrc_netif_pktq.h" + +gnrc_netif_t _netif; + +static void set_up(void) +{ + while (gnrc_netif_pktq_get(&_netif)) { } +} + +static void test_pktq_get__empty(void) +{ + TEST_ASSERT_NULL(gnrc_netif_pktq_get(&_netif)); +} + +static void test_pktq_put__full(void) +{ + gnrc_pktsnip_t pkt; + + for (unsigned i = 0; i < CONFIG_GNRC_NETIF_PKTQ_POOL_SIZE; i++) { + TEST_ASSERT_EQUAL_INT(0, gnrc_netif_pktq_put(&_netif, &pkt)); + } + TEST_ASSERT_EQUAL_INT(-1, gnrc_netif_pktq_put(&_netif, &pkt)); +} + +static void test_pktq_put_get1(void) +{ + gnrc_pktsnip_t pkt_in, *pkt_out; + + TEST_ASSERT_EQUAL_INT(0, gnrc_netif_pktq_put(&_netif, &pkt_in)); + TEST_ASSERT_NOT_NULL((pkt_out = gnrc_netif_pktq_get(&_netif))); + TEST_ASSERT(&pkt_in == pkt_out); +} + +static void test_pktq_put_get3(void) +{ + gnrc_pktsnip_t pkt_in[3]; + + for (unsigned i = 0; i < 3; i++) { + TEST_ASSERT_EQUAL_INT(0, gnrc_netif_pktq_put(&_netif, &pkt_in[i])); + } + for (unsigned i = 0; i < 3; i++) { + gnrc_pktsnip_t *pkt_out; + + TEST_ASSERT_NOT_NULL((pkt_out = gnrc_netif_pktq_get(&_netif))); + TEST_ASSERT(&pkt_in[i] == pkt_out); + } +} + +static void test_pktq_push_back__full(void) +{ + gnrc_pktsnip_t pkt; + + for (unsigned i = 0; i < CONFIG_GNRC_NETIF_PKTQ_POOL_SIZE; i++) { + TEST_ASSERT_EQUAL_INT(0, gnrc_netif_pktq_put(&_netif, &pkt)); + } + TEST_ASSERT_EQUAL_INT(-1, gnrc_netif_pktq_push_back(&_netif, &pkt)); +} + +static void test_pktq_push_back_get1(void) +{ + gnrc_pktsnip_t pkt_in, *pkt_out; + + TEST_ASSERT_EQUAL_INT(0, gnrc_netif_pktq_push_back(&_netif, &pkt_in)); + TEST_ASSERT_NOT_NULL((pkt_out = gnrc_netif_pktq_get(&_netif))); + TEST_ASSERT(&pkt_in == pkt_out); +} + +static void test_pktq_push_back_get3(void) +{ + gnrc_pktsnip_t pkt_in[3]; + + for (unsigned i = 0; i < 3; i++) { + TEST_ASSERT_EQUAL_INT(0, gnrc_netif_pktq_push_back(&_netif, &pkt_in[i])); + } + for (unsigned i = 0; i < 3; i++) { + gnrc_pktsnip_t *pkt_out; + + TEST_ASSERT_NOT_NULL((pkt_out = gnrc_netif_pktq_get(&_netif))); + TEST_ASSERT(&pkt_in[3 - i - 1] == pkt_out); + } +} + +static void test_pktq_empty(void) +{ + gnrc_pktsnip_t pkt_in; + + TEST_ASSERT(gnrc_netif_pktq_empty(&_netif)); + TEST_ASSERT_EQUAL_INT(0, gnrc_netif_pktq_put(&_netif, &pkt_in)); + TEST_ASSERT(!gnrc_netif_pktq_empty(&_netif)); + TEST_ASSERT_NOT_NULL(gnrc_netif_pktq_get(&_netif)); + TEST_ASSERT(gnrc_netif_pktq_empty(&_netif)); + TEST_ASSERT_EQUAL_INT(0, gnrc_netif_pktq_push_back(&_netif, &pkt_in)); + TEST_ASSERT(!gnrc_netif_pktq_empty(&_netif)); + TEST_ASSERT_NOT_NULL(gnrc_netif_pktq_get(&_netif)); + TEST_ASSERT(gnrc_netif_pktq_empty(&_netif)); +} + +static Test *test_gnrc_netif_pktq(void) +{ + EMB_UNIT_TESTFIXTURES(fixtures) { + new_TestFixture(test_pktq_get__empty), + new_TestFixture(test_pktq_put__full), + new_TestFixture(test_pktq_put_get1), + new_TestFixture(test_pktq_put_get3), + new_TestFixture(test_pktq_push_back__full), + new_TestFixture(test_pktq_push_back_get1), + new_TestFixture(test_pktq_push_back_get3), + new_TestFixture(test_pktq_empty), + }; + + EMB_UNIT_TESTCALLER(pktq_tests, set_up, NULL, fixtures); + + return (Test *)&pktq_tests; +} + +void tests_gnrc_netif_pktq(void) +{ + TESTS_RUN(test_gnrc_netif_pktq()); +} + +/** @} */ diff --git a/tests/unittests/tests-gnrc_netif_pktq/tests-gnrc_netif_pktq.h b/tests/unittests/tests-gnrc_netif_pktq/tests-gnrc_netif_pktq.h new file mode 100644 index 0000000000..c7443df293 --- /dev/null +++ b/tests/unittests/tests-gnrc_netif_pktq/tests-gnrc_netif_pktq.h @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2019 Freie Universität Berlin + * + * This file is subject to the terms and conditions of the GNU Lesser + * General Public License v2.1. See the file LICENSE in the top level + * directory for more details. + */ + +/** + * @ingroup unittests + * @{ + * + * @file + * @brief unittests for the `gnrc_netif_pktq` module + * + * @author Martine Lenders + */ +#ifndef TESTS_GNRC_NETIF_PKTQ_H +#define TESTS_GNRC_NETIF_PKTQ_H + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @brief The entry point of this test suite. + */ +void tests_pktqueue(void); + +#ifdef __cplusplus +} +#endif + +#endif /* TESTS_GNRC_NETIF_PKTQ_H */ +/** @} */