Skip to content

Import gvisor-tap-vsock/pkg/tcpproxy into pkg/tcpproxy #3686

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

Closed
wants to merge 4 commits into from
+489 −89
Closed

Import gvisor-tap-vsock/pkg/tcpproxy into pkg/tcpproxy #3686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5ccf6c8
Import gvisor-tap-vsock/pkg/tcpproxy
AkihiroSuda Jul 4, 2025
ade5f8a
pkg/tcpproxy: typo
AkihiroSuda Jul 8, 2025
736e7fb
pkg/tcpproxy: set nolint
AkihiroSuda Jul 8, 2025
5fe7f8f
Replace `pkg/bicopy` and `g-t-v/pkg/tcpproxy` with `pkg/tcpproxy`
AkihiroSuda Jul 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 0 additions & 84 deletions pkg/bicopy/bicopy.go
View file
Open in desktop

This file was deleted.

7 changes: 5 additions & 2 deletions pkg/hostagent/port_darwin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import (
"github.com/lima-vm/sshocker/pkg/ssh"
"github.com/sirupsen/logrus"

"github.com/lima-vm/lima/pkg/bicopy"
"github.com/lima-vm/lima/pkg/portfwd"
"github.com/lima-vm/lima/pkg/tcpproxy"
)

// forwardTCP is not thread-safe.
Expand Down Expand Up @@ -153,7 +153,10 @@ func (plf *pseudoLoopbackForwarder) forward(ac *net.TCPConn) error {
return err
}
defer unixConn.Close()
bicopy.Bicopy(ac, unixConn, nil)
proxy := tcpproxy.DialProxy{DialContext: func(_ context.Context, _, _ string) (net.Conn, error) {
return unixConn, nil
}}
proxy.HandleConn(ac)
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/portfwd/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ import (
"time"

"github.com/containers/gvisor-tap-vsock/pkg/services/forwarder"
"github.com/containers/gvisor-tap-vsock/pkg/tcpproxy"
"github.com/sirupsen/logrus"

"github.com/lima-vm/lima/pkg/guestagent/api"
guestagentclient "github.com/lima-vm/lima/pkg/guestagent/api/client"
"github.com/lima-vm/lima/pkg/tcpproxy"
)

