Skip to content
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.

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

ACM MVP Behavior changes #2738

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

ACM MVP Behavior changes #2738

wants to merge 6 commits into from

Conversation

scott-brust
Copy link
Member

@scott-brust scott-brust commented Feb 14, 2024

Problem

Changes in ACM behavior as discussed internally, and as in this epic

Solution

-Remove ability to bind cloud connection to specific networks interfaces
-Disconnect cloud connection if the network interface reports link down in order to "fail over" faster
-Implement Hard preference for cloud connection.

  • If a connection is set to be preferred, and the cloud is connected, and that network is available: disconnect the cloud and reconnect with the preferred interface immediately
  • If a preferred connection becomes ready, and the cloud is connected on a different connection: disconnect the cloud and reconnect with the preferred interface immediately

Steps to Test

Run connection-manager app

Example App

Run connection-manager app

make PLATFORM=msom APPDIR=../user/tests/app/connection-manager all program-dfu


if (spark_cloud_flag_connected() && disconnectCloud) {
auto systemOptions = options.toSystemOptions();
spark_cloud_disconnect(&systemOptions, nullptr);
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we defer the actual disabling of the SPARK_CLOUD_AUTO_CONNECT flag to the system task queue? I'm worried we would get into a situation where the cloud reconnect options would be consumed/overridden before cloud_disconnect() runs and re-sets the cloud auto connect flag...

// If cloud is already connected, and a preferred network is set, and it is up, move cloud connection to it immediately
if (spark_cloud_flag_connected() && network_ready(spark::Network.from(preferredNetwork_), 0, nullptr)) {
auto options = CloudDisconnectOptions().graceful(true).reconnect(true).toSystemOptions();
spark_cloud_disconnect(&options, nullptr);
Copy link
Member Author

Choose a reason for hiding this comment

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

Same concern here about executing this on the system task as from network manager context

@scott-brust scott-brust marked this pull request as ready for review February 15, 2024 23:11
@Kategrode
Copy link

[sc-125332]

@@ -609,11 +616,25 @@ void NetworkManager::handleIfLink(if_t iface, const struct if_event* ev) {
} else if (state_ == State::IP_CONFIGURED || state_ == State::IFACE_LINK_UP) {
refreshIpState();
}

// If the preferred network becomes available, close the cloud connection to force migration to this network
if (ConnectionManager::instance()->getPreferredNetwork() == netIfIndex) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This works, but technically is not the correct spot to check if a network is ready (ie IP configured).
This state is just link up. The implicit assumption here is that the interface will be assigned an IP by the time we try to reopen the cloud socket, which usually is true.

I think to be strictly correct, the transition from link up -> ip configured is detected in refreshIpState(), so perhaps we should check for connection migration there?

@scott-brust
Copy link
Member Author

[sc-125333]

@scott-brust scott-brust added this to the 5.8.2 milestone May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants