-
Notifications
You must be signed in to change notification settings - Fork 13
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
dut monitor: Support local port forwarding option #264
Merged
mhiramat
merged 5 commits into
main
from
263-add-new-port-forwarding-mode-to-cro3-dut-shell-or-other-command
Oct 16, 2024
Merged
dut monitor: Support local port forwarding option #264
mhiramat
merged 5 commits into
main
from
263-add-new-port-forwarding-mode-to-cro3-dut-shell-or-other-command
Oct 16, 2024
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
Add the options for local port forwarding for each DUT on `cro3 dut monitor`. This allows user to make DUT working as gateway. e.g. cro3 dut monitor mydut,192.168.111.222:3456 This will map some local port to 192.168.111.222:3456 via mydut.
mhiramat
requested review from
hikalium,
HidenoriKobayashi,
cynthia and
brandonpollack23
as code owners
August 28, 2024 00:45
brandonpollack23
suggested changes
Aug 28, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
initial comments for now
Accept (very lazy)IPv6 address for port fowarding. User can pass the address as; cro3 dut monitor DUT_NAME,IPV4:PORT,[IPV6]:port
Instead of pushing each sub status to the DUT status, just create another status vector string and join it.
There is no reason to pass Vec with Option, and if it is not mutable the slice is enough.
Simply return an errror instead of abort, and parse port_str without temporary variable.
HidenoriKobayashi
approved these changes
Oct 2, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
mhiramat
deleted the
263-add-new-port-forwarding-mode-to-cro3-dut-shell-or-other-command
branch
October 16, 2024 04:02
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.
Add the options for local port forwarding for each DUT on
cro3 dut monitor
. This allows user to make DUT working as gateway. e.g.cro3 dut monitor mydut,192.168.111.222:3456
This will map some local port to 192.168.111.222:3456 via mydut.