Skip to content

Commit

Permalink
net: conn_mgr: Support Auto-Down
Browse files Browse the repository at this point in the history 
To reduce the amount of boiler-plate needed in applications, this commit
grants conn_mgr the ability to automatically take ifaces that have given
up on connecting into the admin-down state.

Tests adjusted as appropriate.

This behavior can be disabled globally by disabling
NET_CONNECTION_MANAGER_AUTO_IF_DOWN, or disabled per-iface using the
CONN_MGR_IF_NO_AUTO_DOWN flag.

Signed-off-by: Georges Oates_Larsen <georges.larsen@nordicsemi.no>
  • Loading branch information
@glarsennordic @carlescufi
glarsennordic authored and carlescufi committed Jun 30, 2023
1 parent f3d75c4 commit b65613e
Show file tree
Hide file tree
Showing 8 changed files with 367 additions and 47 deletions.
6 changes: 6 additions & 0 deletions include/zephyr/net/conn_mgr_connectivity.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,12 @@ enum conn_mgr_if_flag {
*/
CONN_MGR_IF_NO_AUTO_CONNECT,

/* No auto-down
* When set, conn_mgr will not automatically take the iface admin-down when it stops
* trying to connect, even if NET_CONNECTION_MANAGER_AUTO_IF_DOWN is enabled.
*/
CONN_MGR_IF_NO_AUTO_DOWN,

/** @cond INTERNAL_HIDDEN */
/* Total number of flags - must be at the end of the enum */
CONN_MGR_NUM_IF_FLAGS,
Expand Down
4 changes: 4 additions & 0 deletions subsys/net/conn_mgr/Kconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,8 @@ config NET_CONNECTION_MANAGER_PRIORITY
help
This sets the starting priority of the connection manager thread.

config NET_CONNECTION_MANAGER_AUTO_IF_DOWN
bool "Automatically call net_if_down on ifaces that have given up on connecting"
default y

endif # NET_CONNECTION_MANAGER
106 changes: 105 additions & 1 deletion subsys/net/conn_mgr/conn_mgr_connectivity.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ int conn_mgr_if_connect(struct net_if *iface)
return status;
}

static void conn_mgr_conn_if_auto_admin_down(struct net_if *iface);

int conn_mgr_if_disconnect(struct net_if *iface)
{
struct conn_mgr_conn_binding *binding;
Expand Down Expand Up @@ -77,6 +79,16 @@ int conn_mgr_if_disconnect(struct net_if *iface)
out:
k_mutex_unlock(binding->mutex);

/* Since the connectivity implementation will not automatically attempt to reconnect after
* a call to conn_mgr_if_disconnect, conn_mgr_conn_if_auto_admin_down should be called.
*
* conn_mgr_conn_handle_iface_down will only call conn_mgr_conn_if_auto_admin_down if
* persistence is disabled. To ensure conn_mgr_conn_if_auto_admin_down is called in all
* cases, we must call it directly from here. If persistence is disabled, this will result
* in conn_mgr_conn_if_auto_admin_down being called twice, but that is not an issue.
*/
conn_mgr_conn_if_auto_admin_down(iface);

return status;
}

Expand Down Expand Up @@ -263,6 +275,61 @@ static void conn_mgr_conn_handle_iface_admin_up(struct net_if *iface)
}
}

/**
* @brief Take the provided iface admin-down.
*
* Called automatically by conn_mgr when an iface has lost connection and will not attempt to
* regain it.
*
* @param iface - The iface to take admin-down
*/
static void conn_mgr_conn_if_auto_admin_down(struct net_if *iface)
{
/* NOTE: This will be double-fired for ifaces that are both non-persistent
* and are being directly requested to disconnect, since both of these conditions
* separately trigger conn_mgr_conn_if_auto_admin_down.
*
* This is fine, because net_if_down is idempotent, but if you are adding other
* behaviors to this function, bear it in mind.
*/

/* Ignore ifaces that don't have connectivity implementations */
if (!conn_mgr_if_is_bound(iface)) {
return;
}

/* Take the iface admin-down if AUTO_DOWN is enabled */
if (IS_ENABLED(CONFIG_NET_CONNECTION_MANAGER_AUTO_IF_DOWN) &&
!conn_mgr_if_get_flag(iface, CONN_MGR_IF_NO_AUTO_DOWN)) {
net_if_down(iface);
}
}

/**
* @brief Perform automated behaviors in response to any iface that loses oper-up state.
*
* This is how conn_mgr_conn automatically takes such ifaces admin-down if they are not persistent.
*
* @param iface - The iface which lost oper-up state.
*/
static void conn_mgr_conn_handle_iface_down(struct net_if *iface)
{
/* Ignore ifaces that don't have connectivity implementations */
if (!conn_mgr_if_is_bound(iface)) {
return;
}

/* If the iface is persistent, we expect it to try to reconnect, so nothing else to do */
if (conn_mgr_if_get_flag(iface, CONN_MGR_IF_PERSISTENT)) {
return;
}

/* Otherwise, we do not expect the iface to reconnect, and we should call
* conn_mgr_conn_if_auto_admin_down
*/
conn_mgr_conn_if_auto_admin_down(iface);
}

