From e7fac9f661042e8ba19fd98a592f3c9b526b29d2 Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Wed, 29 Sep 2021 16:52:58 +0200 Subject: [PATCH 1/8] net/gnrc_netif: add return values to init --- pkg/nimble/netif/nimble_netif.c | 4 ++- sys/include/net/gnrc/netif.h | 25 +++++++++++-------- sys/net/gnrc/link_layer/gomach/gomach.c | 6 +++-- sys/net/gnrc/link_layer/lwmac/lwmac.c | 6 +++-- sys/net/gnrc/netif/gnrc_netif.c | 3 ++- .../gnrc/netif/lorawan/gnrc_netif_lorawan.c | 6 +++-- tests/gnrc_netif/main.c | 6 +++-- 7 files changed, 36 insertions(+), 20 deletions(-) diff --git a/pkg/nimble/netif/nimble_netif.c b/pkg/nimble/netif/nimble_netif.c index 0a6d196814c6..38a1eb33d016 100644 --- a/pkg/nimble/netif/nimble_netif.c +++ b/pkg/nimble/netif/nimble_netif.c @@ -79,7 +79,7 @@ static void _notify(int handle, nimble_netif_event_t event, uint8_t *addr) } } -static void _netif_init(gnrc_netif_t *netif) +static int _netif_init(gnrc_netif_t *netif) { (void)netif; @@ -92,6 +92,8 @@ static void _netif_init(gnrc_netif_t *netif) * of this */ _netif.sixlo.max_frag_size = 0; #endif /* IS_USED(MODULE_GNRC_NETIF_6LO) */ + + return 0; } static int _send_pkt(nimble_netif_conn_t *conn, gnrc_pktsnip_t *pkt) diff --git a/sys/include/net/gnrc/netif.h b/sys/include/net/gnrc/netif.h index 6bfc214fddf9..2a3b515a48a4 100644 --- a/sys/include/net/gnrc/netif.h +++ b/sys/include/net/gnrc/netif.h @@ -194,21 +194,26 @@ typedef struct { */ struct gnrc_netif_ops { /** - * @brief Initializes network interface beyond the default settings + * @brief Initializes network interface. * * @pre `netif != NULL` * * @param[in] netif The network interface. * - * This is called after the network device's initial configuration, right - * before the interface's thread starts receiving messages. It is not - * necessary to lock the interface's mutex gnrc_netif_t::mutex, since it is - * already locked. Set to @ref gnrc_netif_default_init() if you do not need - * any special initialization. If you do need special initialization, it is - * recommended to call @ref gnrc_netif_default_init() at the start of the - * custom initialization function set here. + * This function should init the device driver or MAC underlying MAC layer. + * This is called right before the interface's thread starts receiving + * messages. It is not necessary to lock the interface's mutex + * gnrc_netif_t::mutex, since it is already locked. Set to @ref + * gnrc_netif_default_init() if you do not need any special initialization. + * If you do need special initialization, it is recommended to call @ref + * gnrc_netif_default_init() at the start of the custom initialization + * function set here. + * + * @return 0 if the initialization of the device or MAC layer was + * successful + * @return negative errno on error. */ - void (*init)(gnrc_netif_t *netif); + int (*init)(gnrc_netif_t *netif); /** * @brief Send a @ref net_gnrc_pkt "packet" over the network interface @@ -537,7 +542,7 @@ static inline int gnrc_netif_ipv6_group_leave(const gnrc_netif_t *netif, * * @param[in] netif The network interface. */ -void gnrc_netif_default_init(gnrc_netif_t *netif); +int gnrc_netif_default_init(gnrc_netif_t *netif); /** * @brief Default operation for gnrc_netif_ops_t::get() diff --git a/sys/net/gnrc/link_layer/gomach/gomach.c b/sys/net/gnrc/link_layer/gomach/gomach.c index e16ec867f6f3..1c119e8e3017 100644 --- a/sys/net/gnrc/link_layer/gomach/gomach.c +++ b/sys/net/gnrc/link_layer/gomach/gomach.c @@ -55,7 +55,7 @@ */ static kernel_pid_t gomach_pid; -static void _gomach_init(gnrc_netif_t *netif); +static int _gomach_init(gnrc_netif_t *netif); static int _send(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt); static gnrc_pktsnip_t *_recv(gnrc_netif_t *netif); static void _gomach_msg_handler(gnrc_netif_t *netif, msg_t *msg); @@ -2115,7 +2115,7 @@ static void _gomach_event_cb(netdev_t *dev, netdev_event_t event) } } -static void _gomach_init(gnrc_netif_t *netif) +static int _gomach_init(gnrc_netif_t *netif) { netdev_t *dev; @@ -2216,4 +2216,6 @@ static void _gomach_init(gnrc_netif_t *netif) gnrc_gomach_set_update(netif, false); gomach_update(netif); } + + return 0; } diff --git a/sys/net/gnrc/link_layer/lwmac/lwmac.c b/sys/net/gnrc/link_layer/lwmac/lwmac.c index 1f6db582121d..e4de75b6ef19 100644 --- a/sys/net/gnrc/link_layer/lwmac/lwmac.c +++ b/sys/net/gnrc/link_layer/lwmac/lwmac.c @@ -63,7 +63,7 @@ static void rtt_cb(void *arg); static void lwmac_set_state(gnrc_netif_t *netif, gnrc_lwmac_state_t newstate); static void lwmac_schedule_update(gnrc_netif_t *netif); static void rtt_handler(uint32_t event, gnrc_netif_t *netif); -static void _lwmac_init(gnrc_netif_t *netif); +static int _lwmac_init(gnrc_netif_t *netif); static int _send(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt); static gnrc_pktsnip_t *_recv(gnrc_netif_t *netif); static void _lwmac_msg_handler(gnrc_netif_t *netif, msg_t *msg); @@ -939,7 +939,7 @@ static void _lwmac_msg_handler(gnrc_netif_t *netif, msg_t *msg) } } -static void _lwmac_init(gnrc_netif_t *netif) +static int _lwmac_init(gnrc_netif_t *netif) { netdev_t *dev; @@ -984,4 +984,6 @@ static void _lwmac_init(gnrc_netif_t *netif) netif->mac.prot.lwmac.awake_duration_sum_ticks = 0; netif->mac.prot.lwmac.lwmac_info |= GNRC_LWMAC_RADIO_IS_ON; #endif + + return 0; } diff --git a/sys/net/gnrc/netif/gnrc_netif.c b/sys/net/gnrc/netif/gnrc_netif.c index c0f1dd8bc88b..f39752fc3ba7 100644 --- a/sys/net/gnrc/netif/gnrc_netif.c +++ b/sys/net/gnrc/netif/gnrc_netif.c @@ -1577,7 +1577,7 @@ static void _test_options(gnrc_netif_t *netif) } #endif /* DEVELHELP */ -void gnrc_netif_default_init(gnrc_netif_t *netif) +int gnrc_netif_default_init(gnrc_netif_t *netif) { _init_from_device(netif); #ifdef DEVELHELP @@ -1590,6 +1590,7 @@ void gnrc_netif_default_init(gnrc_netif_t *netif) #ifdef MODULE_GNRC_IPV6_NIB gnrc_ipv6_nib_init_iface(netif); #endif + return 0; } static void _send(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt, bool push_back); diff --git a/sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c b/sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c index 17cca566a7a9..33314fe8374a 100644 --- a/sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c +++ b/sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c @@ -48,7 +48,7 @@ static gnrc_pktsnip_t *_recv(gnrc_netif_t *netif); static void _msg_handler(gnrc_netif_t *netif, msg_t *msg); static int _get(gnrc_netif_t *netif, gnrc_netapi_opt_t *opt); static int _set(gnrc_netif_t *netif, const gnrc_netapi_opt_t *opt); -static void _init(gnrc_netif_t *netif); +static int _init(gnrc_netif_t *netif); static const gnrc_netif_ops_t lorawan_ops = { .init = _init, @@ -252,7 +252,7 @@ netdev_t *gnrc_lorawan_get_netdev(gnrc_lorawan_t *mac) return netif->dev; } -static void _init(gnrc_netif_t *netif) +static int _init(gnrc_netif_t *netif) { gnrc_netif_default_init(netif); netif->dev->event_callback = _driver_cb; @@ -279,6 +279,8 @@ static void _init(gnrc_netif_t *netif) ztimer_set_msg(ZTIMER_MSEC, &netif->lorawan.backoff_timer, GNRC_LORAWAN_BACKOFF_WINDOW_TICK / 1000, &backoff_msg, thread_getpid()); + + return 0; } int gnrc_netif_lorawan_create(gnrc_netif_t *netif, char *stack, int stacksize, diff --git a/tests/gnrc_netif/main.c b/tests/gnrc_netif/main.c index 9a72758e31f1..120fb8919324 100644 --- a/tests/gnrc_netif/main.c +++ b/tests/gnrc_netif/main.c @@ -55,7 +55,7 @@ static bool init_called = false; static uint8_t ethernet_groups[ETHERNET_GROUPS_MAX][ETHERNET_ADDR_LEN]; static BITFIELD(ethernet_groups_set, ETHERNET_GROUPS_MAX); -static inline void _test_init(gnrc_netif_t *netif); +static inline int _test_init(gnrc_netif_t *netif); static inline int _mock_netif_send(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt); static inline gnrc_pktsnip_t *_mock_netif_recv(gnrc_netif_t * netif); static int _get_netdev_address(netdev_t *dev, void *value, size_t max_len); @@ -113,11 +113,13 @@ static void _set_up(void) while (msg_try_receive(&msg) > 0) {} } -static inline void _test_init(gnrc_netif_t *netif) +static inline int _test_init(gnrc_netif_t *netif) { (void)netif; gnrc_netif_default_init(netif); init_called = true; + + return 0; } static void test_creation(void) From 415704fa76971a51960eb29d9f7e52c5bdacd78c Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Thu, 30 Sep 2021 10:44:57 +0200 Subject: [PATCH 2/8] net/gnrc_netif: move netdev init code to ops->init --- sys/net/gnrc/netif/gnrc_netif.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/sys/net/gnrc/netif/gnrc_netif.c b/sys/net/gnrc/netif/gnrc_netif.c index f39752fc3ba7..ff420bdad25e 100644 --- a/sys/net/gnrc/netif/gnrc_netif.c +++ b/sys/net/gnrc/netif/gnrc_netif.c @@ -1579,6 +1579,16 @@ static void _test_options(gnrc_netif_t *netif) int gnrc_netif_default_init(gnrc_netif_t *netif) { + netdev_t *dev = netif->dev; + /* register the event callback with the device driver */ + dev->event_callback = _event_cb; + dev->context = netif; + int res = dev->driver->init(dev); + if (res < 0) { + return res; + } + netif_register(&netif->netif); + _check_netdev_capabilities(dev); _init_from_device(netif); #ifdef DEVELHELP _test_options(netif); @@ -1829,7 +1839,6 @@ static void *_gnrc_netif_thread(void *args) DEBUG("gnrc_netif: starting thread %i\n", thread_getpid()); netif = ctx->netif; gnrc_netif_acquire(netif); - dev = netif->dev; netif->pid = thread_getpid(); #if IS_USED(MODULE_GNRC_NETIF_EVENTS) @@ -1840,20 +1849,15 @@ static void *_gnrc_netif_thread(void *args) /* setup the link-layer's message queue */ msg_init_queue(msg_queue, GNRC_NETIF_MSG_QUEUE_SIZE); - /* register the event callback with the device driver */ - dev->event_callback = _event_cb; - dev->context = netif; /* initialize low-level driver */ - ctx->result = dev->driver->init(dev); + ctx->result = netif->ops->init(netif); /* signal that driver init is done */ mutex_unlock(&ctx->init_done); if (ctx->result < 0) { - LOG_ERROR("gnrc_netif: netdev init failed: %d\n", ctx->result); + LOG_ERROR("gnrc_netif: init failed: %d\n", ctx->result); return NULL; } - netif_register(&netif->netif); - _check_netdev_capabilities(dev); - netif->ops->init(netif); + dev = netif->dev; #if DEVELHELP assert(options_tested); #endif From 49527fc9943821f0370dfbdfdf58fc6fd7f3988e Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Thu, 30 Sep 2021 10:46:58 +0200 Subject: [PATCH 3/8] pkg/nimble/netif: adapt ops->init to handle driver initialization --- pkg/nimble/netif/nimble_netif.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/nimble/netif/nimble_netif.c b/pkg/nimble/netif/nimble_netif.c index 38a1eb33d016..ab307bccb1d8 100644 --- a/pkg/nimble/netif/nimble_netif.c +++ b/pkg/nimble/netif/nimble_netif.c @@ -83,7 +83,11 @@ static int _netif_init(gnrc_netif_t *netif) { (void)netif; - gnrc_netif_default_init(netif); + int res = gnrc_netif_default_init(netif); + if (res < 0) { + return res; + } + /* save the threads context pointer, so we can set its flags */ _netif_thread = thread_get_active(); @@ -92,8 +96,9 @@ static int _netif_init(gnrc_netif_t *netif) * of this */ _netif.sixlo.max_frag_size = 0; #endif /* IS_USED(MODULE_GNRC_NETIF_6LO) */ + res = 0; - return 0; + return res; } static int _send_pkt(nimble_netif_conn_t *conn, gnrc_pktsnip_t *pkt) From 298f16fee786f798bf77de7f3f25092b3b784d6f Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Thu, 30 Sep 2021 10:47:26 +0200 Subject: [PATCH 4/8] net/gomach: adapt ops->init to handle driver initialization --- sys/net/gnrc/link_layer/gomach/gomach.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sys/net/gnrc/link_layer/gomach/gomach.c b/sys/net/gnrc/link_layer/gomach/gomach.c index 1c119e8e3017..cf2364a2c297 100644 --- a/sys/net/gnrc/link_layer/gomach/gomach.c +++ b/sys/net/gnrc/link_layer/gomach/gomach.c @@ -2119,7 +2119,11 @@ static int _gomach_init(gnrc_netif_t *netif) { netdev_t *dev; - gnrc_netif_default_init(netif); + int res = gnrc_netif_default_init(netif); + if (res < 0) { + return res; + } + dev = netif->dev; dev->event_callback = _gomach_event_cb; From 08ec9693cd02879cffcd0dec273e03a03de7b725 Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Thu, 30 Sep 2021 10:47:42 +0200 Subject: [PATCH 5/8] net/lwmac: adapt ops->init to handle driver initialization --- sys/net/gnrc/link_layer/lwmac/lwmac.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/sys/net/gnrc/link_layer/lwmac/lwmac.c b/sys/net/gnrc/link_layer/lwmac/lwmac.c index e4de75b6ef19..cf5876a392ab 100644 --- a/sys/net/gnrc/link_layer/lwmac/lwmac.c +++ b/sys/net/gnrc/link_layer/lwmac/lwmac.c @@ -943,7 +943,12 @@ static int _lwmac_init(gnrc_netif_t *netif) { netdev_t *dev; - gnrc_netif_default_init(netif); + int res = gnrc_netif_default_init(netif); + + if (res < 0) { + return res; + } + dev = netif->dev; dev->event_callback = _lwmac_event_cb; @@ -985,5 +990,5 @@ static int _lwmac_init(gnrc_netif_t *netif) netif->mac.prot.lwmac.lwmac_info |= GNRC_LWMAC_RADIO_IS_ON; #endif - return 0; + return res; } From 5f46ab72e07257ea2136141b0aa024b32403bab3 Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Thu, 30 Sep 2021 10:47:56 +0200 Subject: [PATCH 6/8] net/gnrc/lorawan: adapt ops->init to handle driver initialization --- sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c b/sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c index 33314fe8374a..0bc1d1d9f9d0 100644 --- a/sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c +++ b/sys/net/gnrc/netif/lorawan/gnrc_netif_lorawan.c @@ -254,7 +254,12 @@ netdev_t *gnrc_lorawan_get_netdev(gnrc_lorawan_t *mac) static int _init(gnrc_netif_t *netif) { - gnrc_netif_default_init(netif); + int res = gnrc_netif_default_init(netif); + + if (res < 0) { + return res; + } + netif->dev->event_callback = _driver_cb; _reset(netif); @@ -280,7 +285,7 @@ static int _init(gnrc_netif_t *netif) GNRC_LORAWAN_BACKOFF_WINDOW_TICK / 1000, &backoff_msg, thread_getpid()); - return 0; + return res; } int gnrc_netif_lorawan_create(gnrc_netif_t *netif, char *stack, int stacksize, From eb23813467561e1936f3abfcd7e6e929c6116166 Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Thu, 30 Sep 2021 10:48:52 +0200 Subject: [PATCH 7/8] tests/gnrc_netif: adapt ops->init to handle driver initialization --- tests/gnrc_netif/main.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/gnrc_netif/main.c b/tests/gnrc_netif/main.c index 120fb8919324..96f47b7f159b 100644 --- a/tests/gnrc_netif/main.c +++ b/tests/gnrc_netif/main.c @@ -116,7 +116,12 @@ static void _set_up(void) static inline int _test_init(gnrc_netif_t *netif) { (void)netif; - gnrc_netif_default_init(netif); + int res = gnrc_netif_default_init(netif); + + if (res < 0) { + return res; + } + init_called = true; return 0; From e40f97c4daf81e713ec2ca5f5cdd177f84224cd1 Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Mon, 22 Nov 2021 11:03:40 +0100 Subject: [PATCH 8/8] net/gnrc/netif: move interface registration to init function --- sys/include/net/gnrc/netif.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sys/include/net/gnrc/netif.h b/sys/include/net/gnrc/netif.h index 2a3b515a48a4..7b5ccbb2127d 100644 --- a/sys/include/net/gnrc/netif.h +++ b/sys/include/net/gnrc/netif.h @@ -194,7 +194,7 @@ typedef struct { */ struct gnrc_netif_ops { /** - * @brief Initializes network interface. + * @brief Initializes and registers network interface. * * @pre `netif != NULL` * @@ -207,7 +207,8 @@ struct gnrc_netif_ops { * gnrc_netif_default_init() if you do not need any special initialization. * If you do need special initialization, it is recommended to call @ref * gnrc_netif_default_init() at the start of the custom initialization - * function set here. + * function set here. This function MUST call @ref netif_register if the + * initialization is successful. * * @return 0 if the initialization of the device or MAC layer was * successful @@ -538,7 +539,8 @@ static inline int gnrc_netif_ipv6_group_leave(const gnrc_netif_t *netif, /** * @brief Default operation for gnrc_netif_ops_t::init() * - * @note Can also be used to be called *before* a custom operation. + * @note Can also be used to be called *before* a custom operation. This + * function calls @ref netif_register internally. * * @param[in] netif The network interface. */