connect over stdio #177
connect over stdio #177
Conversation
|
@sakai135 I understood from others on this repo that they lack context why this is proposed. Perhaps you can add this a little on the technical side? This is a possible change to allow an easier integration of the needed parts to make the usermode networkstack work for WSL2. Originally an alternative client for wsl+vm was necessary. The only concerns is that performance might be a problem. WDYT? |
|
What is not clear to me is why this has to go over stdio instead of other channels (some kind of pipe/socket/file descriptor/...). I'm sure there are very good reasons for that, but it would help to state these :) |
|
@cfergeau updated the PR description with the reasons for using stdio |
| @@ -275,7 +283,7 @@ func run(ctx context.Context, g *errgroup.Group, configuration *types.Configurat | |||
| for { | |||
| select { | |||
| case <-time.After(5 * time.Second): | |||
| fmt.Printf("%v sent to the VM, %v received from the VM\n", humanize.Bytes(vn.BytesSent()), humanize.Bytes(vn.BytesReceived())) | |||
| log.Debugf("%v sent to the VM, %v received from the VM\n", humanize.Bytes(vn.BytesSent()), humanize.Bytes(vn.BytesReceived())) | |||
test/wsl.sh
Outdated
| set -x | ||
|
|
||
| ./bin/vm \ | ||
| -url="stdio:/home/ubuntu/gvisor-tap-vsock/bin/gvproxy-windows.exe?listen-stdio=accept&debug=true" \ |
There was a problem hiding this comment.
can this work with $HOME ? There is no ubuntu user on Fedora ;-)
There was a problem hiding this comment.
Used pwd to make it a bit more flexible.
sakai135
requested review from
gbraad
and removed request for
n1hility and
cfergeau
|
LGTM thanks for the PR btw I assume the usage of PID is for local and remote addressing is debugging? |
|
Regarding usage of stdio, another longer term option that could be explored in the future would be a reverse ssh forwarded connection to a unix socket (sort of like what we do for API forwarding but flipped) |
|
@n1hility Yes, the local/remote address is just for debugging. |
pkg/net/stdio/dial.go
Outdated
| "strconv" | ||
| ) | ||
|
|
||
| var execCommand = exec.Command |
There was a problem hiding this comment.
Any reason for this indirection?
Apart from this, looks good to me, could you squash all the commits together, and add the PR description as the commit log?
There was a problem hiding this comment.
@cfergeau Using the variable was to make the tests here work.
https://github.com/sakai135/wsl-vpnkit/blob/v0.3.8/pkg/transport/dial_test.go
This adds stdio as a way to communicate between gvproxy and vm mainly for use with WSL2, although it should work for other cases as well. When network connections between WSL2 and the Windows host are blocked, stdio is the only reliable way to establish a channel between WSL2 and the Windows host. Hyper-V socket for WSL2 is a possibility, but it requires undocumented APIs and admin privileges. Signed-off-by: Keiichi Shimamura <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: cfergeau, sakai135 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
👍 |
|
There's another pending PR #175 |
This adds stdio as a way to communicate between
gvproxyandvmmainly for use with WSL2, although it should work for other cases as well.When network connections between WSL2 and the Windows host are blocked, stdio is the only reliable way to establish a channel between WSL2 and the Windows host. Hyper-V socket for WSL2 is a possibility, but it requires undocumented APIs and admin privileges.
Related to #139. Related: microsoft/WSL#4131