Skip to content
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

Added HV Socket known IDs, Dial, bug fixes #239

Merged
merged 3 commits into from
Jul 21, 2022

Conversation

helsaawy
Copy link
Contributor

@helsaawy helsaawy commented Mar 23, 2022

Added:

  • Well-know Hyper-V VMIDs for parents, children, and loopback.
  • VSock interop service GUID.
  • Dial() and DialContext() to dial a specific Hyper-V socket at a known address (along with a corresponding HvsockDialer struct.

Bug fixes:

  • Dial (and Listen) now properly initialize and set properties of their sockets after ConnectEx (and AcceptEx).
  • The socketError used by bind was incorrect, it should be int32(-1), not uintptr(^0)

Created a sockets package, currently only with syscalls to Bind, ConnectEx and GetSockName, bypassing syscall/windows restrictions on the types that cane be used

Signed-off-by: Hamza El-Saawy [email protected]

@helsaawy helsaawy requested a review from a team as a code owner March 23, 2022 19:57
@helsaawy helsaawy mentioned this pull request Mar 23, 2022
go.mod Outdated Show resolved Hide resolved
hvsock.go Outdated Show resolved Hide resolved
@kevpar
Copy link
Member

kevpar commented Mar 24, 2022

This seems like a change that should be split across multiple PRs. For instance can we add sockets package, and then use it in another PR?

I'd also like to understand better what the motivation is for these changes. What can a user of go-winio not do without these changes?

hvsock.go Outdated Show resolved Hide resolved
@helsaawy
Copy link
Contributor Author

helsaawy commented Mar 24, 2022

This seems like a change that should be split across multiple PRs. For instance can we add sockets package, and then use it in another PR?

I'd also like to understand better what the motivation is for these changes. What can a user of go-winio not do without these changes?

Without the sockets package, a user wouldn't be able to call GetPeerName, Bind, or ConnectEx on an arbitrary socket type (the syscall packages only allow them on types implemented internal to them). We had our own bind internally here, but it wasnt exported and required handling raw pointers.

Those three (GetPeerName, Bind, andConnectEx) are fairly useless on their own, but they allow use to easily define our own Dial for HV sockets.

I could add sockets alone, but I think it would look fairly unnecessary, as without Hvsock defining its own socket type, it would appear fairly redundant with golang's socket functions.

That said, I can split this into sockets and HV sockets updates (or HV sockets feature additions and bug fixes).

hvsock.go Outdated Show resolved Hide resolved
hvsock.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
hvsock.go Show resolved Hide resolved
hvsock.go Outdated Show resolved Hide resolved
pkg/sockets/sockets.go Outdated Show resolved Hide resolved
@helsaawy helsaawy force-pushed the he/hvsock branch 2 times, most recently from df6977a to 144d754 Compare May 5, 2022 20:40
hvsock.go Outdated Show resolved Hide resolved
Added:
* Well-know Hyper-V VMIDs for parents, children, and loopback.
* VSock interop service GUID.
* `Dial()` and `DialContext()` to dial a specific Hyper-V socket at a
  known address (along with a corresponding `HvsockDialer` struct.

Bug fixes:
* Dial (and Listen) now properly initialize and set properties of their
  sockets after ConnectEx (and AcceptEx).
* The `socketError` used by `bind` was incorrect, it should be `int32(-1)`,
  not `uintptr(^0)`
* Return errors for `(*HvsockConn) SetDeadline`

Created a `sockets` package, currently only with syscalls to `Bind`,
`ConnectEx` and `GetSockName`, bypassing `syscall/windows` restrictions
on the types that can do so.

Signed-off-by: Hamza El-Saawy <[email protected]>
hvsock.go Outdated Show resolved Hide resolved
hvsock.go Outdated Show resolved Hide resolved
hvsock.go Outdated Show resolved Hide resolved
internal/socket/rawaddr.go Outdated Show resolved Hide resolved
internal/socket/rawaddr.go Outdated Show resolved Hide resolved
internal/socket/socket.go Outdated Show resolved Hide resolved
hvsock.go Show resolved Hide resolved
hvsock.go Show resolved Hide resolved
hvsock.go Outdated Show resolved Hide resolved
hvsock.go Show resolved Hide resolved
hvsock.go Outdated Show resolved Hide resolved
hvsock.go Outdated Show resolved Hide resolved
hvsock.go Outdated Show resolved Hide resolved
hvsock.go Show resolved Hide resolved
* comments and todos statements
* spelling
* removed dead code
* changed names to be more conventional
* unexported socket code
* made `(*HvsockDialer) Dial` take `Context`, removed `DialContext`
* added default `Dial` function
* rebased onto main
* cleaned up `Dial(` retry loop

Signed-off-by: Hamza El-Saawy <[email protected]>
@helsaawy helsaawy force-pushed the he/hvsock branch 2 times, most recently from 0997ca6 to a615ab2 Compare July 5, 2022 23:04
internal/socket/socket.go Outdated Show resolved Hide resolved
hvsock.go Outdated Show resolved Hide resolved
hvsock.go Show resolved Hide resolved
@msscotb msscotb assigned anmaxvl and unassigned kevpar Jul 13, 2022
@helsaawy helsaawy merged commit d68e55c into microsoft:master Jul 21, 2022
@helsaawy helsaawy deleted the he/hvsock branch July 21, 2022 23:35
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.

4 participants