From ea8461c99f76ca1f29381e8352c147738efecb5f Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Fri, 16 Sep 2022 17:20:52 +0200 Subject: [PATCH 1/4] vsock: Use net.FileConn to create VirtioSocketConnection Currently, newVirtioSocketConnection manually sets its objc file descriptor to non-blocking, calls dup() on it, ... so that it matches what go expects from a file descriptor. go provides a helper doing all of this for us: net.FileConn() This commit makes use of that helper. This will allow for more code simplifications in the next commits. However this removes vz.VirtioSocketConnection.FileDescriptor() since the file descriptor was created internally by net.FileConn(). It would be possible to keep it, but I'm not sure it's really useful. It would be more consistent with other go APIs to add a File() method if we want to expose this. One side-effect of this change is to fix connections returned by ConnectToPort. At the moment, they are missing a call to Dup() for the file descriptor they use. Without this, their file descriptor is closed too early, and the connection cannot be used. This fixes https://github.com/Code-Hex/vz/issues/54 --- socket.go | 77 ++++++++++++++++--------------------------------------- 1 file changed, 22 insertions(+), 55 deletions(-) diff --git a/socket.go b/socket.go index 9c9dbf9..6175761 100644 --- a/socket.go +++ b/socket.go @@ -12,7 +12,6 @@ import ( "os" "runtime" "runtime/cgo" - "syscall" "time" "unsafe" @@ -106,11 +105,11 @@ func connectionHandler(connPtr, errPtr, cgoHandlerPtr unsafe.Pointer) { handler := cgoHandler.Value().(func(*VirtioSocketConnection, error)) defer cgoHandler.Delete() // see: startHandler - conn := newVirtioSocketConnection(connPtr) if err := newNSError(errPtr); err != nil { - handler(conn, err) + handler(nil, err) } else { - handler(conn, nil) + conn, err := newVirtioSocketConnection(connPtr) + handler(conn, err) } } @@ -138,7 +137,7 @@ type dup struct { err error } -var shouldAcceptNewConnectionHandlers = map[unsafe.Pointer]func(conn *VirtioSocketConnection) bool{} +var shouldAcceptNewConnectionHandlers = map[unsafe.Pointer]func(conn *VirtioSocketConnection, err error) bool{} // NewVirtioSocketListener creates a new VirtioSocketListener with connection handler. // @@ -165,10 +164,9 @@ func NewVirtioSocketListener(handler func(conn *VirtioSocketConnection, err erro go handler(dup.conn, dup.err) } }() - shouldAcceptNewConnectionHandlers[ptr] = func(conn *VirtioSocketConnection) bool { - dupConn, err := conn.dup() + shouldAcceptNewConnectionHandlers[ptr] = func(conn *VirtioSocketConnection, err error) bool { dupCh <- dup{ - conn: dupConn, + conn: conn, err: err, } return true // must be connected @@ -185,8 +183,8 @@ func shouldAcceptNewConnectionHandler(listenerPtr, connPtr, devicePtr unsafe.Poi _ = devicePtr // NOTO(codehex): Is this really required? How to use? // see: startHandler - conn := newVirtioSocketConnection(connPtr) - return (C.bool)(shouldAcceptNewConnectionHandlers[listenerPtr](conn)) + conn, err := newVirtioSocketConnection(connPtr) + return (C.bool)(shouldAcceptNewConnectionHandlers[listenerPtr](conn, err)) } // VirtioSocketConnection is a port-based connection between the guest operating system and the host computer. @@ -204,25 +202,25 @@ func shouldAcceptNewConnectionHandler(listenerPtr, connPtr, devicePtr unsafe.Poi type VirtioSocketConnection struct { sourcePort uint32 destinationPort uint32 - fileDescriptor uintptr - file *os.File + conn net.Conn laddr net.Addr // local raddr net.Addr // remote } var _ net.Conn = (*VirtioSocketConnection)(nil) -func newVirtioSocketConnection(ptr unsafe.Pointer) *VirtioSocketConnection { +func newVirtioSocketConnection(ptr unsafe.Pointer) (*VirtioSocketConnection, error) { vzVirtioSocketConnection := C.convertVZVirtioSocketConnection2Flat(ptr) - err := unix.SetNonblock(int(vzVirtioSocketConnection.fileDescriptor), true) + file := os.NewFile((uintptr)(vzVirtioSocketConnection.fileDescriptor), "") + defer file.Close() + rawConn, err := net.FileConn(file) if err != nil { - fmt.Printf("set nonblock: %s\n", err.Error()) + return nil, err } conn := &VirtioSocketConnection{ sourcePort: (uint32)(vzVirtioSocketConnection.sourcePort), destinationPort: (uint32)(vzVirtioSocketConnection.destinationPort), - fileDescriptor: (uintptr)(vzVirtioSocketConnection.fileDescriptor), - file: os.NewFile((uintptr)(vzVirtioSocketConnection.fileDescriptor), ""), + conn: rawConn, laddr: &Addr{ CID: unix.VMADDR_CID_HOST, Port: (uint32)(vzVirtioSocketConnection.destinationPort), @@ -232,40 +230,18 @@ func newVirtioSocketConnection(ptr unsafe.Pointer) *VirtioSocketConnection { Port: (uint32)(vzVirtioSocketConnection.sourcePort), }, } - return conn -} - -func (v *VirtioSocketConnection) dup() (*VirtioSocketConnection, error) { - nfd, err := syscall.Dup(int(v.fileDescriptor)) - if err != nil { - return nil, &net.OpError{ - Op: "dup", - Net: "vsock", - Source: v.laddr, - Addr: v.raddr, - Err: err, - } - } - - dupConn := new(VirtioSocketConnection) - *dupConn = *v - dupConn.fileDescriptor = uintptr(nfd) - dupConn.file = os.NewFile(uintptr(nfd), v.file.Name()) - dupConn.laddr = v.laddr - dupConn.raddr = v.raddr - - return dupConn, nil + return conn, nil } // Read reads data from connection of the vsock protocol. -func (v *VirtioSocketConnection) Read(b []byte) (n int, err error) { return v.file.Read(b) } +func (v *VirtioSocketConnection) Read(b []byte) (n int, err error) { return v.conn.Read(b) } // Write writes data to the connection of the vsock protocol. -func (v *VirtioSocketConnection) Write(b []byte) (n int, err error) { return v.file.Write(b) } +func (v *VirtioSocketConnection) Write(b []byte) (n int, err error) { return v.conn.Write(b) } // Close will be called when caused something error in socket. func (v *VirtioSocketConnection) Close() error { - return v.file.Close() + return v.conn.Close() } // LocalAddr returns the local network address. @@ -277,13 +253,13 @@ func (v *VirtioSocketConnection) RemoteAddr() net.Addr { return v.raddr } // SetDeadline sets the read and write deadlines associated // with the connection. It is equivalent to calling both // SetReadDeadline and SetWriteDeadline. -func (v *VirtioSocketConnection) SetDeadline(t time.Time) error { return v.file.SetDeadline(t) } +func (v *VirtioSocketConnection) SetDeadline(t time.Time) error { return v.conn.SetDeadline(t) } // SetReadDeadline sets the deadline for future Read calls // and any currently-blocked Read call. // A zero value for t means Read will not time out. func (v *VirtioSocketConnection) SetReadDeadline(t time.Time) error { - return v.file.SetReadDeadline(t) + return v.conn.SetReadDeadline(t) } // SetWriteDeadline sets the deadline for future Write calls @@ -292,7 +268,7 @@ func (v *VirtioSocketConnection) SetReadDeadline(t time.Time) error { // some of the data was successfully written. // A zero value for t means Write will not time out. func (v *VirtioSocketConnection) SetWriteDeadline(t time.Time) error { - return v.file.SetWriteDeadline(t) + return v.conn.SetWriteDeadline(t) } // DestinationPort returns the destination port number of the connection. @@ -305,15 +281,6 @@ func (v *VirtioSocketConnection) SourcePort() uint32 { return v.sourcePort } -// FileDescriptor returns the file descriptor associated with the socket. -// -// Data is sent by writing to the file descriptor. -// Data is received by reading from the file descriptor. -// A file descriptor of -1 indicates a closed connection. -func (v *VirtioSocketConnection) FileDescriptor() uintptr { - return v.fileDescriptor -} - // Addr represents a network end point address for the vsock protocol. type Addr struct { CID uint32 From 8dc8720cf8ecfc7ccc274aef21098553402a25b3 Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Tue, 11 Oct 2022 15:50:22 +0200 Subject: [PATCH 2/4] vsock: Remove ports from VirtioSocketConnection The source and destination vsock ports for VirtioSocketConnection are set both as `sourcePort` and `destinationPort`, but also as part of `laddr` and `raddr`. This commit only keeps the latter as it's redundant to have both. --- socket.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/socket.go b/socket.go index 6175761..7cdad5a 100644 --- a/socket.go +++ b/socket.go @@ -200,11 +200,9 @@ func shouldAcceptNewConnectionHandler(listenerPtr, connPtr, devicePtr unsafe.Poi // // see: https://developer.apple.com/documentation/virtualization/vzvirtiosocketconnection?language=objc type VirtioSocketConnection struct { - sourcePort uint32 - destinationPort uint32 - conn net.Conn - laddr net.Addr // local - raddr net.Addr // remote + conn net.Conn + laddr *Addr // local + raddr *Addr // remote } var _ net.Conn = (*VirtioSocketConnection)(nil) @@ -218,9 +216,7 @@ func newVirtioSocketConnection(ptr unsafe.Pointer) (*VirtioSocketConnection, err return nil, err } conn := &VirtioSocketConnection{ - sourcePort: (uint32)(vzVirtioSocketConnection.sourcePort), - destinationPort: (uint32)(vzVirtioSocketConnection.destinationPort), - conn: rawConn, + conn: rawConn, laddr: &Addr{ CID: unix.VMADDR_CID_HOST, Port: (uint32)(vzVirtioSocketConnection.destinationPort), @@ -273,12 +269,12 @@ func (v *VirtioSocketConnection) SetWriteDeadline(t time.Time) error { // DestinationPort returns the destination port number of the connection. func (v *VirtioSocketConnection) DestinationPort() uint32 { - return v.destinationPort + return v.laddr.Port } // SourcePort returns the source port number of the connection. func (v *VirtioSocketConnection) SourcePort() uint32 { - return v.sourcePort + return v.raddr.Port } // Addr represents a network end point address for the vsock protocol. From ff58ded7b1b5f59fad4f9dfc79c10e0233fb8932 Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Tue, 11 Oct 2022 15:52:54 +0200 Subject: [PATCH 3/4] vsock: Simplify VirtioSocketConnection implementation Since VirtioSocketConnection is now backed by a net.Conn instance, we can let go reuse the net.Conn methods from the backing instance instead of reimplementing them ourselves. --- socket.go | 39 ++------------------------------------- 1 file changed, 2 insertions(+), 37 deletions(-) diff --git a/socket.go b/socket.go index 7cdad5a..323a4df 100644 --- a/socket.go +++ b/socket.go @@ -12,7 +12,6 @@ import ( "os" "runtime" "runtime/cgo" - "time" "unsafe" "golang.org/x/sys/unix" @@ -200,13 +199,11 @@ func shouldAcceptNewConnectionHandler(listenerPtr, connPtr, devicePtr unsafe.Poi // // see: https://developer.apple.com/documentation/virtualization/vzvirtiosocketconnection?language=objc type VirtioSocketConnection struct { - conn net.Conn + net.Conn laddr *Addr // local raddr *Addr // remote } -var _ net.Conn = (*VirtioSocketConnection)(nil) - func newVirtioSocketConnection(ptr unsafe.Pointer) (*VirtioSocketConnection, error) { vzVirtioSocketConnection := C.convertVZVirtioSocketConnection2Flat(ptr) file := os.NewFile((uintptr)(vzVirtioSocketConnection.fileDescriptor), "") @@ -216,7 +213,7 @@ func newVirtioSocketConnection(ptr unsafe.Pointer) (*VirtioSocketConnection, err return nil, err } conn := &VirtioSocketConnection{ - conn: rawConn, + Conn: rawConn, laddr: &Addr{ CID: unix.VMADDR_CID_HOST, Port: (uint32)(vzVirtioSocketConnection.destinationPort), @@ -229,44 +226,12 @@ func newVirtioSocketConnection(ptr unsafe.Pointer) (*VirtioSocketConnection, err return conn, nil } -// Read reads data from connection of the vsock protocol. -func (v *VirtioSocketConnection) Read(b []byte) (n int, err error) { return v.conn.Read(b) } - -// Write writes data to the connection of the vsock protocol. -func (v *VirtioSocketConnection) Write(b []byte) (n int, err error) { return v.conn.Write(b) } - -// Close will be called when caused something error in socket. -func (v *VirtioSocketConnection) Close() error { - return v.conn.Close() -} - // LocalAddr returns the local network address. func (v *VirtioSocketConnection) LocalAddr() net.Addr { return v.laddr } // RemoteAddr returns the remote network address. func (v *VirtioSocketConnection) RemoteAddr() net.Addr { return v.raddr } -// SetDeadline sets the read and write deadlines associated -// with the connection. It is equivalent to calling both -// SetReadDeadline and SetWriteDeadline. -func (v *VirtioSocketConnection) SetDeadline(t time.Time) error { return v.conn.SetDeadline(t) } - -// SetReadDeadline sets the deadline for future Read calls -// and any currently-blocked Read call. -// A zero value for t means Read will not time out. -func (v *VirtioSocketConnection) SetReadDeadline(t time.Time) error { - return v.conn.SetReadDeadline(t) -} - -// SetWriteDeadline sets the deadline for future Write calls -// and any currently-blocked Write call. -// Even if write times out, it may return n > 0, indicating that -// some of the data was successfully written. -// A zero value for t means Write will not time out. -func (v *VirtioSocketConnection) SetWriteDeadline(t time.Time) error { - return v.conn.SetWriteDeadline(t) -} - // DestinationPort returns the destination port number of the connection. func (v *VirtioSocketConnection) DestinationPort() uint32 { return v.laddr.Port From 12bf645af164844e195a2f022623bef4c1835155 Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Thu, 22 Sep 2022 17:31:54 +0200 Subject: [PATCH 4/4] vsock: Remove dup/dupCh The dup/dupCh types are now badly named as the dup() function is gone. They seem redundant though. shouldAcceptNewConnectionHandler will queue (conn, err) in dupCh. NewVirtioSocketListener() started a go routine which iterates on dupCh, and runs `handler(conn, err)` in its own go routine. It seems shouldAcceptNewConnectionHandler could directly run `handler(conn, err)` in a new go routine instead of going through an intermediate channel before doing this. This makes the code simpler. --- socket.go | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/socket.go b/socket.go index 323a4df..173cae6 100644 --- a/socket.go +++ b/socket.go @@ -131,11 +131,6 @@ type VirtioSocketListener struct { pointer } -type dup struct { - conn *VirtioSocketConnection - err error -} - var shouldAcceptNewConnectionHandlers = map[unsafe.Pointer]func(conn *VirtioSocketConnection, err error) bool{} // NewVirtioSocketListener creates a new VirtioSocketListener with connection handler. @@ -157,17 +152,8 @@ func NewVirtioSocketListener(handler func(conn *VirtioSocketConnection, err erro }, } - dupCh := make(chan dup, 1) - go func() { - for dup := range dupCh { - go handler(dup.conn, dup.err) - } - }() shouldAcceptNewConnectionHandlers[ptr] = func(conn *VirtioSocketConnection, err error) bool { - dupCh <- dup{ - conn: conn, - err: err, - } + go handler(conn, err) return true // must be connected }