func HandleTCPConnection(ctx context.Context, client *guestagentclient.GuestAgentClient, conn net.Conn, guestAddr string) {
Expand Down
8 changes: 6 additions & 2 deletions pkg/portfwdserver/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
package portfwdserver

import (
"context"
"errors"
"io"
"net"
"time"

"github.com/lima-vm/lima/pkg/bicopy"
"github.com/lima-vm/lima/pkg/guestagent/api"
"github.com/lima-vm/lima/pkg/tcpproxy"
)

type TunnelServer struct{}
Expand All @@ -35,7 +36,10 @@ func (s *TunnelServer) Start(stream api.GuestService_TunnelServer) error {
return err
}
rw := &GRPCServerRW{stream: stream, id: in.Id}
bicopy.Bicopy(rw, conn, nil)
proxy := tcpproxy.DialProxy{DialContext: func(_ context.Context, _, _ string) (net.Conn, error) {
return conn, nil
}}
proxy.HandleConn(rw)
Copy link
Member

Choose a reason for hiding this comment

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

Unlike bicopy.Bicopy(), this ignores errors silently. I don't think this library should be used for anything in the current state.

Copy link
Member Author

Choose a reason for hiding this comment

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

What should we do then ?

Copy link
Member

Choose a reason for hiding this comment

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

Port forwarding seems to be lima core functionality, and using a library makes it hard to get good logging, so It think the best way is to copy the missing bits from tcpproxy into our own implementation.

This may be related:
inetaf/tcpproxy#46

tcpproxy seems to be about

Package tcpproxy lets users build TCP proxies, optionally making routing decisions based on HTTP/1 Host headers and the SNI hostname in TLS connections.

Our usage seems to be hacky way to reuse part of the library.

It also says:

This package makes no API stability promises. If you depend on it, vendor it.

Other options:

  • Log errors in the wrappers we pass to tcpproxy
  • Fix tcpproxy to log errors properly

If nobody can own this code in lima we can use tcpproxy as a temporary quick fix.

Copy link
Member

Choose a reason for hiding this comment

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

Port forwarding seems to be lima core functionality, and using a library makes it hard to get good logging, so It think the best way is to copy the missing bits from tcpproxy into our own implementation.

I have no idea how big tcpproxy is, but we have copied single file implementations from other projects before:

❯ ag adapted
pkg/hostagent/dns/dns.go
4:// This file has been adapted from https://github.com/norouter/norouter/blob/v0.6.4/pkg/agent/dns/dns.go

pkg/reflectutil/reflectutil.go
4:// This file has been adapted from https://github.com/containerd/nerdctl/blob/v1.0.0/pkg/reflectutil/reflectutil.go

So assuming the implementation isn't too big, I think importing it gives us the maximum flexibility going forward.

Copy link
Member

Choose a reason for hiding this comment

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

Port forwarding seems to be lima core functionality, and using a library makes it hard to get good logging, so It think the best way is to copy the missing bits from tcpproxy into our own implementation.

I have no idea how big tcpproxy is

tcpproxy is pretty small, and we basically use one function:
https://github.com/inetaf/tcpproxy/blob/c4b9df066048ad2ab5c32235362fa94444a24ebe/tcpproxy.go#L376

func (dp *DialProxy) HandleConn(src net.Conn) {
	ctx := context.Background()
	var cancel context.CancelFunc
	if dp.DialTimeout >= 0 {
		ctx, cancel = context.WithTimeout(ctx, dp.dialTimeout())
	}
	dst, err := dp.dialContext()(ctx, "tcp", dp.Addr)
	if cancel != nil {
		cancel()
	}
	if err != nil {
		dp.onDialError()(src, err)
		return
	}
	defer dst.Close()

	if err = dp.sendProxyHeader(dst, src); err != nil {
		dp.onDialError()(src, err)
		return
	}
	defer src.Close()

	if ka := dp.keepAlivePeriod(); ka > 0 {
		for _, c := range []net.Conn{src, dst} {
			if c, ok := tcpConn(c); ok {
				c.SetKeepAlive(true)
				c.SetKeepAlivePeriod(ka)
			}
		}
	}

	errc := make(chan error, 2)
	go proxyCopy(errc, src, dst)
	go proxyCopy(errc, dst, src)
	<-errc
	<-errc
}

In this file we use it to proxy packets between GRPC endpoint and net.Con - both are connected, so we hack the proxy DialContext function to not dial anything.

We don't need the sendProxyHeader() thing since we don't to the optional http proxy feature.

I'm not sure if the keepalive is needed for our use case, but GRPC is not a net.TCPCon so it does nothing for it.

So we are left with:

	defer dst.Close()
	defer src.Close()

	if ka := dp.keepAlivePeriod(); ka > 0 {
		for _, c := range []net.Conn{src, dst} {
			if c, ok := tcpConn(c); ok {
				c.SetKeepAlive(true)
				c.SetKeepAlivePeriod(ka)
			}
		}
	}

	errc := make(chan error, 2)
	go proxyCopy(errc, src, dst)
	go proxyCopy(errc, dst, src)
	<-errc
	<-errc

Note how the errors are dropped silently to make the user life more interesting.

proxyCopy is:

func proxyCopy(errc chan<- error, dst, src net.Conn) {
	defer closeRead(src)
	defer closeWrite(dst)

	// Before we unwrap src and/or dst, copy any buffered data.
	if wc, ok := src.(*Conn); ok && len(wc.Peeked) > 0 {
		if _, err := dst.Write(wc.Peeked); err != nil {
			errc <- err
			return
		}
		wc.Peeked = nil
	}

	// Unwrap the src and dst from *Conn to *net.TCPConn so Go
	// 1.11's splice optimization kicks in.
	src = UnderlyingConn(src)
	dst = UnderlyingConn(dst)

	_, err := io.Copy(dst, src)
	errc <- err
}

Since we don't use tcpproxy.Con, and we don't do route matching we don't need to copy buffered data.

Since GRPC endpoint is not a net.TCPConn unwrapping the underlying connection will fail, and the splice optimization will not kick in.

So we are left with:

func proxyCopy(errc chan<- error, dst, src net.Conn) {
	defer closeRead(src)
	defer closeWrite(dst)
	_, err := io.Copy(dst, src)
	errc <- err
}

I'm not sure how this code supports TCP half-close and bicopy does not. They seem to do the same thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

The issue might not be relevant to tcp half-close , as this does not seem to implement half-close methods

lima/pkg/portfwdserver/server.go

Line 42 in 3bc63b6

type GRPCServerRW struct {

Not sure why tcpproxy fixes the issue, except on WSL2 🤔

return nil
}

Expand Down
Loading
Loading