static struct net_mgmt_event_callback conn_mgr_conn_iface_cb;
static void conn_mgr_conn_iface_handler(struct net_mgmt_event_callback *cb, uint32_t mgmt_event,
struct net_if *iface)
Expand All @@ -272,12 +339,45 @@ static void conn_mgr_conn_iface_handler(struct net_mgmt_event_callback *cb, uint
}

switch (mgmt_event) {
case NET_EVENT_IF_UP:
case NET_EVENT_IF_DOWN:
conn_mgr_conn_handle_iface_down(iface);
break;
case NET_EVENT_IF_ADMIN_UP:
conn_mgr_conn_handle_iface_admin_up(iface);
break;
}
}

static struct net_mgmt_event_callback conn_mgr_conn_self_cb;
static void conn_mgr_conn_self_handler(struct net_mgmt_event_callback *cb, uint32_t mgmt_event,
struct net_if *iface)
{
if ((mgmt_event & CONN_MGR_CONN_SELF_EVENTS_MASK) != mgmt_event) {
return;
}

switch (NET_MGMT_GET_COMMAND(mgmt_event)) {
case NET_EVENT_CONN_CMD_IF_FATAL_ERROR:
if (cb->info) {
NET_ERR("Fatal connectivity error on iface %d (%p). Reason: %d.",
net_if_get_by_iface(iface), iface, *((int *)cb->info)
);
} else {
NET_ERR("Unknown fatal connectivity error on iface %d (%p).",
net_if_get_by_iface(iface), iface
);
}
__fallthrough;
case NET_EVENT_CONN_CMD_IF_TIMEOUT:
/* If a timeout or fatal error occurs, we do not expect the iface to try to
* reconnect, so call conn_mgr_conn_if_auto_admin_down.
*/
conn_mgr_conn_if_auto_admin_down(iface);
break;
}

}

void conn_mgr_conn_init(void)
{
/* Initialize connectivity bindings. */
Expand Down Expand Up @@ -305,6 +405,10 @@ void conn_mgr_conn_init(void)
CONN_MGR_CONN_IFACE_EVENTS_MASK);
net_mgmt_add_event_callback(&conn_mgr_conn_iface_cb);

net_mgmt_init_event_callback(&conn_mgr_conn_self_cb, conn_mgr_conn_self_handler,
CONN_MGR_CONN_SELF_EVENTS_MASK);
net_mgmt_add_event_callback(&conn_mgr_conn_self_cb);

/* Trigger any initial automated behaviors for ifaces */
STRUCT_SECTION_FOREACH(conn_mgr_conn_binding, binding) {
if (binding->impl->api) {
Expand Down
6 changes: 5 additions & 1 deletion subsys/net/conn_mgr/conn_mgr_private.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@
#define CONN_MGR_IFACE_EVENTS_MASK (NET_EVENT_IF_DOWN | \
NET_EVENT_IF_UP)

#define CONN_MGR_CONN_IFACE_EVENTS_MASK (NET_EVENT_IF_ADMIN_UP)
#define CONN_MGR_CONN_IFACE_EVENTS_MASK (NET_EVENT_IF_ADMIN_UP |\
NET_EVENT_IF_DOWN)

#define CONN_MGR_CONN_SELF_EVENTS_MASK (NET_EVENT_CONN_IF_TIMEOUT | \
NET_EVENT_CONN_IF_FATAL_ERROR)

#define CONN_MGR_IPV6_EVENTS_MASK (NET_EVENT_IPV6_ADDR_ADD | \
NET_EVENT_IPV6_ADDR_DEL | \
Expand Down
43 changes: 0 additions & 43 deletions tests/net/conn_mgr/src/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ static void reset_test_iface(struct net_if *iface)
struct in6_addr *ll_ipv6;

if (net_if_is_admin_up(iface)) {
if (conn_mgr_if_is_bound(iface)) {
(void)conn_mgr_if_disconnect(iface);
}
(void)net_if_down(iface);
}

Expand Down Expand Up @@ -206,10 +203,6 @@ static void cycle_ready_ifaces(struct net_if *ifa, struct net_if *ifb)
"No events should be fired if connectivity availability did not change.");

/* Take A down */
if (conn_mgr_if_is_bound(ifa)) {
zassert_equal(conn_mgr_if_disconnect(ifa), 0,
"conn_mgr_if_disconnect should succeed for ifa.");
}
zassert_equal(net_if_down(ifa), 0, "net_if_down should succeed for ifa.");

