Skip to content

portfwd: create separate gRPC streams for each UDP client #3724

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

Merged
merged 1 commit into from
Jul 17, 2025

Conversation

stek29
Copy link
Contributor

@stek29 stek29 commented Jul 14, 2025

The UDP port forwarder previously used a single gRPC stream for all clients, which could cause responses from the guest to be sent to the wrong client on the host.

This occurred because the stream was created before client connections were demultiplexed by gvisor-tap-vsock's UDPProxy.

The root cause is the interaction with gvisor-tap-vsock's UDPProxy, which handles client demultiplexing internally based on the source address of incoming datagrams. It expects its dialer function to return a new net.Conn for each new client it detects.

This commit moves the gRPC stream creation into the UDPProxy dialer function. This ensures a new, dedicated stream is created for each new client, fixing the incorrect response routing.

@stek29
Copy link
Contributor Author

stek29 commented Jul 14, 2025

easiest way to reproduce the issue is to run an actual long-running udp server with port-forwarding -- a DNS server, for example:

  1. start a lima vm with udp port forwarding configured, here's the config I've used https://gist.github.com/stek29/dcafea785fbde81537db84e8c19ed47f
  2. in the lima vm start a dns server, for example:
    docker run --rm -p 5053:53/udp -p 5053:53/tcp coredns/coredns:latest
    
  3. try to query the server from host:
    dig @127.0.0.1 -p 5053 example.com
    dig +tcp @127.0.0.1 -p 5053 example.com
    
  4. notice how for UDP only the first query succeeds, while TCP works fine

The UDP port forwarder previously used a single gRPC stream for all
clients, which could cause responses from the guest to be sent to the
wrong client on the host.

This occurred because the stream was created before client connections
were demultiplexed by `gvisor-tap-vsock`'s `UDPProxy`.

The root cause is the interaction with `gvisor-tap-vsock`'s `UDPProxy`,
which handles client demultiplexing internally based on the source
address of incoming datagrams. It expects its `dialer` function to
return a new `net.Conn` for each new client it detects.

This commit moves the gRPC stream creation into the `UDPProxy` dialer
function. This ensures a new, dedicated stream is created for each new
client, fixing the incorrect response routing.

Signed-off-by: Viktor Oreshkin <imselfish@stek29.rocks>
@stek29
Copy link
Contributor Author

stek29 commented Jul 14, 2025

the cleanup is handled by gvisor-vtap-sock:

https://github.com/containers/gvisor-tap-vsock/blob/23f6f8364426a40294dc97da55865b30d4419819/pkg/services/forwarder/udp_proxy.go#L69-L101

it ends up calling .Close on the GrpcClientRW if there are no packets for UDPConnTrackTimeout = 90 * time.Second, which ends up calling g.stream.CloseSend() on the stream.

however, it is done by utilizing SetReadDeadline, which is just not implemented on GrpcClientRW:
https://github.com/stek29/lima/blob/a04f2447db7127203b64c6f92e608817d6d6f7ef/pkg/portfwd/client.go#L122-L132

so the streams and sockets will stay there forever until the socket on the vm is closed -- then they'll be collected and closed

@stek29
Copy link
Contributor Author

stek29 commented Jul 14, 2025

There are also no tests for UDP forwarding whatsoever, and that feels like it's too big to fix in this PR for me

@AkihiroSuda AkihiroSuda requested a review from balajiv113 July 14, 2025 07:33
@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Jul 14, 2025
@AkihiroSuda AkihiroSuda requested a review from a team July 16, 2025 16:14
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 AkihiroSuda merged commit a55e052 into lima-vm:master Jul 17, 2025
62 of 63 checks passed
This was referenced Jul 17, 2025
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Jul 27, 2025
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` ([#&#8203;3750](lima-vm/lima#3750))
  - portfwd: create separate gRPC streams for each UDP client ([#&#8203;3724](lima-vm/lima#3724), thanks to [@&#8203;stek29](https://github.com/stek29))
  - qemuimgutil: fix "Failed to get "write" lock" error ([#&#8203;3742](lima-vm/lima#3742), thanks to [@&#8203;pavelanni](https://github.com/pavelanni))
  - Embed templates edited from the user interface ([#&#8203;3745](lima-vm/lima#3745), thanks to [@&#8203;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: [@&#8203;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 ([#&#8203;3677](lima-vm/lima#3677))
  - Add `limactl clone OLDINST NEWINST` command. Not to be confused with `limactl copy`, which copies files. ([#&#8203;3673](lima-vm/lima#3673))
  - Add `limactl edit --mount-none` flag ([#&#8203;3647](lima-vm/lima#3647))
- Port forwarding:
  - Improve stability ([#&#8203;3684](lima-vm/lima#3684). [#&#8203;3708](lima-vm/lima#3708), thanks to [@&#8203;balajiv113](https://github.com/balajiv113))
  - De-deprecate `LIMA_SSH_PORT_FORWARDER` that was once deprecated in v1.1 ([#&#8203;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) ([#&#8203;3701](lima-vm/lima#3701))
- Templates:
  - `default`: update from Ubuntu 24.10 to 25.04 ([#&#8203;3643](lima-vm/lima#3643))
  - `fedora`: update from Fedora 41 to 42 ([#&#8203;3643](lima-vm/lima#3643))
  - `almalinux-10`: new template ([#&#8203;3602](lima-vm/lima#3602), thanks to [@&#8203;refi64](https://github.com/refi64))
  - `rocky-10`: new template ([#&#8203;3642](lima-vm/lima#3642))
  - `experimental/debian-testing`: new template ([#&#8203;3645](lima-vm/lima#3645))
  - `experimental/ubuntu-next`: new template ([#&#8203;3645](lima-vm/lima#3645))
- Makefile:
  - Kconfig: deprecate ([#&#8203;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
[@&#8203;AkinoKaede](https://github.com/AkinoKaede) [@&#8203;Horiodino](https://github.com/Horiodino) [@&#8203;afbjorklund](https://github.com/afbjorklund) [@&#8203;alexandear](https://github.com/alexandear) [@&#8203;arixmkii](https://github.com/arixmkii) [@&#8203;balajiv113](https://github.com/balajiv113) [@&#8203;fruzitent](https://github.com/fruzitent) [@&#8203;jandubois](https://github.com/jandubois) [@&#8203;kachick](https://github.com/kachick) [@&#8203;mazzz1y](https://github.com/mazzz1y) [@&#8203;nirs](https://github.com/nirs) [@&#8203;refi64](https://github.com/refi64) [@&#8203;songponssw](https://github.com/songponssw) [@&#8203;thomasjm](https://github.com/thomasjm) [@&#8203;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: [@&#8203;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-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants