-
Notifications
You must be signed in to change notification settings - Fork 694
Fix tunneling of connection when keepalive is set #3684
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
Conversation
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.
Pull Request Overview
Fix tunneling of connection when keepalive is set by replacing the existing bicopy-based data forwarding with a TCP proxy that preserves keepalive semantics.
- Swap out
bicopy.Bicopy
fortcpproxy.DialProxy
to handle TCP connections - Update imports from
lima/pkg/bicopy
tocontainers/gvisor-tap-vsock/pkg/tcpproxy
Comments suppressed due to low confidence (1)
pkg/portfwd/client.go:36
- This new proxy path for TCP connections lacks dedicated tests; consider adding unit or integration tests to verify tunneling behavior when keepalive is enabled.
proxy := tcpproxy.DialProxy{DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
9b11171
to
7b94043
Compare
Signed-off-by: Balaji Vijayakumar <[email protected]>
@AkihiroSuda & @jandubois This works for me. with given reproduction steps |
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.
Thanks, let's merge this, but we should eventually run a test for this on CI
Can you explain what is the issue and how it is solved in tcpproxy? What is missing in bicopy to implement TCP half-close? Looking at bicopy and tcpproxy, both start goroutines running io.Copy(dot, src), and both call src.CloseRead() and dst.CloseWrite() the goroutine exits. Finally both call src.Close() and dst.Close() at the end. The only thing missing in biCopy is setting keepalive on both ends, but since we don't set KeepAlivePeriod the keepalive is not set on the net.Con. Do we have a way to write a test reproducing this issue? |
I tried to write a test but couldn't but the steps mentioned in the issue is perfectly reproducible |
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [lima-vm/lima](https://github.com/lima-vm/lima) | minor | `v1.1.1` -> `v1.2.1` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>lima-vm/lima (lima-vm/lima)</summary> ### [`v1.2.1`](https://github.com/lima-vm/lima/releases/tag/v1.2.1) [Compare Source](lima-vm/lima@v1.2.0...v1.2.1) #### Changes - Cherry-picks from `master` to `release/1.2` ([#​3750](lima-vm/lima#3750)) - portfwd: create separate gRPC streams for each UDP client ([#​3724](lima-vm/lima#3724), thanks to [@​stek29](https://github.com/stek29)) - qemuimgutil: fix "Failed to get "write" lock" error ([#​3742](lima-vm/lima#3742), thanks to [@​pavelanni](https://github.com/pavelanni)) - Embed templates edited from the user interface ([#​3745](lima-vm/lima#3745), thanks to [@​afbjorklund](https://github.com/afbjorklund)) #### Usage ```console $ limactl create $ limactl start ... INFO[0029] READY. Run `lima` to open the shell. $ lima uname Linux ``` *** The binaries were built automatically on GitHub Actions. The build log is available for 90 days: https://github.com/lima-vm/lima/actions/runs/16405168554 The sha256sum of the SHA256SUMS file itself is `fda636e062d8f75a689071b308a94dc902111620f7122746fc0d172959d7d26a` . *** Release manager: [@​AkihiroSuda](https://github.com/AkihiroSuda) ### [`v1.2.0`](https://github.com/lima-vm/lima/releases/tag/v1.2.0) [Compare Source](lima-vm/lima@v1.1.1...v1.2.0) #### Changes - `limactl` CLI: - Add `limactl network (list|create|delete)` commands ([#​3677](lima-vm/lima#3677)) - Add `limactl clone OLDINST NEWINST` command. Not to be confused with `limactl copy`, which copies files. ([#​3673](lima-vm/lima#3673)) - Add `limactl edit --mount-none` flag ([#​3647](lima-vm/lima#3647)) - Port forwarding: - Improve stability ([#​3684](lima-vm/lima#3684). [#​3708](lima-vm/lima#3708), thanks to [@​balajiv113](https://github.com/balajiv113)) - De-deprecate `LIMA_SSH_PORT_FORWARDER` that was once deprecated in v1.1 ([#​3709](lima-vm/lima#3709)) - nerdctl: - Update from v2.1.2 to [v2.1.3](https://github.com/containerd/nerdctl/releases/tag/v2.1.3) ([#​3701](lima-vm/lima#3701)) - Templates: - `default`: update from Ubuntu 24.10 to 25.04 ([#​3643](lima-vm/lima#3643)) - `fedora`: update from Fedora 41 to 42 ([#​3643](lima-vm/lima#3643)) - `almalinux-10`: new template ([#​3602](lima-vm/lima#3602), thanks to [@​refi64](https://github.com/refi64)) - `rocky-10`: new template ([#​3642](lima-vm/lima#3642)) - `experimental/debian-testing`: new template ([#​3645](lima-vm/lima#3645)) - `experimental/ubuntu-next`: new template ([#​3645](lima-vm/lima#3645)) - Makefile: - Kconfig: deprecate ([#​3706](lima-vm/lima#3706)) > \[!NOTE] > On Intel Mac, macOS 15.5 or later is needed to boot the default Ubuntu 25.04 template. > > Workarounds for macOS <= 15.4 : > > - Option 1: Use QEMU > > ``` > limactl create --vm-type=qemu > ``` > > - Option2: Use Ubuntu 24.04 template > > ``` > limactl create --name=default template://ubuntu-24.04 > ``` Full changes: https://github.com/lima-vm/lima/milestone/61?closed=1 Thanks to [@​AkinoKaede](https://github.com/AkinoKaede) [@​Horiodino](https://github.com/Horiodino) [@​afbjorklund](https://github.com/afbjorklund) [@​alexandear](https://github.com/alexandear) [@​arixmkii](https://github.com/arixmkii) [@​balajiv113](https://github.com/balajiv113) [@​fruzitent](https://github.com/fruzitent) [@​jandubois](https://github.com/jandubois) [@​kachick](https://github.com/kachick) [@​mazzz1y](https://github.com/mazzz1y) [@​nirs](https://github.com/nirs) [@​refi64](https://github.com/refi64) [@​songponssw](https://github.com/songponssw) [@​thomasjm](https://github.com/thomasjm) [@​unsuman](https://github.com/unsuman) #### Usage ```console $ limactl create $ limactl start ... INFO[0029] READY. Run `lima` to open the shell. $ lima uname Linux ``` *** The binaries were built automatically on GitHub Actions. The build log is available for 90 days: https://github.com/lima-vm/lima/actions/runs/16187325385 The sha256sum of the SHA256SUMS file itself is `f4d57cdd637022ead291af680c0c88a8d07fd35fd6a779b7c6348e2c9a2ec2cc` . *** Release manager: [@​AkihiroSuda](https://github.com/AkihiroSuda) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MC42Mi4xIiwidXBkYXRlZEluVmVyIjoiNDAuNjIuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
Fixes #3601