fix: Use platform specific fdsets

* FdSet struct is platform specific depending on if we are on 32b or 64b platforms. We need to initialise it appropriately

* Set timeout on select directly and remove unnecessary busy loop while waiting to read response from socket

Signed-off-by: Mahendra Paipuri <[email protected]>

pkg/ipmi/client.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"math"
 	"os"
 	"syscall"
 	"time"
@@ -23,6 +24,10 @@ const (
 	IPMI_BMC_CHANNEL                = 0xF        //nolint:stylecheck
 )
 
+type timeout struct {
+	value time.Duration
+}
+
 type IPMIClient struct {
 	Logger  *slog.Logger
 	DevFile *os.File
@@ -74,7 +79,7 @@ func NewIPMIClient(devNum int, logger *slog.Logger) (*IPMIClient, error) {
 }
 
 // Do sends IPMI request and returns the response.
-func (i *IPMIClient) Do(req *ipmiReq, timeout time.Duration) (*ipmiResp, error) {
+func (i *IPMIClient) Do(req *ipmiReq, t time.Duration) (*ipmiResp, error) {
 	// Device file descriptor
 	fd := i.DevFile.Fd()
 
@@ -87,10 +92,15 @@ func (i *IPMIClient) Do(req *ipmiReq, timeout time.Duration) (*ipmiResp, error)
 
 	var activeFdSet unix.FdSet
 
-	serverFD := int(fd) //nolint: gosec
+	var serverFD int
+	if fd < math.MaxInt {
+		serverFD = int(fd) //nolint:gosec
+	} else {
+		serverFD = math.MaxInt - 1
+	}
 
 	FDZero(&activeFdSet)
-	FDSet(serverFD, &activeFdSet)
+	FDSet(fd, &activeFdSet)
 
 	resp := ipmiResp{}
 	addr := ipmiAddr{}
@@ -103,36 +113,35 @@ func (i *IPMIClient) Do(req *ipmiReq, timeout time.Duration) (*ipmiResp, error)
 		},
 	}
 
-	// Read the response back
-	start := time.Now()
+	// Set timeout for select
+	timeout := timeout{t}
 
-	for {
-		_, err := unix.Select(serverFD+1, &activeFdSet, nil, nil, nil)
-		if err != nil {
-			i.Logger.Error("Failed to receive response from IPMI device interface", "err", err)
+	_, err := unix.Select(serverFD+1, &activeFdSet, nil, nil, timeout.timeval())
+	if err != nil {
+		i.Logger.Error("Failed to receive response from IPMI device interface", "err", err)
 
-			return nil, fmt.Errorf("failed to receive response from IPMI device interface: %w", err)
-		}
+		return nil, fmt.Errorf("failed to receive response from IPMI device interface: %w", err)
+	}
 
-		if _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, fd, IPMICTL_RECEIVE_MSG_TRUNC, uintptr(unsafe.Pointer(&recv))); errno != 0 {
-			i.Logger.Error("Failed to read response from IPMI device interface", "err", errno)
+	// Check if fd is ready to read
+	if !FDIsSet(fd, &activeFdSet) {
+		i.Logger.Error("No response received from IPMI device interface")
 
-			return nil, fmt.Errorf("failed to read response from IPMI device interface: %w", errno)
-		}
+		return nil, errors.New("no response received from IPMI device interface")
+	}
 
-		// If Msgids match between response and request break
-		if req.Msgid == recv.Msgid {
-			i.Logger.Debug("IPMI response received from device interface")
+	// Read data into recv struct
+	if _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, fd, IPMICTL_RECEIVE_MSG_TRUNC, uintptr(unsafe.Pointer(&recv))); errno != 0 {
+		i.Logger.Error("Failed to read response from IPMI device interface", "err", errno)
 
-			break
-		}
+		return nil, fmt.Errorf("failed to read response from IPMI device interface: %w", errno)
+	}
 
-		// Check if we reached timeout
-		if time.Since(start) > timeout {
-			i.Logger.Error("IPMI response timed out")
+	// If Msgids match between response and request break
+	if req.Msgid != recv.Msgid {
+		i.Logger.Error("Received response with unexpected ID", "req_id", req.Msgid, "resp_id", recv.Msgid)
 
-			return nil, errors.New("timeout exceeded waiting for response from IPMI device interface")
-		}
+		return nil, fmt.Errorf("received response with unexpected id: %d", recv.Msgid)
 	}
 
 	// Read response data

pkg/ipmi/fd.go

@@ -2,22 +2,17 @@ package ipmi
 
 import "golang.org/x/sys/unix"
 
-// FDZero set to zero the fdSet.
-func FDZero(p *unix.FdSet) {
-	p.Bits = [16]int64{}
-}
-
 // FDSet set a fd of fdSet.
-func FDSet(fd int, p *unix.FdSet) {
-	p.Bits[fd/32] |= (1 << (uint(fd) % 32))
+func FDSet(fd uintptr, p *unix.FdSet) {
+	p.Bits[fd/NFDBits] |= (1 << (fd % NFDBits))
 }
 
 // FDClr clear a fd of fdSet.
-func FDClr(fd int, p *unix.FdSet) {
-	p.Bits[fd/32] &^= (1 << (uint(fd) % 32))
+func FDClr(fd uintptr, p *unix.FdSet) {
+	p.Bits[fd/NFDBits] &^= (1 << fd % NFDBits)
 }
 
 // FDIsSet return true if fd is set.
-func FDIsSet(fd int, p *unix.FdSet) bool {
-	return p.Bits[fd/32]&(1<<(uint(fd)%32)) != 0
+func FDIsSet(fd uintptr, p *unix.FdSet) bool {
+	return p.Bits[fd/NFDBits]&(1<<(fd%NFDBits)) != 0
 }

pkg/ipmi/helpers32b.go

@@ -0,0 +1,34 @@
+//go:build 386 || mips || mipsle
+// +build 386 mips mipsle
+
+package ipmi
+
+import (
+	"time"
+
+	"golang.org/x/sys/unix"
+)
+
+const (
+	// NFDBitS is the amount of bits per mask
+	NFDBits = 4 * 8
+)
+
+// FDZero set to zero the fdSet.
+func FDZero(p *unix.FdSet) {
+	p.Bits = [32]int32{}
+}
+
+// timeval returns a pointer to unix.Timeval based on timeout value.
+func (t *timeout) timeval() *unix.Timeval {
+	var timeoutS, timeoutUs time.Duration
+	if t.value >= time.Second {
+		timeoutS = t.value.Truncate(time.Second)
+		timeoutUs = t.value - timeoutS
+	} else {
+		timeoutS = 0
+		timeoutUs = t.value
+	}
+
+	return &unix.Timeval{Sec: int32(timeoutS.Seconds()), Usec: int32(timeoutUs.Microseconds())}
+}

pkg/ipmi/helpers64b.go

@@ -0,0 +1,34 @@
+//go:build amd64 || arm64 || mips64 || mips64le || ppc64le || riscv64
+// +build amd64 arm64 mips64 mips64le ppc64le riscv64
+
+package ipmi
+
+import (
+	"time"
+
+	"golang.org/x/sys/unix"
+)
+
+const (
+	// NFDBitS is the amount of bits per mask.
+	NFDBits = 8 * 8
+)
+
+// FDZero set to zero the fdSet.
+func FDZero(p *unix.FdSet) {
+	p.Bits = [16]int64{}
+}
+
+// timeval returns a pointer to unix.Timeval based on timeout value.
+func (t *timeout) timeval() *unix.Timeval {
+	var timeoutS, timeoutUs time.Duration
+	if t.value >= time.Second {
+		timeoutS = t.value.Truncate(time.Second)
+		timeoutUs = t.value - timeoutS
+	} else {
+		timeoutS = 0
+		timeoutUs = t.value
+	}
+
+	return &unix.Timeval{Sec: int64(timeoutS.Seconds()), Usec: timeoutUs.Microseconds()}
+}