Added HV Socket tests #235
Conversation
Added tests for core Hyper-V socket functionality, including testing CloseRead and CloseWrite, as well as checking addresses are appropriate and timeouts work. Added resources to support testing: * 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)` Added functionality to register application (service GUID and name) in the registry to enable Hyper-V sockets using that service GUID Created a `sockets` package, currently only with syscalls to `Bind`, `ConnectEx` and `GetSockName`, bypassing `syscall/windows` restrictions on the types that can do so. Upgraded to go v1.17 to use `unsafe.Slice()`. Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
| sock, err := newHvSocket() | ||
| if err != nil { | ||
| return nil, err | ||
| return nil, conn.opErr("dial", err) |
There was a problem hiding this comment.
Should use the op var we made on the first line
| windows.Handle(sock.handle), &sa, | ||
| nil /*sendBuf*/, 0, /*sendDataLen*/ | ||
| &bytes, (*windows.Overlapped)(unsafe.Pointer(&c.o))) |
There was a problem hiding this comment.
I'm not sure what the idiomatic answer is here, but I generally feel like every argument on a new line is easier to read. Also I'd prefer // comments to the c block comment style
| // "golang.org/x/sys/windows".ConnectEx and .Bind only accept internal implementations of the | ||
| // their sockaddr interface, so they cannot be used with HvsockAddr | ||
| // Replicate functionality here from | ||
| // https://cs.opensource.google/go/x/sys/+/master:windows/syscall_windows.go |
There was a problem hiding this comment.
Ahh, this may be why you defined bind above.
|
This does a heck of a lot more than the title leads on 😆. Preferably this should be split across multiple commits (possibly even multiple PRs). I don't think we should bump to 1.17 in this PR at the very least, we can shoot to get this in #230, but I think the more important fact is the dial and registry work here |
|
Ill split this into a functionality PR and a tests PR |
Added tests for Hyper-V sockets, including
CloseReadandCloseWrite, and that addresses are set appropriately.Bug fixes:
AcceptEx(andConnectEx), so thatGetSockName,GetPeerName, andShutdown(forCloseReadandCloseWrite) now function properly.socketErrorused to checkbind's return value was incorrect, it should beuintptr(^uint32(0)), not^uintptr(0)Added resources to support testing:
Dial()andDialContext()(along with a correspondingHvsockDialerstruct) to dial a specific Hyper-V socket at an address.Additionally, added functionality to register an application (service GUID and name) in the registry to enable Hyper-V sockets using that service GUID
Created a
socketspackage, currently only with syscalls toBind,ConnectExandGetSockName, bypassingsyscall/windowsrestrictions that they only be used on socket address types defined by the package.Upgraded to go v1.17 to use
unsafe.Slice().Signed-off-by: Hamza El-Saawy hamzaelsaawy@microsoft.com