Add cli command visor ping and test #1405
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The latency in networkStats is stored as uint32 miliseconds but while converting it to time.Duration, it requires input of nanoseconds. Before we were passing in uint32 miliseconds instead of nanoseconds which caused the time.duration to be as ns and not ms. To fix this we multiply the latencyMs with a miliseconds with a nanosecond integer count to convert our latencyMs from milisecond to nanosecond.
This commit adds the subcommand ping to the command visor. It takes one argument that is a remote PK. This is the first version of the ping where we are pinging the skychat app on the skynet and retrieving the latency of the connection.
The json of summary is expected to have the latency in ms but we were sending it in ns after the recent change. So we convert it back to ms while creating the summary.
This commit fixes the issue where the rule used to be the same for fwd and rev when both source and destination pk is the same. This happens becase the pk is used as a key in the RulesMap and in the above case the rule is overwritten instead of creating a new one. We fix this by changing the key to be a string of 'pk:port' instead of just the PK which makes the key unique in all cases.
We fix this error by fixgin the noise config in PingRoute by setting the RemotePK the same as LocalPK.
|
get panic when trying to ping moses visor |
mrpalide
changed the title
This commit fixes the issue with the limited data size of read of app conn by first sending the total size of the PingMsg in the new struct PingSizeMsg. Then in the ping module we read the size of the incoming PingMsg and read on the conn until we have received all the ping data packets then then unmarshall it. This way the ping still remains backwards compatable without needing to change the size of noise and break backwards compatability.
mrpalide
approved these changes
Dec 27, 2022
|
We also need to update skywire-services to use this PR after the merge, because of the changes made to the setup-node. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Did you run
make format && make check?Fixes #1377
Changes:
pingcommand as visor subcommand inskywire-cli visor ping <pk>testcommand as visor subcommand inskywire-cli visor testHow to test this PR:
PING
skywire-cli visor ping <visor-a-pk> -t <tries> -s <size-of-packet>TEST
skywire-cli visor test -t <tries> -s <size-of-packet>Result should be like this, but number of test should be same as number of public visors on service discovery
