diff --git a/.circleci/config.yml b/.circleci/config.yml index 49ca031..7749f1e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -52,7 +52,7 @@ jobs: paths: - "/usr/include/google/protobuf" - - run: sudo apt update && sudo apt install python3 python3-pip libgirepository1.0-dev + - run: sudo apt update && sudo apt install python3 python3-pip libgirepository1.0-dev gawk - run: ./lazy.sh devsetup - run: make zrepl-bin diff --git a/Makefile b/Makefile index 8077f7a..01f9815 100644 --- a/Makefile +++ b/Makefile @@ -93,6 +93,7 @@ clean: docs-clean .PHONY: bins-all lint test vet zrepl-bin platformtest-bin BINS_ALL_TARGETS := zrepl-bin platformtest-bin vet lint +GO_SUPPORTS_ILLUMOS := $(shell $(GO) version | gawk -F '.' '/^go version /{split($$0, comps, " "); split(comps[3], v, "."); if (v[1] == "go1" && v[2] >= 13) { print "illumos"; } else { print "noillumos"; }}') bins-all: $(MAKE) $(BINS_ALL_TARGETS) GOOS=freebsd GOARCH=amd64 $(MAKE) $(BINS_ALL_TARGETS) GOOS=freebsd GOARCH=386 @@ -100,6 +101,14 @@ bins-all: $(MAKE) $(BINS_ALL_TARGETS) GOOS=linux GOARCH=arm64 $(MAKE) $(BINS_ALL_TARGETS) GOOS=linux GOARCH=386 $(MAKE) $(BINS_ALL_TARGETS) GOOS=darwin GOARCH=amd64 + $(MAKE) $(BINS_ALL_TARGETS) GOOS=solaris GOARCH=amd64 +ifeq ($(GO_SUPPORTS_ILLUMOS), illumos) + $(MAKE) $(BINS_ALL_TARGETS) GOOS=illumos GOARCH=amd64 +else ifeq ($(GO_SUPPORTS_ILLUMOS), noillumos) + @echo "SKIPPING ILLUMOS BUILD BECAUSE GO VERSION DOESN'T SUPPORT IT" +else + @echo "CANNOT DETERMINE WHETHER GO VERSION SUPPORTS GOOS=illumos"; exit 1 +endif lint: $(GO_ENV_VARS) $(GOLANGCI_LINT) run ./... diff --git a/rpc/dataconn/timeoutconn/timeoutconn.go b/rpc/dataconn/timeoutconn/timeoutconn.go index 44168d8..24b682c 100644 --- a/rpc/dataconn/timeoutconn/timeoutconn.go +++ b/rpc/dataconn/timeoutconn/timeoutconn.go @@ -14,7 +14,6 @@ import ( "sync/atomic" "syscall" "time" - "unsafe" ) type Wire interface { @@ -155,25 +154,6 @@ type SyscallConner interface { var _ SyscallConner = (*net.TCPConn)(nil) -func buildIovecs(buffers net.Buffers) (totalLen int64, vecs []syscall.Iovec) { - vecs = make([]syscall.Iovec, 0, len(buffers)) - for i := range buffers { - totalLen += int64(len(buffers[i])) - if len(buffers[i]) == 0 { - continue - } - - v := syscall.Iovec{ - Base: &buffers[i][0], - } - // syscall.Iovec.Len has platform-dependent size, thus use SetLen - v.SetLen(len(buffers[i])) - - vecs = append(vecs, v) - } - return totalLen, vecs -} - // Reads the given buffers full: // Think of io.ReadvFull, but for net.Buffers + using the readv syscall. // @@ -184,25 +164,10 @@ func buildIovecs(buffers net.Buffers) (totalLen int64, vecs []syscall.Iovec) { // then + io.EOF is returned. This behavior is different to io.ReadFull // which returns io.ErrUnexpectedEOF. func (c Conn) ReadvFull(buffers net.Buffers) (n int64, err error) { - totalLen, iovecs := buildIovecs(buffers) - if debugReadvNoShortReadsAssertEnable { - defer debugReadvNoShortReadsAssert(totalLen, n, err) - } - scc, ok := c.Wire.(SyscallConner) - if !ok { - return c.readvFallback(buffers) - } - raw, err := scc.SyscallConn() - if err == SyscallConnNotSupported { - return c.readvFallback(buffers) - } - if err != nil { - return 0, err - } - n, err = c.readv(raw, iovecs) - return + return c.readv(buffers) } +// invoked by c.readv if readv system call cannot be used func (c Conn) readvFallback(nbuffers net.Buffers) (n int64, err error) { buffers := [][]byte(nbuffers) for i := range buffers { @@ -226,91 +191,3 @@ func (c Conn) readvFallback(nbuffers net.Buffers) (n int64, err error) { } return n, nil } - -func (c Conn) readv(rawConn syscall.RawConn, iovecs []syscall.Iovec) (n int64, err error) { - for len(iovecs) > 0 { - if err := c.renewReadDeadline(); err != nil { - return n, err - } - oneN, oneErr := c.doOneReadv(rawConn, &iovecs) - n += oneN - if netErr, ok := oneErr.(net.Error); ok && netErr.Timeout() && oneN > 0 { // TODO likely not working - continue - } else if oneErr == nil && oneN > 0 { - continue - } else { - return n, oneErr - } - } - return n, nil -} - -func (c Conn) doOneReadv(rawConn syscall.RawConn, iovecs *[]syscall.Iovec) (n int64, err error) { - rawReadErr := rawConn.Read(func(fd uintptr) (done bool) { - // iovecs, n and err must not be shadowed! - - // NOTE: unsafe.Pointer safety rules - // https://tip.golang.org/pkg/unsafe/#Pointer - // - // (4) Conversion of a Pointer to a uintptr when calling syscall.Syscall. - // ... - // uintptr() conversions must appear within the syscall.Syscall argument list. - // (even though we are not the escape analysis Likely not ) - thisReadN, _, errno := syscall.Syscall( - syscall.SYS_READV, - fd, - uintptr(unsafe.Pointer(&(*iovecs)[0])), - uintptr(len(*iovecs)), - ) - if thisReadN == ^uintptr(0) { - if errno == syscall.EAGAIN { - return false - } - err = syscall.Errno(errno) - return true - } - if int(thisReadN) < 0 { - panic("unexpected return value") - } - n += int64(thisReadN) // TODO check overflow - - // shift iovecs forward - for left := int(thisReadN); left > 0; { - // conversion to uint does not change value, see TestIovecLenFieldIsMachineUint, and left > 0 - thisIovecConsumedCompletely := uint((*iovecs)[0].Len) <= uint(left) - if thisIovecConsumedCompletely { - // Update left, cannot go below 0 due to - // a) definition of thisIovecConsumedCompletely - // b) left > 0 due to loop invariant - // Convertion .Len to int64 is thus also safe now, because it is < left < INT_MAX - left -= int((*iovecs)[0].Len) - *iovecs = (*iovecs)[1:] - } else { - // trim this iovec to remaining length - - // NOTE: unsafe.Pointer safety rules - // https://tip.golang.org/pkg/unsafe/#Pointer - // (3) Conversion of a Pointer to a uintptr and back, with arithmetic. - // ... - // Note that both conversions must appear in the same expression, - // with only the intervening arithmetic between them: - (*iovecs)[0].Base = (*byte)(unsafe.Pointer(uintptr(unsafe.Pointer((*iovecs)[0].Base)) + uintptr(left))) - curVecNewLength := uint((*iovecs)[0].Len) - uint(left) // casts to uint do not change value - (*iovecs)[0].SetLen(int(curVecNewLength)) // int and uint have the same size, no change of value - - break // inner - } - } - if thisReadN == 0 { - err = io.EOF - return true - } - return true - }) - - if rawReadErr != nil { - err = rawReadErr - } - - return n, err -} diff --git a/rpc/dataconn/timeoutconn/timeoutconn_debug.go b/rpc/dataconn/timeoutconn/timeoutconn_debug.go deleted file mode 100644 index d5a7cb3..0000000 --- a/rpc/dataconn/timeoutconn/timeoutconn_debug.go +++ /dev/null @@ -1,19 +0,0 @@ -package timeoutconn - -import ( - "fmt" - "io" -) - -const debugReadvNoShortReadsAssertEnable = false - -func debugReadvNoShortReadsAssert(expectedLen, returnedLen int64, returnedErr error) { - readShort := expectedLen != returnedLen - if !readShort { - return - } - if returnedErr != io.EOF { - return - } - panic(fmt.Sprintf("ReadvFull short and error is not EOF%v\n", returnedErr)) -} diff --git a/rpc/dataconn/timeoutconn/timeoutconn_readv_unsupported.go b/rpc/dataconn/timeoutconn/timeoutconn_readv_unsupported.go new file mode 100644 index 0000000..5cc169b --- /dev/null +++ b/rpc/dataconn/timeoutconn/timeoutconn_readv_unsupported.go @@ -0,0 +1,11 @@ +// +build illumos solaris + +package timeoutconn + +import "net" + +func (c Conn) readv(buffers net.Buffers) (n int64, err error) { + // Go does not expose the SYS_READV symbol for Solaris / Illumos - do they have it? + // Anyhow, use the fallback + return c.readvFallback(buffers) +} diff --git a/rpc/dataconn/timeoutconn/timoutconn_readv.go b/rpc/dataconn/timeoutconn/timoutconn_readv.go new file mode 100644 index 0000000..6432510 --- /dev/null +++ b/rpc/dataconn/timeoutconn/timoutconn_readv.go @@ -0,0 +1,133 @@ +// +build !illumos +// +build !solaris + +package timeoutconn + +import ( + "io" + "net" + "syscall" + "unsafe" +) + +func buildIovecs(buffers net.Buffers) (totalLen int64, vecs []syscall.Iovec) { + vecs = make([]syscall.Iovec, 0, len(buffers)) + for i := range buffers { + totalLen += int64(len(buffers[i])) + if len(buffers[i]) == 0 { + continue + } + + v := syscall.Iovec{ + Base: &buffers[i][0], + } + // syscall.Iovec.Len has platform-dependent size, thus use SetLen + v.SetLen(len(buffers[i])) + + vecs = append(vecs, v) + } + return totalLen, vecs +} + +func (c Conn) readv(buffers net.Buffers) (n int64, err error) { + + scc, ok := c.Wire.(SyscallConner) + if !ok { + return c.readvFallback(buffers) + } + rawConn, err := scc.SyscallConn() + if err == SyscallConnNotSupported { + return c.readvFallback(buffers) + } + if err != nil { + return 0, err + } + + _, iovecs := buildIovecs(buffers) + + for len(iovecs) > 0 { + if err := c.renewReadDeadline(); err != nil { + return n, err + } + oneN, oneErr := c.doOneReadv(rawConn, &iovecs) + n += oneN + if netErr, ok := oneErr.(net.Error); ok && netErr.Timeout() && oneN > 0 { // TODO likely not working + continue + } else if oneErr == nil && oneN > 0 { + continue + } else { + return n, oneErr + } + } + return n, nil +} + +func (c Conn) doOneReadv(rawConn syscall.RawConn, iovecs *[]syscall.Iovec) (n int64, err error) { + rawReadErr := rawConn.Read(func(fd uintptr) (done bool) { + // iovecs, n and err must not be shadowed! + + // NOTE: unsafe.Pointer safety rules + // https://tip.golang.org/pkg/unsafe/#Pointer + // + // (4) Conversion of a Pointer to a uintptr when calling syscall.Syscall. + // ... + // uintptr() conversions must appear within the syscall.Syscall argument list. + // (even though we are not the escape analysis Likely not ) + thisReadN, _, errno := syscall.Syscall( + syscall.SYS_READV, + fd, + uintptr(unsafe.Pointer(&(*iovecs)[0])), + uintptr(len(*iovecs)), + ) + if thisReadN == ^uintptr(0) { + if errno == syscall.EAGAIN { + return false + } + err = syscall.Errno(errno) + return true + } + if int(thisReadN) < 0 { + panic("unexpected return value") + } + n += int64(thisReadN) // TODO check overflow + + // shift iovecs forward + for left := int(thisReadN); left > 0; { + // conversion to uint does not change value, see TestIovecLenFieldIsMachineUint, and left > 0 + thisIovecConsumedCompletely := uint((*iovecs)[0].Len) <= uint(left) + if thisIovecConsumedCompletely { + // Update left, cannot go below 0 due to + // a) definition of thisIovecConsumedCompletely + // b) left > 0 due to loop invariant + // Convertion .Len to int64 is thus also safe now, because it is < left < INT_MAX + left -= int((*iovecs)[0].Len) + *iovecs = (*iovecs)[1:] + } else { + // trim this iovec to remaining length + + // NOTE: unsafe.Pointer safety rules + // https://tip.golang.org/pkg/unsafe/#Pointer + // (3) Conversion of a Pointer to a uintptr and back, with arithmetic. + // ... + // Note that both conversions must appear in the same expression, + // with only the intervening arithmetic between them: + (*iovecs)[0].Base = (*byte)(unsafe.Pointer(uintptr(unsafe.Pointer((*iovecs)[0].Base)) + uintptr(left))) + curVecNewLength := uint((*iovecs)[0].Len) - uint(left) // casts to uint do not change value + (*iovecs)[0].SetLen(int(curVecNewLength)) // int and uint have the same size, no change of value + + break // inner + } + } + if thisReadN == 0 { + err = io.EOF + return true + } + return true + }) + + if rawReadErr != nil { + err = rawReadErr + } + + return n, err +}