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 <ofabry@cisco.com>

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 088c84d..72c7a27 100644 (file)
--- a/CHANGELOG.md
+++ b/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_
 

diff --git a/adapter/stats_api.go b/adapter/stats_api.go
index d67434c..90dbeb3 100644 (file)
--- a/adapter/stats_api.go
+++ b/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 {

diff --git a/adapter/statsclient/stat_segment.go b/adapter/statsclient/stat_segment.go
index 9f028eb..0d988ba 100644 (file)
--- a/adapter/statsclient/stat_segment.go
+++ b/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,16 +231,16 @@ 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
@@ -236,7 +248,7 @@ func (c *statSegment) copyEntryData(dirEntry *statSegDirectoryEntry) adapter.Sta
                }
                return adapter.SimpleCounterStat(data)
 
-       case adapter.CombinedCounterVector:
+       case statDirCounterVectorCombined:
                if dirEntry.unionData == 0 {
                        debugf("offset invalid for %s", dirEntry.name)
                        break
@@ -245,16 +257,16 @@ 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
@@ -262,7 +274,7 @@ func (c *statSegment) copyEntryData(dirEntry *statSegDirectoryEntry) adapter.Sta
                }
                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
 }
 

diff --git a/adapter/statsclient/statsclient.go b/adapter/statsclient/statsclient.go
index d4e5a56..9110275 100644 (file)
--- a/adapter/statsclient/statsclient.go
+++ b/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
                }

diff --git a/adapter/statsclient/statseg.go b/adapter/statsclient/statseg.go
index 7f1c381..42ab3de 100644 (file)
--- a/adapter/statsclient/statseg.go
+++ b/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
 }
 

diff --git a/version/version.go b/version/version.go
index 36eaa3a..ed5922e 100644 (file)
--- a/version/version.go
+++ b/version/version.go
@@ -23,7 +23,7 @@ import (
 
 var (
        name        = "govpp"
-       version     = "v0.3.0-dev"
+       version     = "v0.3.2"
        commitHash  = "unknown"
        buildBranch = "HEAD"
        buildStamp  = ""