Skip to content

portfwd: support HostSocket #4008

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
Merged
merged 2 commits into from
Sep 23, 2025
Merged

portfwd: support HostSocket #4008

merged 2 commits into from
Sep 23, 2025

Conversation

@norio-nomura
Copy link
Contributor

@norio-nomura norio-nomura commented Sep 9, 2025 •
edited
Loading

Add missing hostSocket support to GRPC port forwarder.

  • Added support for listening on Unix domain sockets
  • Do not set HostPortRange if HostSocket is used.

edit: dropped closing ClosableListeners on exit

@norio-nomura norio-nomura force-pushed the portfwd-support-host-socket branch 2 times, most recently from 277bbce to 141e02e Compare September 9, 2025 07:49
@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Sep 9, 2025
@norio-nomura norio-nomura force-pushed the portfwd-support-host-socket branch 2 times, most recently from 64a2088 to 991e95f Compare September 12, 2025 01:27
@norio-nomura
Copy link
Contributor Author

norio-nomura commented Sep 12, 2025

Dropped closing ClosableListeners on exit.
I'll open it as a separate PR.

@norio-nomura norio-nomura mentioned this pull request Sep 12, 2025
Merged
@norio-nomura norio-nomura force-pushed the portfwd-support-host-socket branch from 991e95f to 935aa20 Compare September 17, 2025 07:50
AkihiroSuda
AkihiroSuda previously approved these changes Sep 19, 2025
View reviewed changes
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks, can we cover this in test-port-forwarding ?

@norio-nomura norio-nomura force-pushed the portfwd-support-host-socket branch from 935aa20 to e7a9d6b Compare September 21, 2025 01:21
@norio-nomura
Copy link
Contributor Author

norio-nomura commented Sep 21, 2025

added a test to hack/test-port-forwarding.pl

@norio-nomura norio-nomura force-pushed the portfwd-support-host-socket branch 4 times, most recently from bedf7de to 7ef6404 Compare September 22, 2025 01:13
@norio-nomura
Copy link
Contributor Author

norio-nomura commented Sep 22, 2025

I give up on making hostSocket tests pass on Windows, since I don't have any Windows environments for debugging.

@jandubois
Copy link
Member

jandubois commented Sep 22, 2025

Afaik there is no AF_UNIX interop between WSL2 and Windows; it only worked for WSL, but has never been implemented for WSL2.

@norio-nomura norio-nomura force-pushed the portfwd-support-host-socket branch 3 times, most recently from 8c60f68 to 5ec1a53 Compare September 22, 2025 11:28
@norio-nomura norio-nomura force-pushed the portfwd-support-host-socket branch 3 times, most recently from b80fe85 to 37c46c8 Compare September 22, 2025 14:24
@norio-nomura
portfwd: support HostSocket
8108012 
Signed-off-by: Norio Nomura <[email protected]>

portfwd: remove "unixgram" forwarding code because that does not work

Signed-off-by: Norio Nomura <[email protected]>

portfwd: do not use `listenConfig` param on Unix domain sockets

Signed-off-by: Norio Nomura <[email protected]>
@norio-nomura
hack/test-port-forwarding.pl: Add a test using hostSocket
ef88218 
Signed-off-by: Norio Nomura <[email protected]>

hack/test-port-forwarding.pl: use platform-independent path on hostSocket

Signed-off-by: Norio Nomura <[email protected]>

hack/test-port-forwarding.pl: Skip hostSocket test on Windows host

Signed-off-by: Norio Nomura <[email protected]>
@norio-nomura norio-nomura force-pushed the portfwd-support-host-socket branch from 37c46c8 to ef88218 Compare September 22, 2025 14:59
@norio-nomura
Copy link
Contributor Author

norio-nomura commented Sep 22, 2025

I give up on making hostSocket tests pass on Windows, since I don't have any Windows environments for debugging.

I didn’t give up, even though I said I had. But this time, I truly gave up.
Changed the hostSocket test to skip on Windows host.

AkihiroSuda
AkihiroSuda approved these changes Sep 23, 2025
View reviewed changes
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda
Copy link
Member

AkihiroSuda commented Sep 23, 2025

With the introduction of ssh-over-vsock, which port forwarder works best for host sockets?

@AkihiroSuda AkihiroSuda merged commit e827aa4 into lima-vm:master Sep 23, 2025
36 checks passed
@norio-nomura norio-nomura deleted the portfwd-support-host-socket branch September 23, 2025 06:07
@norio-nomura
Copy link
Contributor Author

norio-nomura commented Sep 23, 2025

Thanks! 🙏🏻

With the introduction of ssh-over-vsock, which port forwarder works best for host sockets?

If it's just the transfer speed, the port forwarder by SSH over VSOCK was fast, but the stability is still unknown.

If the cause of the gRPC forwarder's issue is caused by VSOCK, which is the transporting path, SSH over VSOCK may be affected in the same way.

AkihiroSuda
AkihiroSuda reviewed Oct 9, 2025
View reviewed changes
pkg/portfwd/listener_darwin.go
"github.com/sirupsen/logrus"
)

func Listen(ctx context.Context, listenConfig net.ListenConfig, hostAddress string) (net.Listener, error) {
Copy link
Member

@AkihiroSuda AkihiroSuda Oct 9, 2025

Choose a reason for hiding this comment

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

The function seems called only from forwardTCP()?
How does this function work with unix sockets?

Copy link
Contributor Author

@norio-nomura norio-nomura Oct 9, 2025

Choose a reason for hiding this comment

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

GRPC port forwarder does not support guest-side Unix sockets.
forwardTCP()'s TCP indicates "guest side is TCP".

Copy link
Member

@AkihiroSuda AkihiroSuda Oct 9, 2025

Choose a reason for hiding this comment

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

Thanks, got it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@jandubois jandubois Awaiting requested review from jandubois

Assignees

No one assigned

Labels

impact/changelog

Projects

None yet

Milestone

v2.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@norio-nomura @jandubois @AkihiroSuda