Skip to content

Revert "vsock: Fix TCP connection bug" and implement a different approach #295

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 3 commits into from
Mar 31, 2025

Conversation

slp
Copy link
Collaborator

@slp slp commented Mar 31, 2025

This fixes #294 while implementing another approach for fixing the problem addressed by 3bd3b98

…y state"

This reverts commit 3bd3b98.

Signed-off-by: Sergio Lopez <[email protected]>
@slp slp force-pushed the revert-vsock-unconfirmed branch from cef45c9 to b2434ea Compare March 31, 2025 12:05
slp added 2 commits March 31, 2025 08:07
As indicated in 3bd3b98, when a client
in the guest attempts to send data to the server immediately after
connecting to it, we might receive data in the TCP socket before we're
able to actually process it.

To avoid this situation, delay setting EventSet::IN until the vsock
transport has been established in confirm_connect.

Signed-off-by: Sergio Lopez <[email protected]>
On slow hosts (i.e. running with nested virt), when the server is
inside the guest, the client may fail to connect because the first is
not yet ready.

Make the test attempt to connect multiple times, waiting a second
between attempts.

Signed-off-by: Sergio Lopez <[email protected]>
@slp slp force-pushed the revert-vsock-unconfirmed branch from b2434ea to f8cef18 Compare March 31, 2025 12:07
@slp slp mentioned this pull request Mar 31, 2025
@nohajc
Copy link
Contributor

nohajc commented Mar 31, 2025

I can confirm it fixes my issue. Thank you!

Copy link
Collaborator

@mtjhrc mtjhrc left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@tylerfanelli tylerfanelli left a comment

Choose a reason for hiding this comment

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

LGTM.

@jakecorrenti jakecorrenti merged commit 097b88a into containers:main Mar 31, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vsock + TSI regression
5 participants