Fix statsclient for VPP 20.05-rc0 (master)

- this change fixes panic that was occurring with recent VPP  that was caused by incorrectly calculated vector length
- converting returned vector length from uint64 to uint32 results in correct length value

Change-Id: I76a4b9d147c3df3bea9d3e5ef5853e2809dc42e8
Signed-off-by: Ondrej Fabry <[email protected]>

Author: ondrej-fabry

CHANGELOG.md

@@ -11,6 +11,12 @@ This file lists changes for the GoVPP releases.
 -
 -->
 
+## 0.3.2
+> _20 March 2020_
+
+### Fixes
+- statsclient: Fix panic occurring with VPP 20.05-rc0 (master)
+
 ## 0.3.0
 > _18 March 2020_
 

adapter/stats_api.go

@@ -113,7 +113,7 @@ func (n Name) String() string {
 	return string(n)
 }
 
-// Data represents some type of stat which is usually defined by StatType.
+// Stat represents some type of stat which is usually defined by StatType.
 type Stat interface {
 	// IsZero returns true if all of its values equal to zero.
 	IsZero() bool
@@ -205,7 +205,7 @@ func ReduceSimpleCounterStatIndex(s SimpleCounterStat, i int) uint64 {
 	return val
 }
 
-// ReduceSimpleCounterStatIndex returns reduced CombinedCounterStat s for index i.
+// ReduceCombinedCounterStatIndex returns reduced CombinedCounterStat s for index i.
 func ReduceCombinedCounterStatIndex(s CombinedCounterStat, i int) [2]uint64 {
 	var val [2]uint64
 	for _, w := range s {

adapter/statsclient/stat_segment.go

@@ -52,20 +52,11 @@ type statSegment struct {
 	legacyVersion bool
 }
 
-func (c *statSegment) getStatDirVector() unsafe.Pointer {
-	dirOffset, _, _ := c.getOffsets()
-	return unsafe.Pointer(&c.sharedHeader[dirOffset])
-}
-
-func (c *statSegment) getStatDirIndex(p unsafe.Pointer, index uint32) *statSegDirectoryEntry {
-	return (*statSegDirectoryEntry)(unsafe.Pointer(uintptr(p) + uintptr(index)*unsafe.Sizeof(statSegDirectoryEntry{})))
-}
-
-func (c *statSegment) getHeader() (header statSegSharedHeader) {
+func (c *statSegment) getHeader() (header sharedHeader) {
 	if c.legacyVersion {
-		return statSegHeaderLegacy(c.sharedHeader)
+		return loadSharedHeaderLegacy(c.sharedHeader)
 	}
-	return statSegHeader(c.sharedHeader)
+	return loadSharedHeader(c.sharedHeader)
 }
 
 func (c *statSegment) getEpoch() (int64, bool) {
@@ -129,17 +120,17 @@ func (c *statSegment) connect(sockName string) error {
 		return fmt.Errorf("mapping shared memory failed: %v", err)
 	}
 
+	Log.Debugf("successfuly mmapped shared memory segment (size: %v) %v", size, len(data))
+
 	c.sharedHeader = data
 	c.memorySize = size
 
-	Log.Debugf("successfuly mmapped shared memory segment (size: %v)", size)
-
-	hdr := statSegHeader(c.sharedHeader)
+	hdr := loadSharedHeader(c.sharedHeader)
 	Log.Debugf("stat segment header: %+v", hdr)
 
 	if hdr.legacyVersion() {
 		c.legacyVersion = true
-		hdr = statSegHeaderLegacy(c.sharedHeader)
+		hdr = loadSharedHeaderLegacy(c.sharedHeader)
 		Log.Debugf("falling back to legacy version (VPP <=19.04) of stat segment (header: %+v)", hdr)
 	}
 
@@ -167,6 +158,16 @@ func (c *statSegment) disconnect() error {
 
 type statDirectoryType int32
 
+const (
+	statDirIllegal               = 0
+	statDirScalarIndex           = 1
+	statDirCounterVectorSimple   = 2
+	statDirCounterVectorCombined = 3
+	statDirErrorIndex            = 4
+	statDirNameVector            = 5
+	statDirEmpty                 = 6
+)
+
 func (t statDirectoryType) String() string {
 	return adapter.StatType(t).String()
 }
@@ -182,14 +183,23 @@ type statSegDirectoryEntry struct {
 	name         [128]byte
 }
 
+func (c *statSegment) getStatDirVector() unsafe.Pointer {
+	dirOffset, _, _ := c.getOffsets()
+	return unsafe.Pointer(&c.sharedHeader[dirOffset])
+}
+
+func (c *statSegment) getStatDirIndex(p unsafe.Pointer, index uint32) *statSegDirectoryEntry {
+	return (*statSegDirectoryEntry)(unsafe.Pointer(uintptr(p) + uintptr(index)*unsafe.Sizeof(statSegDirectoryEntry{})))
+}
+
 func (c *statSegment) copyEntryData(dirEntry *statSegDirectoryEntry) adapter.Stat {
 	dirType := adapter.StatType(dirEntry.directoryType)
 
 	switch dirType {
-	case adapter.ScalarIndex:
+	case statDirScalarIndex:
 		return adapter.ScalarStat(dirEntry.unionData)
 
-	case adapter.ErrorIndex:
+	case statDirErrorIndex:
 		_, errOffset, _ := c.getOffsets()
 		offsetVector := unsafe.Pointer(&c.sharedHeader[errOffset])
 
@@ -200,17 +210,19 @@ func (c *statSegment) copyEntryData(dirEntry *statSegDirectoryEntry) adapter.Sta
 			val := *(*adapter.Counter)(statSegPointer(offsetVector, offset))
 			errData = val
 		} else {
-			vecLen := vectorLen(offsetVector)
-			for i := uint64(0); i < vecLen; i++ {
+			vecLen := uint32(vectorLen(offsetVector))
+
+			for i := uint32(0); i < vecLen; i++ {
 				cb := *(*uint64)(statSegPointer(offsetVector, uintptr(i)*unsafe.Sizeof(uint64(0))))
 				offset := uintptr(cb) + uintptr(dirEntry.unionData)*unsafe.Sizeof(adapter.Counter(0))
+				debugf("error index, cb: %d, offset: %d", cb, offset)
 				val := *(*adapter.Counter)(statSegPointer(unsafe.Pointer(&c.sharedHeader[0]), offset))
 				errData += val
 			}
 		}
 		return adapter.ErrorStat(errData)
 
-	case adapter.SimpleCounterVector:
+	case statDirCounterVectorSimple:
 		if dirEntry.unionData == 0 {
 			debugf("offset invalid for %s", dirEntry.name)
 			break
@@ -219,24 +231,24 @@ func (c *statSegment) copyEntryData(dirEntry *statSegDirectoryEntry) adapter.Sta
 			break
 		}
 
-		vecLen := vectorLen(unsafe.Pointer(&c.sharedHeader[dirEntry.unionData]))
+		vecLen := uint32(vectorLen(unsafe.Pointer(&c.sharedHeader[dirEntry.unionData])))
 		offsetVector := statSegPointer(unsafe.Pointer(&c.sharedHeader[0]), uintptr(dirEntry.offsetVector))
 
 		data := make([][]adapter.Counter, vecLen)
-		for i := uint64(0); i < vecLen; i++ {
+		for i := uint32(0); i < vecLen; i++ {
 			cb := *(*uint64)(statSegPointer(offsetVector, uintptr(i)*unsafe.Sizeof(uint64(0))))
 			counterVec := unsafe.Pointer(&c.sharedHeader[uintptr(cb)])
-			vecLen2 := vectorLen(counterVec)
+			vecLen2 := uint32(vectorLen(counterVec))
 			data[i] = make([]adapter.Counter, vecLen2)
-			for j := uint64(0); j < vecLen2; j++ {
+			for j := uint32(0); j < vecLen2; j++ {
 				offset := uintptr(j) * unsafe.Sizeof(adapter.Counter(0))
 				val := *(*adapter.Counter)(statSegPointer(counterVec, offset))
 				data[i][j] = val
 			}
 		}
 		return adapter.SimpleCounterStat(data)
 
-	case adapter.CombinedCounterVector:
+	case statDirCounterVectorCombined:
 		if dirEntry.unionData == 0 {
 			debugf("offset invalid for %s", dirEntry.name)
 			break
@@ -245,24 +257,24 @@ func (c *statSegment) copyEntryData(dirEntry *statSegDirectoryEntry) adapter.Sta
 			break
 		}
 
-		vecLen := vectorLen(unsafe.Pointer(&c.sharedHeader[dirEntry.unionData]))
+		vecLen := uint32(vectorLen(unsafe.Pointer(&c.sharedHeader[dirEntry.unionData])))
 		offsetVector := statSegPointer(unsafe.Pointer(&c.sharedHeader[0]), uintptr(dirEntry.offsetVector))
 
 		data := make([][]adapter.CombinedCounter, vecLen)
-		for i := uint64(0); i < vecLen; i++ {
+		for i := uint32(0); i < vecLen; i++ {
 			cb := *(*uint64)(statSegPointer(offsetVector, uintptr(i)*unsafe.Sizeof(uint64(0))))
 			counterVec := unsafe.Pointer(&c.sharedHeader[uintptr(cb)])
-			vecLen2 := vectorLen(counterVec)
+			vecLen2 := uint32(vectorLen(counterVec))
 			data[i] = make([]adapter.CombinedCounter, vecLen2)
-			for j := uint64(0); j < vecLen2; j++ {
+			for j := uint32(0); j < vecLen2; j++ {
 				offset := uintptr(j) * unsafe.Sizeof(adapter.CombinedCounter{})
 				val := *(*adapter.CombinedCounter)(statSegPointer(counterVec, offset))
 				data[i][j] = val
 			}
 		}
 		return adapter.CombinedCounterStat(data)
 
-	case adapter.NameVector:
+	case statDirNameVector:
 		if dirEntry.unionData == 0 {
 			debugf("offset invalid for %s", dirEntry.name)
 			break
@@ -271,21 +283,21 @@ func (c *statSegment) copyEntryData(dirEntry *statSegDirectoryEntry) adapter.Sta
 			break
 		}
 
-		vecLen := vectorLen(unsafe.Pointer(&c.sharedHeader[dirEntry.unionData]))
+		vecLen := uint32(vectorLen(unsafe.Pointer(&c.sharedHeader[dirEntry.unionData])))
 		offsetVector := statSegPointer(unsafe.Pointer(&c.sharedHeader[0]), uintptr(dirEntry.offsetVector))
 
 		data := make([]adapter.Name, vecLen)
-		for i := uint64(0); i < vecLen; i++ {
+		for i := uint32(0); i < vecLen; i++ {
 			cb := *(*uint64)(statSegPointer(offsetVector, uintptr(i)*unsafe.Sizeof(uint64(0))))
 			if cb == 0 {
 				debugf("name vector out of range for %s (%v)", dirEntry.name, i)
 				continue
 			}
 			nameVec := unsafe.Pointer(&c.sharedHeader[cb])
-			vecLen2 := vectorLen(nameVec)
+			vecLen2 := uint32(vectorLen(nameVec))
 
 			nameStr := make([]byte, 0, vecLen2)
-			for j := uint64(0); j < vecLen2; j++ {
+			for j := uint32(0); j < vecLen2; j++ {
 				offset := uintptr(j) * unsafe.Sizeof(byte(0))
 				val := *(*byte)(statSegPointer(nameVec, offset))
 				if val > 0 {
@@ -296,11 +308,13 @@ func (c *statSegment) copyEntryData(dirEntry *statSegDirectoryEntry) adapter.Sta
 		}
 		return adapter.NameStat(data)
 
+	case statDirEmpty:
+		// no-op
+
 	default:
 		// TODO: monitor occurrences with metrics
 		debugf("Unknown type %d for stat entry: %q", dirEntry.directoryType, dirEntry.name)
 	}
-
 	return nil
 }
 

adapter/statsclient/statsclient.go

@@ -121,12 +121,24 @@ func (c *StatsClient) ListStats(patterns ...string) (names []string, err error)
 	if err != nil {
 		return nil, err
 	}
+
+	dirVector := c.getStatDirVector()
+	vecLen := uint32(vectorLen(dirVector))
+
 	for _, index := range indexes {
-		name, err := c.entryName(index)
-		if err != nil {
-			return nil, err
+		if index >= vecLen {
+			return nil, fmt.Errorf("stat entry index %d out of dir vector len (%d)", index, vecLen)
 		}
-		names = append(names, name)
+
+		dirEntry := c.getStatDirIndex(dirVector, index)
+		var name []byte
+		for n := 0; n < len(dirEntry.name); n++ {
+			if dirEntry.name[n] == 0 {
+				name = dirEntry.name[:n]
+				break
+			}
+		}
+		names = append(names, string(name))
 	}
 
 	if !c.accessEnd(&sa) {
@@ -142,11 +154,11 @@ func (c *StatsClient) DumpStats(patterns ...string) (entries []adapter.StatEntry
 		return nil, adapter.ErrStatsAccessFailed
 	}
 
-	dir, err := c.listIndexes(patterns...)
+	indexes, err := c.listIndexes(patterns...)
 	if err != nil {
 		return nil, err
 	}
-	if entries, err = c.dumpEntries(dir); err != nil {
+	if entries, err = c.dumpEntries(indexes); err != nil {
 		return nil, err
 	}
 
@@ -261,7 +273,7 @@ func (c *StatsClient) listIndexes(patterns ...string) (indexes []uint32, err err
 
 func (c *StatsClient) listIndexesFunc(f func(name []byte) bool) (indexes []uint32, err error) {
 	if f == nil {
-		// there is around ~3150 stats, so to avoid too many allocations
+		// there is around ~3157 stats, so to avoid too many allocations
 		// we set capacity to 3200 when listing all stats
 		indexes = make([]uint32, 0, 3200)
 	}
@@ -290,33 +302,18 @@ func (c *StatsClient) listIndexesFunc(f func(name []byte) bool) (indexes []uint3
 	return indexes, nil
 }
 
-func (c *StatsClient) entryName(index uint32) (string, error) {
+func (c *StatsClient) dumpEntries(indexes []uint32) (entries []adapter.StatEntry, err error) {
 	dirVector := c.getStatDirVector()
-	vecLen := uint32(vectorLen(dirVector))
+	dirLen := uint32(vectorLen(dirVector))
 
-	if index >= vecLen {
-		return "", fmt.Errorf("stat entry index %d out of range (%d)", index, vecLen)
-	}
-
-	dirEntry := c.getStatDirIndex(dirVector, index)
-
-	var name []byte
-	for n := 0; n < len(dirEntry.name); n++ {
-		if dirEntry.name[n] == 0 {
-			name = dirEntry.name[:n]
-			break
-		}
-	}
-
-	return string(name), nil
-}
+	debugf("dumping entres for %d indexes", len(indexes))
 
-func (c *StatsClient) dumpEntries(indexes []uint32) (entries []adapter.StatEntry, err error) {
 	entries = make([]adapter.StatEntry, 0, len(indexes))
-
-	dirVector := c.getStatDirVector()
-
 	for _, index := range indexes {
+		if index >= dirLen {
+			return nil, fmt.Errorf("stat entry index %d out of dir vector length (%d)", index, dirLen)
+		}
+
 		dirEntry := c.getStatDirIndex(dirVector, index)
 
 		var name []byte
@@ -326,6 +323,12 @@ func (c *StatsClient) dumpEntries(indexes []uint32) (entries []adapter.StatEntry
 				break
 			}
 		}
+
+		if Debug {
+			debugf(" - %3d. dir: %q type: %v offset: %d union: %d", index, name,
+				adapter.StatType(dirEntry.directoryType), dirEntry.offsetVector, dirEntry.unionData)
+		}
+
 		if len(name) == 0 {
 			continue
 		}

adapter/statsclient/statseg.go

@@ -19,12 +19,16 @@ type sharedHeaderBase struct {
 	statsOffset     int64
 }
 
-type statSegSharedHeader struct {
+type sharedHeaderV0 struct {
+	sharedHeaderBase
+}
+
+type sharedHeader struct {
 	version uint64
 	sharedHeaderBase
 }
 
-func (h *statSegSharedHeader) legacyVersion() bool {
+func (h *sharedHeader) legacyVersion() bool {
 	// older VPP (<=19.04) did not have version in stat segment header
 	// we try to provide fallback support by skipping it in header
 	if h.version > maxVersion && h.inProgress > 1 && h.epoch == 0 {
@@ -33,8 +37,8 @@ func (h *statSegSharedHeader) legacyVersion() bool {
 	return false
 }
 
-func statSegHeader(b []byte) (header statSegSharedHeader) {
-	h := (*statSegSharedHeader)(unsafe.Pointer(&b[0]))
+func loadSharedHeader(b []byte) (header sharedHeader) {
+	h := (*sharedHeader)(unsafe.Pointer(&b[0]))
 	header.version = atomic.LoadUint64(&h.version)
 	header.epoch = atomic.LoadInt64(&h.epoch)
 	header.inProgress = atomic.LoadInt64(&h.inProgress)
@@ -44,8 +48,8 @@ func statSegHeader(b []byte) (header statSegSharedHeader) {
 	return
 }
 
-func statSegHeaderLegacy(b []byte) (header statSegSharedHeader) {
-	h := (*sharedHeaderBase)(unsafe.Pointer(&b[0]))
+func loadSharedHeaderLegacy(b []byte) (header sharedHeader) {
+	h := (*sharedHeaderV0)(unsafe.Pointer(&b[0]))
 	header.version = 0
 	header.epoch = atomic.LoadInt64(&h.epoch)
 	header.inProgress = atomic.LoadInt64(&h.inProgress)
@@ -90,7 +94,7 @@ type vecHeader struct {
 }
 
 func vectorLen(v unsafe.Pointer) uint64 {
-	vec := *(*vecHeader)(unsafe.Pointer(uintptr(v) - unsafe.Sizeof(uintptr(0))))
+	vec := *(*vecHeader)(unsafe.Pointer(uintptr(v) - unsafe.Sizeof(uint64(0))))
 	return vec.length
 }