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

update ndt5 runner #81

Merged
merged 1 commit into from
Oct 28, 2020
Merged

update ndt5 runner #81

merged 1 commit into from
Oct 28, 2020

Conversation

critzo
Copy link
Contributor

@critzo critzo commented Oct 28, 2020

Updates the ndt5 test runner to:

  • Always use -protocol=ndt5 so all tests are in plaintext, or insecure websockets. This will eventually be replaced to support either protocol using an env var, but since most Murakami beacons we're testing on are either armv7 or arm64, removing the overhead of TLS is needed to support line rate measurement.
  • Test runner supports using MURAKAMI_TESTS_NDT5_ENABLED=1 and MURAKAMI_TESTS_NDT5_HOST=<IP or FQDN> environment variables to define a specific, self-provisioned ndt-server to test to. Noting that this is an interim solution.

This update has been tested and confirmed working using a Balena managed Raspberry Pi 4.

Copy link
Collaborator

@robertodauria robertodauria left a comment

Choose a reason for hiding this comment

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

A question: why not leaving the option to enable secure connections is the user wants to?

@critzo
Copy link
Contributor Author

critzo commented Oct 28, 2020

The issue is that Murakami is meant to run unattended, and the secure option is not suitable on the current recommended and tested hardware. I would like eventually to allow all of the binary's options to be configurable using the config file/env var methods we're using now, which I think should happen in the next re-write. So I'm definitely in favor of supporting it, but the current runner needs to only use non-TLS. In my tests, the measurements are at line rate for non-TLS, and perhaps at least 50% less when using TLS or wss.

@robertodauria robertodauria self-requested a review October 28, 2020 15:47
Copy link
Collaborator

@robertodauria robertodauria left a comment

Choose a reason for hiding this comment

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

LGTM if it's an interim solution, having the option to enable secure connections in future sounds nice but not needed and likely a bad idea for the current use cases.

@critzo critzo merged commit 3b708f2 into master Oct 28, 2020
@critzo critzo deleted the sandbox-correct-ndt5-runner branch October 28, 2020 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants