Skip to content

net/gnrc_netif: add bus for interface events, gnrc_netif_up()/gnrc_netif_down() functions #17902

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
+169 −23
Open

net/gnrc_netif: add bus for interface events, gnrc_netif_up()/gnrc_netif_down() functions #17902

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4c012cb
net/gnrc_netif: add bus for interface events
benpicco Apr 4, 2022
ab47098
net/gnrc_netif: add default value for NETOPT_LINK
benpicco Sep 18, 2022
bb2697b
net/gnrc_netif: add gnrc_netif_up()/gnrc_netif_down()
benpicco Apr 4, 2022
8de8d05
gnrc_ipv6_nib: make use of gnrc_netif_bus for interface events
benpicco Oct 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions sys/include/net/gnrc/ipv6/nib.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,16 +233,6 @@ extern "C" {
* @note Only handled with @ref CONFIG_GNRC_IPV6_NIB_DNS != 0
*/
#define GNRC_IPV6_NIB_RDNSS_TIMEOUT (0x4fd3U)

/**
* @brief Interface up event
*/
#define GNRC_IPV6_NIB_IFACE_UP (0x4fd4U)

/**
* @brief Interface down event
*/
#define GNRC_IPV6_NIB_IFACE_DOWN (0x4fd5U)
/** @} */

/**
Expand Down
43 changes: 43 additions & 0 deletions sys/include/net/gnrc/netif.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,30 @@ typedef enum {
GNRC_NETIF_BUS_IPV6, /**< provides @ref gnrc_ipv6_event_t
messages to subscribers */
#endif
GNRC_NETIF_BUS_IFACE, /**< provides @ref gnrc_netif_event_t
messages to subscribers */
GNRC_NETIF_BUS_NUMOF
} gnrc_netif_bus_t;

/**
* @brief Event types for GNRC_NETIF_BUS_IFACE per-interface message bus
*/
typedef enum {
/**
* @brief Link state changed to UP
*
* The event is generated when the link state on the interface became online.
*/
GNRC_NETIF_EVENT_LINK_STATE_CHANGED_DOWN,

/**
* @brief Link state changed to DOWN
*
* The event is generated when the link state on the interface become offline.
*/
GNRC_NETIF_EVENT_LINK_STATE_CHANGED_UP,
} gnrc_netif_event_t;
Comment on lines +114 to +128
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation seems to be swapped


/**
* @brief Event types for GNRC_NETIF_BUS_IPV6 per-interface message bus
*/
Expand Down Expand Up @@ -731,6 +752,28 @@ static inline int gnrc_netif_send(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt)
return gnrc_netapi_send(netif->pid, pkt);
}

/**
* @brief Bring up an interface that has previously been disabled
*
* @param netif pointer to the interface
* @param timeout_ms timeout for confirmation from the interface
*
* @return 0 on success
* negative error
*/
int gnrc_netif_up(gnrc_netif_t *netif, unsigned timeout_ms);

/**
* @brief Disable a network interface
*
* @param netif pointer to the interface
* @param timeout_ms timeout for confirmation from the interface
*
* @return 0 on success
* negative error
*/
int gnrc_netif_down(gnrc_netif_t *netif, unsigned timeout_ms);

#if defined(MODULE_GNRC_NETIF_BUS) || DOXYGEN
/**
* @brief Get a message bus of a given @ref gnrc_netif_t interface.
Expand Down
19 changes: 19 additions & 0 deletions sys/include/net/gnrc/netif/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,25 @@ static inline void gnrc_netif_ipv6_bus_post(gnrc_netif_t *netif, int type,
}
#endif /* IS_USED(MODULE_GNRC_NETIF_IPV6) || defined(DOXYGEN) */

/**
* @brief Posts a message to the event bus of the interface
*
* @param[in] netif Pointer to the interface
* @param[in] type [Type of the event](@ref gnrc_netif_event_t)
* @param[in] ctx The context of the event
*/
static inline void gnrc_netif_event_bus_post(gnrc_netif_t *netif, int type,
const void *ctx)
{
#ifdef MODULE_GNRC_NETIF_BUS
msg_bus_post(&netif->bus[GNRC_NETIF_BUS_IFACE], type, ctx);
#else
(void) netif;
(void) type;
(void) ctx;
#endif
}

/**
* @brief Checks if the interface represents a router according to RFC 4861
*
Expand Down
4 changes: 4 additions & 0 deletions sys/include/net/gnrc/netif/ipv6.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ typedef struct {
netstats_t stats;
#endif
#if defined(MODULE_GNRC_IPV6_NIB) || DOXYGEN
/**
* @brief IPv6 interface bus event subscription
*/
msg_bus_entry_t netif_sub;
#if IS_ACTIVE(CONFIG_GNRC_IPV6_NIB_ROUTER) || DOXYGEN
/**
* @brief Route info callback
Expand Down
1 change: 1 addition & 0 deletions sys/net/gnrc/Makefile.dep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ ifneq (,$(filter gnrc_ipv6_nib,$(USEMODULE)))
USEMODULE += gnrc_ndp
USEMODULE += gnrc_netif
USEMODULE += gnrc_netif_ipv6
USEMODULE += gnrc_netif_bus
USEMODULE += ipv6_addr
USEMODULE += random
endif
Expand Down
93 changes: 83 additions & 10 deletions sys/net/gnrc/netif/gnrc_netif.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,28 @@ int gnrc_netif_get_from_netdev(gnrc_netif_t *netif, gnrc_netapi_opt_t *opt)
res = netif->dev->driver->get(netif->dev, opt->opt, opt->data,
opt->data_len);
}
if (res == -ENOTSUP) {
/* handle default values */
switch (opt->opt) {
case NETOPT_LINK: {
netopt_state_t state;
res = netif->dev->driver->get(netif->dev, NETOPT_STATE, &state, sizeof(state));
if (res < 0) {
break;
}
res = sizeof(netopt_enable_t);
if ((state == NETOPT_STATE_SLEEP) || (state == NETOPT_STATE_OFF) ||
(state == NETOPT_STATE_STANDBY)) {
*((netopt_enable_t *)opt->data) = NETOPT_DISABLE;
} else {
*((netopt_enable_t *)opt->data) = NETOPT_ENABLE;
}
break;
}
default:
break;
}
}
gnrc_netif_release(netif);
return res;
}
Expand Down Expand Up @@ -1437,6 +1459,61 @@ bool gnrc_netif_ipv6_wait_for_global_address(gnrc_netif_t *netif,
#endif /* IS_USED(MODULE_GNRC_NETIF_BUS) */
#endif /* IS_USED(MODULE_GNRC_NETIF_IPV6) */

static int _change_state_and_wait(gnrc_netif_t *netif, netopt_state_t state, unsigned timeout_ms)
{
int res = 0;

netopt_state_t state_prev;
while (gnrc_netapi_get(netif->pid, NETOPT_STATE, 0, &state_prev, sizeof(state_prev)) == -EBUSY) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line and the one below are a bit too long (warning of > 100 characters)


if (state == state_prev) {
return 0;
}

#if IS_USED(MODULE_GNRC_NETIF_BUS)
gnrc_netif_event_t event;

if ((state == NETOPT_STATE_SLEEP) || (state == NETOPT_STATE_OFF) ||
(state == NETOPT_STATE_STANDBY)) {
event = GNRC_NETIF_EVENT_LINK_STATE_CHANGED_DOWN;
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else {
}
else {

event = GNRC_NETIF_EVENT_LINK_STATE_CHANGED_UP;
}

msg_bus_entry_t sub;
msg_bus_t *bus = gnrc_netif_get_bus(netif, GNRC_NETIF_BUS_IFACE);
msg_bus_attach(bus, &sub);
msg_bus_subscribe(&sub, event);
#endif

while (gnrc_netapi_set(netif->pid, NETOPT_STATE, 0, &state, sizeof(state)) == -EBUSY) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must it be NETOPT_STATE or can it be NETOPT_LINK? Because our ifconfig <i> {up | down} command generates NETOPT_LINK.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh interesting.
My use case was setting STATE_SLEEP which should also generate a link down event…

NETOPT_LINK is read-only on all 3 drivers that implement it.


if (timeout_ms == 0) {
return 0;
}

#if IS_USED(MODULE_GNRC_NETIF_BUS)
msg_t m;
if (ztimer_msg_receive_timeout(ZTIMER_MSEC, &m, timeout_ms) < 0) {
DEBUG_PUTS("gnrc_netif: timeout waiting for state change");
res = -ETIMEDOUT;
}
msg_bus_detach(bus, &sub);
#endif

return res;
}

int gnrc_netif_up(gnrc_netif_t *netif, unsigned timeout_ms)
{
return _change_state_and_wait(netif, NETOPT_STATE_IDLE, timeout_ms);
}

int gnrc_netif_down(gnrc_netif_t *netif, unsigned timeout_ms)
{
return _change_state_and_wait(netif, NETOPT_STATE_SLEEP, timeout_ms);
}

static void _update_l2addr_from_dev(gnrc_netif_t *netif)
{
netdev_t *dev = netif->dev;
Expand Down Expand Up @@ -2051,18 +2128,14 @@ static void _event_cb(netdev_t *dev, netdev_event_t event)
gnrc_pktsnip_t *pkt = NULL;
switch (event) {
case NETDEV_EVENT_LINK_UP:
if (IS_USED(MODULE_GNRC_IPV6)) {
msg_t msg = { .type = GNRC_IPV6_NIB_IFACE_UP, .content = { .ptr = netif } };

msg_send(&msg, gnrc_ipv6_pid);
}
gnrc_netif_event_bus_post(netif,
GNRC_NETIF_EVENT_LINK_STATE_CHANGED_UP,
netif);
break;
case NETDEV_EVENT_LINK_DOWN:
if (IS_USED(MODULE_GNRC_IPV6)) {
msg_t msg = { .type = GNRC_IPV6_NIB_IFACE_DOWN, .content = { .ptr = netif } };

msg_send(&msg, gnrc_ipv6_pid);
}
gnrc_netif_event_bus_post(netif,
GNRC_NETIF_EVENT_LINK_STATE_CHANGED_DOWN,
netif);
break;
case NETDEV_EVENT_RX_COMPLETE:
pkt = netif->ops->recv(netif);
Expand Down
22 changes: 19 additions & 3 deletions sys/net/gnrc/network_layer/ipv6/gnrc_ipv6.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,22 @@ static void *_event_loop(void *args)
/* register interest in all IPv6 packets */
gnrc_netreg_register(GNRC_NETTYPE_IPV6, &me_reg);

DEBUG_PUTS("ipv6: wait for interfaces to be ready");
while (gnrc_netif_iter(NULL) == NULL) {
ztimer_sleep(ZTIMER_MSEC, 250);
}
Comment on lines +189 to +192
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fragile. But I also do not know any way to reliably wait for interface registration to complete off the top of my head or even if such feature exists :/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interfaces could send a message when they are initialized, then we can handle the registration here.


/* subscribe to interface events */
gnrc_netif_t *netif = NULL;
while ((netif = gnrc_netif_iter(netif))) {
DEBUG("ipv6: subscribe to events on interface %u\n", netif->pid);

msg_bus_t *bus = gnrc_netif_get_bus(netif, GNRC_NETIF_BUS_IFACE);
msg_bus_attach(bus, &netif->ipv6.netif_sub);
msg_bus_subscribe(&netif->ipv6.netif_sub, GNRC_NETIF_EVENT_LINK_STATE_CHANGED_UP);
msg_bus_subscribe(&netif->ipv6.netif_sub, GNRC_NETIF_EVENT_LINK_STATE_CHANGED_DOWN);
}

/* preinitialize ACK */
reply.type = GNRC_NETAPI_MSG_TYPE_ACK;

Expand All @@ -194,7 +210,7 @@ static void *_event_loop(void *args)
DEBUG("ipv6: waiting for incoming message.\n");
msg_receive(&msg);

switch (msg.type) {
switch (msg_bus_get_type(&msg)) {
case GNRC_NETAPI_MSG_TYPE_RCV:
DEBUG("ipv6: GNRC_NETAPI_MSG_TYPE_RCV received\n");
_receive(msg.content.ptr);
Expand Down Expand Up @@ -244,10 +260,10 @@ static void *_event_loop(void *args)
DEBUG("ipv6: NIB timer event received\n");
gnrc_ipv6_nib_handle_timer_event(msg.content.ptr, msg.type);
break;
case GNRC_IPV6_NIB_IFACE_UP:
case GNRC_NETIF_EVENT_LINK_STATE_CHANGED_UP:
gnrc_ipv6_nib_iface_up(msg.content.ptr);
break;
case GNRC_IPV6_NIB_IFACE_DOWN:
case GNRC_NETIF_EVENT_LINK_STATE_CHANGED_DOWN:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Care to fix the indent here, since you touch the code anyway?

gnrc_ipv6_nib_iface_down(msg.content.ptr, false);
break;
default:
Expand Down