SDRRepository: implement two-part reads

pkg/ipmi/reserve_sdr_repository.go

@@ -29,9 +29,9 @@ func (*ReserveSDRRepositoryRsp) NextLayerType() gopacket.LayerType {
 }
 
 func (r *ReserveSDRRepositoryRsp) DecodeFromBytes(data []byte, df gopacket.DecodeFeedback) error {
-	if len(data) != 2 {
+	if len(data) < 2 {
 		df.SetTruncated()
-		return fmt.Errorf("response must be 2 bytes, got %v", len(data))
+		return fmt.Errorf("response must be at least 2 bytes, got %v", len(data))
 	}
 
 	r.BaseLayer.Contents = data[:2]

pkg/ipmi/sdr.go

@@ -38,13 +38,14 @@ type SDR struct {
 	//
 	// This means the max SDR size on the wire is 260 bytes. In practice, OEM
 	// records notwithstanding, it is unlikely to be >60.
+	//
 	// If it weren't for this field, the limit for the whole SDR including
 	// header could theoretically be 255 + the max supported payload size (the
 	// SDR Repo Device commands provide no way to address subsequent sections
 	// for reading).
-	//
-	// payload contains the record key and body.
 	Length uint8
+
+	// payload contains the record key and body.
 }
 
 func (*SDR) LayerType() gopacket.LayerType {
@@ -68,7 +69,7 @@ func (s *SDR) DecodeFromBytes(data []byte, df gopacket.DecodeFeedback) error {
 	s.ID = RecordID(binary.LittleEndian.Uint16(data[0:2]))
 	s.Version = bcd.Decode(data[2]&0xf)*10 + bcd.Decode(data[2]>>4)
 	s.Type = RecordType(data[3])
-	s.Length = data[4]
+	s.Length = uint8(data[4])
 
 	s.BaseLayer.Contents = data[:5]
 	s.BaseLayer.Payload = data[5:]

sdr_repository.go

@@ -11,10 +11,14 @@ import (
 	"github.com/google/gopacket"
 )
 
+const (
+	sdrHeaderLength = uint8(5)
+	sdrMaxLength    = uint8(64)
+)
+
 var (
 	errSDRRepositoryModified = errors.New(
 		"the SDR Repository was modified during enumeration")
-	numBytesSDRHeader = uint8(5)
 )
 
 // SDRRepository is a retrieved SDR Repository. For the time being, this is a
@@ -76,7 +80,7 @@ func walkSDRs(ctx context.Context, s Session) (SDRRepository, error) {
 	getSDRCmd := &ipmi.GetSDRCmd{
 		Req: ipmi.GetSDRReq{
 			RecordID:      ipmi.RecordIDFirst,
-			Length:        numBytesSDRHeader,                   // read header only
+			Length:        sdrHeaderLength,                     // read header only
 			ReservationID: reserveSDRRepoCmdResp.ReservationID, // needed for partial reads
 		},
 	}
@@ -95,45 +99,44 @@ func walkSDRs(ctx context.Context, s Session) (SDRRepository, error) {
 				// we can't set NoCopy because we reuse getSDRCmd.Rsp
 			})
 		if headerPacket == nil {
-			return nil, fmt.Errorf("invalid SDR for record ID %d: empty packet",
-				getSDRCmd.Req.RecordID)
+			return nil, fmt.Errorf("invalid SDR: %v", getSDRCmd)
 		}
 		headerLayer := headerPacket.Layer(ipmi.LayerTypeSDR)
 		if headerLayer == nil {
-			return nil, fmt.Errorf("invalid SDR for record ID %d: missing SDR layer",
-				getSDRCmd.Req.RecordID)
+			return nil, fmt.Errorf("packet is missing SDR layer: %v", getSDRCmd)
 		}
 		header := headerLayer.(*ipmi.SDR)
 
 		if header.Type == ipmi.RecordTypeFullSensor {
-			if header.Length > 64-numBytesSDRHeader {
-				// SDR exceeds the specified length of 64. Need to implement partial reads.
-				return nil, fmt.Errorf("invalid SDR for record ID %d: length %d exceeds max of 64 bytes",
-					getSDRCmd.Req.RecordID, header.Length)
+			if header.Length > sdrMaxLength {
+				// SDR exceeds the specified max length, which means we need to implement
+				// partial reading. Hopefully we'll be alright - yet to see a SDR >70 bytes
+				// long - they're specified as 64 after all.
+				return nil, fmt.Errorf("SDR length %d exceeds max of %d bytes: %v",
+					header.Length, sdrMaxLength, getSDRCmd)
 			}
 
-			getSDRCmd.Req.Offset = numBytesSDRHeader
+			getSDRCmd.Req.Offset = sdrHeaderLength
 			getSDRCmd.Req.Length = header.Length
 			if err := ValidateResponse(s.SendCommand(ctx, getSDRCmd)); err != nil {
 				return nil, err
 			}
 			fsrPacket := gopacket.NewPacket(getSDRCmd.Rsp.Payload, ipmi.LayerTypeFullSensorRecord,
 				gopacket.DecodeOptions{Lazy: true})
 			if fsrPacket == nil {
-				return nil, fmt.Errorf("invalid SDR for record ID %d: empty FSR packet",
-					getSDRCmd.Req.RecordID)
+				return nil, fmt.Errorf("invalid Full Sensor Record: %v", getSDRCmd)
 			}
 			fsrLayer := fsrPacket.Layer(ipmi.LayerTypeFullSensorRecord)
 			if fsrLayer == nil {
-				return nil, fmt.Errorf("invalid SDR for record ID %d: missing FSR layer",
-					getSDRCmd.Req.RecordID)
+				return nil, fmt.Errorf("packet is missing Full Sensor Record layer: %v",
+					getSDRCmd)
 			}
 			repo[getSDRCmd.Req.RecordID] = fsrLayer.(*ipmi.FullSensorRecord)
 		}
 
 		getSDRCmd.Req.RecordID = getSDRCmd.Rsp.Next
 		getSDRCmd.Req.Offset = 0x00
-		getSDRCmd.Req.Length = numBytesSDRHeader
+		getSDRCmd.Req.Length = sdrHeaderLength
 	}
 	return repo, nil
 }