Skip to content

forkproxy: proxy device early disconnect fix #15178

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
tomponline merged 3 commits into from
Apr 1, 2025
Merged

forkproxy: proxy device early disconnect fix #15178

tomponline merged 3 commits into from
Apr 1, 2025

Conversation

@kadinsayani
Copy link
Member

@kadinsayani kadinsayani commented Mar 14, 2025 •
edited
Loading

Fixes #15024.

@kadinsayani kadinsayani force-pushed the 15024-proxy-device-early-disconnect-bug-fix branch from 22a33c0 to 2cc87b2 Compare March 14, 2025 21:22
@kadinsayani kadinsayani changed the title LXD: Proxy device fix LXD: Proxy device early disconnect fix Mar 14, 2025
@kadinsayani kadinsayani changed the title LXD: Proxy device early disconnect fix forkproxy: proxy device early disconnect fix Mar 14, 2025
@kadinsayani kadinsayani force-pushed the 15024-proxy-device-early-disconnect-bug-fix branch from 06f7ff7 to 60c2127 Compare March 14, 2025 21:27
@kadinsayani kadinsayani requested a review from nmezhenskyi March 14, 2025 21:33
@kadinsayani kadinsayani force-pushed the 15024-proxy-device-early-disconnect-bug-fix branch from 60c2127 to 9955ecf Compare March 14, 2025 21:46
simondeziel
simondeziel previously approved these changes Mar 15, 2025
View reviewed changes
nmezhenskyi
nmezhenskyi previously approved these changes Mar 17, 2025
View reviewed changes
@tomponline
Copy link
Member

tomponline commented Mar 17, 2025

I'm going to wait until the tests pass before reviewing :)

Status: Started
Warning: Failed to connect to target: dial tcp 127.0.0.1:4321: connect: connection refused
Warning: Failed to prepare new listener instance: dial tcp 127.0.0.1:4321: connect: connection refused
Proxy device did not properly send data from host to container

@kadinsayani kadinsayani marked this pull request as draft March 17, 2025 22:38
@kadinsayani kadinsayani dismissed stale reviews from nmezhenskyi and simondeziel via e4cf20f March 20, 2025 22:58
@kadinsayani kadinsayani force-pushed the 15024-proxy-device-early-disconnect-bug-fix branch 3 times, most recently from 3a8b0bd to 22ff488 Compare March 21, 2025 02:34
@kadinsayani kadinsayani marked this pull request as ready for review March 21, 2025 03:45
@kadinsayani
Copy link
Member Author

kadinsayani commented Mar 21, 2025

I'm going to wait until the tests pass before reviewing :)

Status: Started
Warning: Failed to connect to target: dial tcp 127.0.0.1:4321: connect: connection refused
Warning: Failed to prepare new listener instance: dial tcp 127.0.0.1:4321: connect: connection refused
Proxy device did not properly send data from host to container

Tests passing ✅

@kadinsayani kadinsayani force-pushed the 15024-proxy-device-early-disconnect-bug-fix branch 3 times, most recently from 5f29f74 to 94f322b Compare March 21, 2025 20:43
tomponline
tomponline reviewed Mar 26, 2025
View reviewed changes
@kadinsayani kadinsayani marked this pull request as draft March 26, 2025 14:24
@kadinsayani kadinsayani force-pushed the 15024-proxy-device-early-disconnect-bug-fix branch 3 times, most recently from 3655347 to ca07171 Compare March 27, 2025 15:53
simondeziel
simondeziel reviewed Mar 27, 2025
View reviewed changes
@kadinsayani kadinsayani force-pushed the 15024-proxy-device-early-disconnect-bug-fix branch from ca07171 to 3226489 Compare March 27, 2025 16:44
@kadinsayani kadinsayani marked this pull request as ready for review March 27, 2025 17:03
@kadinsayani kadinsayani force-pushed the 15024-proxy-device-early-disconnect-bug-fix branch 5 times, most recently from 0f5c321 to c7f028e Compare March 31, 2025 17:20
simondeziel
simondeziel previously approved these changes Mar 31, 2025
View reviewed changes
tomponline
tomponline requested changes Apr 1, 2025
View reviewed changes
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Looks like some changes needed to the for loop.

@kadinsayani kadinsayani force-pushed the 15024-proxy-device-early-disconnect-bug-fix branch 2 times, most recently from 2350a2a to 0bc211b Compare April 1, 2025 17:18
@kadinsayani
lxd/main_forkproxy: Improve variable naming consistency in `genericRe…
a0522eb 
…lay` function

`errSnd` -> `errSend`
`errRcv` -> `errRecv`

This aligns better with `chSend` and `chRecv`.

Signed-off-by: Kadin Sayani <[email protected]>
@kadinsayani
lxd/main_forkproxy: Rework genericRelay function to support half-cl…
26b9264 
…osed TCP connections

Fixes canonical#15024.

This commit reworks the `genericRelay` function to support half-closed
TCP connections. `genericRelay` waits for both sending and receiving to
complete when copying data via proxy. When one side finishes, TCP
connections are closed on the write side, allowing data to still be
received.

Signed-off-by: Kadin Sayani <[email protected]>
@kadinsayani
tests: Add container proxy device test for half-closed TCP connections
7d74a1b 
Signed-off-by: Kadin Sayani <[email protected]>
@kadinsayani kadinsayani force-pushed the 15024-proxy-device-early-disconnect-bug-fix branch from 0bc211b to 7d74a1b Compare April 1, 2025 17:20
@kadinsayani
Copy link
Member Author

kadinsayani commented Apr 1, 2025

Looks like some changes needed to the for loop.

Thanks for your review. I've addressed all comments.

simondeziel
simondeziel approved these changes Apr 1, 2025
View reviewed changes
Copy link
Member

@simondeziel simondeziel left a comment

Choose a reason for hiding this comment

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

Thanks

tomponline
tomponline approved these changes Apr 1, 2025
View reviewed changes
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Lgtm thanks!

@tomponline tomponline merged commit a08a4d2 into canonical:main Apr 1, 2025
29 checks passed
@kadinsayani kadinsayani deleted the 15024-proxy-device-early-disconnect-bug-fix branch April 6, 2025 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tomponline tomponline tomponline approved these changes

@simondeziel simondeziel simondeziel approved these changes

@nmezhenskyi nmezhenskyi nmezhenskyi left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Proxy device early disconnect

4 participants

@kadinsayani @tomponline @simondeziel @nmezhenskyi