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.

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

Jump to bottom

Perform ACM-triggered cloud connection migration without a disconnect #2863

Open
wants to merge 4 commits into
base: test/v6.3.0
Choose a base branch
from
Open

Perform ACM-triggered cloud connection migration without a disconnect #2863

wants to merge 4 commits into from
+125 −16

Conversation

avtolstoy
Copy link
Member

@avtolstoy avtolstoy commented Feb 3, 2025 •
edited
Loading

TODO

Add stress test

Problem

After performing an initial cloud connection, ACM will re-evaluate this decision (if there are multiple network interfaces available) and might decide to migrate the connection. This is currently performed by disconnect, session move and re-connect.

Solution

This instead can be performed without tearing down the cloud connection and just re-binding the underlying socket + indicating to the comms layer that session move + ping needs to be performed.

This PR also fixes slo/connect_time test with test runner-forced network (--network=X option).

Log

Having connected to the cloud over cellular, migrating to a connection over wifi:

0000016932 [comm.coap] TRACE: ACK 0.00  size=4 token= id=12
0000017690 [comm.coap] TRACE: Sending CoAP message
0000017704 [comm.coap] TRACE: CON POST /E/blah size=20 token= id=13
0000017942 [comm.coap] TRACE: Received CoAP message
0000017956 [comm.coap] TRACE: ACK 0.00  size=4 token= id=13
0000018724 [comm.coap] TRACE: Sending CoAP message
0000018737 [comm.coap] TRACE: CON POST /E/blah size=20 token= id=14
0000018981 [comm.coap] TRACE: Received CoAP message
0000018995 [comm.coap] TRACE: ACK 0.00  size=4 token= id=14
0000019759 [comm.coap] TRACE: Sending CoAP message
0000019773 [comm.coap] TRACE: CON POST /E/blah size=20 token= id=15
0000020030 [comm.coap] TRACE: Received CoAP message
0000020044 [comm.coap] TRACE: ACK 0.00  size=4 token= id=15
0000020794 [comm.coap] TRACE: Sending CoAP message
0000020808 [comm.coap] TRACE: CON POST /E/blah size=20 token= id=16
0000020828 [system.cm] INFO: Ethernet: 0/0 packets (0 tx errors) 0/0 bytes received, avg rtt: 0, mask=0000, score=4294967295
0000020861 [system.cm] INFO: WiFi: 10/10 packets (0 tx errors) 1813/1943 bytes received, avg rtt: 183, mask=03ff, score=183
0000020894 [system.cm] INFO: Cellular: 10/10 packets (0 tx errors) 2529/2659 bytes received, avg rtt: 271, mask=03ff, score=271
0000020929 [system.cm] TRACE: Using best network: WiFi
0000020945 [system.cm] TRACE: Best network interface for cloud connection changed (to WiFi) - move the cloud session
0000020976 [system] INFO: Attempting to move cloud connection to interface 5
0000020998 [system] INFO: Re-bound cloud socket to iface 5 ("wl4")
0000021016 [comm] INFO: Moving connection (session move + ping)
0000021034 [comm.dtls] INFO: session cmd (CLS,DIS,MOV,LOD,SAV): 2
0000021052 [comm] INFO: Forcing a cloud ping
0000021064 [comm.coap] TRACE: Sending CoAP message
0000021079 [comm.coap] TRACE: CON 0.00  size=4 token= id=17
0000021285 [comm.dtls] INFO: session cmd (CLS,DIS,MOV,LOD,SAV): 4
0000021303 [comm.coap] TRACE: Received CoAP message
0000021317 [comm.coap] TRACE: ACK 0.00  size=4 token= id=17
0000021830 [comm.coap] TRACE: Sending CoAP message
0000021843 [comm.coap] TRACE: CON POST /E/blah size=20 token= id=18
0000022042 [comm.coap] TRACE: Received CoAP message
0000022057 [comm.coap] TRACE: ACK 0.00  size=4 token= id=18
0000022864 [comm.coap] TRACE: Sending CoAP message
0000022878 [comm.coap] TRACE: CON POST /E/blah size=20 token= id=19
0000023076 [comm.coap] TRACE: Received CoAP message
0000023091 [comm.coap] TRACE: ACK 0.00  size=4 token= id=19
0000023899 [comm.coap] TRACE: Sending CoAP message
0000023912 [comm.coap] TRACE: CON POST /E/blah size=20 token= id=20
0000024146 [comm.coap] TRACE: Received CoAP message
0000024161 [comm.coap] TRACE: ACK 0.00  size=4 token= id=20

References

Links to the Community, Docs, Other Issues, etc..

Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@avtolstoy avtolstoy added this to the 6.3.0 milestone Feb 3, 2025
@avtolstoy avtolstoy requested a review from sergeuz February 3, 2025 11:28
sergeuz
sergeuz approved these changes Feb 3, 2025
View reviewed changes
communication/src/dtls_protocol.h Outdated
@@ -115,6 +115,13 @@ class DTLSProtocol : public Protocol
}
return r;
}
case ProtocolCommands::MOVE_CONNECTION: {
LOG(INFO, "Moving connection (session move + ping)");
if (!channel.command(Channel::MOVE_SESSION)) {
Copy link
Member

Choose a reason for hiding this comment

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

Any reason not to return the actual error code instead of UNKNOWN if this fails?

Copy link
Member Author

Choose a reason for hiding this comment

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

🤷 fixed. And added one session use.

system/src/system_cloud_connection.cpp
@@ -361,3 +361,22 @@ int system_cloud_get_inet_family_keepalive(int af, unsigned int* value) {

return 0;
}

int system_cloud_move_connection(network_handle_t network) {
if (system_cloud_is_connected(nullptr)) { // XXX: this is not a bool!
Copy link
Member

Choose a reason for hiding this comment

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

😅

@avtolstoy avtolstoy force-pushed the feature/migrate-cloud-connection-without-disconnect branch from 5ddf34c to f872b50 Compare February 3, 2025 12:03
@avtolstoy avtolstoy modified the milestones: 6.3.0, 6.3.1 Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sergeuz sergeuz sergeuz approved these changes

Assignees
No one assigned
Labels
DO NOT MERGE feature
Projects
None yet
Milestone
6.3.1
Development

Successfully merging this pull request may close these issues.
2 participants
@avtolstoy @sergeuz