/* Expect no events */
Expand All @@ -219,10 +212,6 @@ static void cycle_ready_ifaces(struct net_if *ifa, struct net_if *ifb)
"No events should be fired if connectivity availability did not change.");

/* Take B down */
if (conn_mgr_if_is_bound(ifb)) {
zassert_equal(conn_mgr_if_disconnect(ifb), 0,
"conn_mgr_if_disconnect should succeed for ifb.");
}
zassert_equal(net_if_down(ifb), 0, "net_if_down should succeed for ifb.");

/* Expect connectivity loss */
Expand Down Expand Up @@ -266,10 +255,6 @@ static void cycle_ignored_iface(struct net_if *ifa, struct net_if *ifb)
"No events should be fired if connectivity availability did not change.");

/* Take B down */
if (conn_mgr_if_is_bound(ifb)) {
zassert_equal(conn_mgr_if_disconnect(ifb), 0,
"conn_mgr_if_disconnect should succeed for ifb.");
}
zassert_equal(net_if_down(ifb), 0, "net_if_down should succeed for ifb.");

/* Expect no events */
Expand Down Expand Up @@ -300,10 +285,6 @@ static void cycle_ignored_iface(struct net_if *ifa, struct net_if *ifb)
"No events should be fired if connectivity availability did not change.");

/* Take B down */
if (conn_mgr_if_is_bound(ifb)) {
zassert_equal(conn_mgr_if_disconnect(ifb), 0,
"conn_mgr_if_disconnect should succeed for ifba.");
}
zassert_equal(net_if_down(ifb), 0, "net_if_down should succeed for ifb.");

/* Expect no events */
Expand All @@ -313,10 +294,6 @@ static void cycle_ignored_iface(struct net_if *ifa, struct net_if *ifb)
"No events should be fired if connectivity availability did not change.");

/* Take A down */
if (conn_mgr_if_is_bound(ifa)) {
zassert_equal(conn_mgr_if_disconnect(ifa), 0,
"conn_mgr_if_disconnect should succeed for ifa.");
}
zassert_equal(net_if_down(ifa), 0, "net_if_down should succeed for ifa.");

/* Expect connectivity lost */
Expand Down Expand Up @@ -353,10 +330,6 @@ static void cycle_ignored_iface(struct net_if *ifa, struct net_if *ifb)
zassert_equal(stats.conn_iface, ifa, "ifa should be blamed.");

/* Take B down */
if (conn_mgr_if_is_bound(ifb)) {
zassert_equal(conn_mgr_if_disconnect(ifb), 0,
"conn_mgr_if_disconnect should succeed for ifb.");
}
zassert_equal(net_if_down(ifb), 0, "net_if_down should succeed for ifb.");

/* Expect no events */
Expand All @@ -368,10 +341,6 @@ static void cycle_ignored_iface(struct net_if *ifa, struct net_if *ifb)

/* Take B up */
zassert_equal(net_if_up(ifb), 0, "net_if_up should succeed for ifb.");
if (conn_mgr_if_is_bound(ifb)) {
zassert_equal(conn_mgr_if_connect(ifb), 0,
"conn_mgr_if_connect should succeed for ifb.");
}

/* Expect no events */
k_sleep(EVENT_WAIT_TIME);
Expand All @@ -392,10 +361,6 @@ static void cycle_ignored_iface(struct net_if *ifa, struct net_if *ifb)
zassert_equal(stats.dconn_iface, ifa, "ifa should be blamed.");

/* Take B down */
if (conn_mgr_if_is_bound(ifb)) {
zassert_equal(conn_mgr_if_disconnect(ifb), 0,
"conn_mgr_if_disconnect should succeed for ifb.");
}
zassert_equal(net_if_down(ifb), 0, "net_if_down should succeed for ifb.");

/* Expect no events */
Expand Down Expand Up @@ -566,10 +531,6 @@ static void cycle_iface_states(struct net_if *iface, enum ip_order ifa_ipm)
/* (10 -> 00): Lose oper-up from semi-ready state */

/* Take iface down */
if (conn_mgr_if_is_bound(iface)) {
zassert_equal(conn_mgr_if_disconnect(iface), 0,
"conn_mgr_if_disconnect should succeed.");
}
zassert_equal(net_if_down(iface), 0, "net_if_down should succeed.");

/* Verify there are no events fired */
Expand Down Expand Up @@ -619,10 +580,6 @@ static void cycle_iface_states(struct net_if *iface, enum ip_order ifa_ipm)
/* (11 -> 01): Lose oper-up from ready state */

/* Take iface down */
if (conn_mgr_if_is_bound(iface)) {
zassert_equal(conn_mgr_if_disconnect(iface), 0,
"conn_mgr_if_disconnect should succeed.");
}
zassert_equal(net_if_down(iface), 0, "net_if_down should succeed.");

/* Verify events are fired */
Expand Down
Loading

0 comments on commit b65613e

Please sign in to comment.