Skip to content

vsock: implemented DialContext #53

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

balena
Copy link

@balena balena commented Feb 2, 2023

This PR simply implements a function DialContext so that a context can be used to terminate the dialing. VSOCK has lower chances to delay obtaining a response while dialing, but it can still occur.

Moreover, it is important to be supported while integrating with go-libp2p. Check github.com/balena/go-libp2p-vsock. It allows enclaves to dial remote libp2p nodes by means of relays.

Fred78290 added a commit to Fred78290/vsock that referenced this pull request Jan 3, 2025
@balena
Copy link
Author

balena commented Jan 4, 2025

@mdlayher gentle bump

Copy link
Owner

@mdlayher mdlayher left a comment

Choose a reason for hiding this comment

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

LGTM, sorry I missed this. To clarify this all works as you'd expect in your calling code? I ask because I attempted context plumbing in mdlayher/socket and I think I got a lot of it right but I haven't heavily exercised it with every system call.

@balena
Copy link
Author

balena commented Jan 4, 2025

I can't speak for every case around, but the same worked for AWS VSOCKs, which is definitely a sub case of the more general VSOCK support. Anyways, the context is being passed to the Connect function call that originally received an inlined context.Background() instance. It will be a matter for the Connect to honor the passed context or not.

@mdlayher
Copy link
Owner

mdlayher commented Jan 6, 2025

I'm actually not sure why actions isn't running here, let me see.

@mdlayher
Copy link
Owner

mdlayher commented Jan 6, 2025

I'm good with this but will want to get CI updated first. Will set a reminder.

@natesales
Copy link

The placeholder function for !linux platforms is missing a context parameter:

diff --git a/vsock_others.go b/vsock_others.go
index 5c1e88e..d46fb90 100644
--- a/vsock_others.go
+++ b/vsock_others.go
@@ -4,6 +4,7 @@
 package vsock

 import (
+       "context"
        "fmt"
        "net"
        "os"
@@ -26,7 +27,7 @@ func (*listener) Addr() net.Addr                { return nil }
 func (*listener) Close() error                  { return errUnimplemented }
 func (*listener) SetDeadline(_ time.Time) error { return errUnimplemented }

-func dial(_, _ uint32, _ *Config) (*Conn, error) { return nil, errUnimplemented }
+func dial(_ context.Context, _, _ uint32, _ *Config) (*Conn, error) { return nil, errUnimplemented }

 type conn struct{}

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.

3 participants