VOM: stats fixes
- double free of m_stat_data_seg
- cleanup typedefs
Change-Id: I6aeb070471b6c5c97ac4379d01bd242136f80091
Signed-off-by: Neale Ranns <nranns@cisco.com>
diff --git a/extras/vom/vom/interface.cpp b/extras/vom/vom/interface.cpp
index 504511e..97f7776 100644
--- a/extras/vom/vom/interface.cpp
+++ b/extras/vom/vom/interface.cpp
@@ -418,7 +418,7 @@
}
void
-interface::set(counter_t count, const std::string& stat_type)
+interface::set(const counter_t& count, const std::string& stat_type)
{
if ("rx" == stat_type)
m_stats.m_rx = count;
diff --git a/extras/vom/vom/interface.hpp b/extras/vom/vom/interface.hpp
index 100c3ca..2f6511e 100644
--- a/extras/vom/vom/interface.hpp
+++ b/extras/vom/vom/interface.hpp
@@ -25,7 +25,6 @@
#include "vom/route_domain.hpp"
#include "vom/rpc_cmd.hpp"
#include "vom/singular_db.hpp"
-#include "vom/stat_client.hpp"
namespace VOM {
/**
@@ -619,7 +618,7 @@
/**
* Set the interface stat
*/
- void set(counter_t count, const std::string& stat_type);
+ void set(const counter_t& count, const std::string& stat_type);
/**
* enable the interface stats in the singular instance
diff --git a/extras/vom/vom/stat_client.cpp b/extras/vom/vom/stat_client.cpp
index b348c6c..6406e21 100644
--- a/extras/vom/vom/stat_client.cpp
+++ b/extras/vom/vom/stat_client.cpp
@@ -17,29 +17,22 @@
namespace VOM {
-stat_client::stat_data_t::stat_data_t()
- : m_name("")
- , m_type(STAT_DIR_TYPE_ILLEGAL)
-{
-}
-
-stat_client::stat_data_t::stat_data_t(stat_segment_data_t* stat_seg_data)
- : m_name(stat_seg_data->name)
- , m_type(stat_seg_data->type)
+stat_client::stat_data_t::stat_data_t(const stat_segment_data_t& stat_seg_data)
+ : m_name(stat_seg_data.name)
+ , m_type(stat_seg_data.type)
{
switch (m_type) {
case STAT_DIR_TYPE_SCALAR_INDEX:
- m_scalar_value = stat_seg_data->scalar_value;
+ m_scalar_value = stat_seg_data.scalar_value;
break;
case STAT_DIR_TYPE_COUNTER_VECTOR_SIMPLE:
- m_simple_counter_vec = stat_seg_data->simple_counter_vec;
+ m_simple_counter_vec = stat_seg_data.simple_counter_vec;
break;
case STAT_DIR_TYPE_COUNTER_VECTOR_COMBINED:
- m_combined_counter_vec =
- reinterpret_cast<counter_t**>(stat_seg_data->combined_counter_vec);
+ m_combined_counter_vec = stat_seg_data.combined_counter_vec;
break;
case STAT_DIR_TYPE_ERROR_INDEX:
- m_error_Value = stat_seg_data->error_value;
+ m_error_value = stat_seg_data.error_value;
break;
case STAT_DIR_TYPE_ILLEGAL:
break;
@@ -59,22 +52,25 @@
}
double
-stat_client::stat_data_t::get_stat_segment_scalar_data()
+stat_client::stat_data_t::get_stat_segment_scalar_data() const
{
return m_scalar_value;
}
+
uint64_t
-stat_client::stat_data_t::get_stat_segment_error_data()
+stat_client::stat_data_t::get_stat_segment_error_data() const
{
- return m_error_Value;
+ return m_error_value;
}
+
uint64_t**
-stat_client::stat_data_t::get_stat_segment_simple_counter_data()
+stat_client::stat_data_t::get_stat_segment_simple_counter_data() const
{
return m_simple_counter_vec;
}
-counter_t**
-stat_client::stat_data_t::get_stat_segment_combined_counter_data()
+
+vlib_counter_t**
+stat_client::stat_data_t::get_stat_segment_combined_counter_data() const
{
return m_combined_counter_vec;
}
@@ -83,6 +79,9 @@
: m_socket_name(socket_name)
, m_patterns()
, m_stat_connect(0)
+ , m_counter_vec()
+ , m_stat_seg_data(nullptr)
+ , m_stat_data()
{
m_patterns.push_back("/if");
}
@@ -91,6 +90,9 @@
: m_socket_name("/run/vpp/stats.sock")
, m_patterns(pattern)
, m_stat_connect(0)
+ , m_counter_vec()
+ , m_stat_seg_data(nullptr)
+ , m_stat_data()
{
}
@@ -99,6 +101,9 @@
: m_socket_name(socket_name)
, m_patterns(patterns)
, m_stat_connect(0)
+ , m_counter_vec()
+ , m_stat_seg_data(nullptr)
+ , m_stat_data()
{
}
@@ -106,6 +111,9 @@
: m_socket_name("/run/vpp/stats.sock")
, m_patterns()
, m_stat_connect(0)
+ , m_counter_vec()
+ , m_stat_seg_data(nullptr)
+ , m_stat_data()
{
m_patterns.push_back("/if");
}
@@ -127,8 +135,10 @@
int
stat_client::connect()
{
- if (stat_segment_connect(m_socket_name.c_str()) == 0)
+ if (stat_segment_connect(m_socket_name.c_str()) == 0) {
m_stat_connect = 1;
+ ls();
+ }
return m_stat_connect;
}
@@ -165,19 +175,17 @@
const stat_client::stat_data_vec_t&
stat_client::dump()
{
- stat_segment_data_t* ssd;
stat_segment_data_free(m_stat_seg_data);
if (m_stat_data.size()) {
m_stat_data.clear();
}
- ssd = stat_segment_dump(m_counter_vec);
- if (!ssd) {
+ m_stat_seg_data = stat_segment_dump(m_counter_vec);
+ if (!m_stat_seg_data) {
ls();
return m_stat_data;
}
- m_stat_seg_data = ssd;
- for (int i = 0; i < stat_segment_vec_len(ssd); i++) {
- stat_data_t sd(&ssd[i]);
+ for (int i = 0; i < stat_segment_vec_len(m_stat_seg_data); i++) {
+ stat_data_t sd(m_stat_seg_data[i]);
m_stat_data.push_back(sd);
}
return m_stat_data;
@@ -186,19 +194,17 @@
const stat_client::stat_data_vec_t&
stat_client::dump_entry(uint32_t index)
{
- stat_segment_data_t* ssd;
stat_segment_data_free(m_stat_seg_data);
if (m_stat_data.size()) {
m_stat_data.clear();
}
- ssd = stat_segment_dump_entry(index);
- if (!ssd) {
+ m_stat_seg_data = stat_segment_dump_entry(index);
+ if (!m_stat_seg_data) {
ls();
return m_stat_data;
}
- m_stat_seg_data = ssd;
- for (int i = 0; i < stat_segment_vec_len(ssd); i++) {
- stat_data_t sd(&ssd[i]);
+ for (int i = 0; i < stat_segment_vec_len(m_stat_seg_data); i++) {
+ stat_data_t sd(m_stat_seg_data[i]);
m_stat_data.push_back(sd);
}
return m_stat_data;
diff --git a/extras/vom/vom/stat_client.hpp b/extras/vom/vom/stat_client.hpp
index 4da968a..40729fe 100644
--- a/extras/vom/vom/stat_client.hpp
+++ b/extras/vom/vom/stat_client.hpp
@@ -26,13 +26,6 @@
namespace VOM {
-typedef struct
-{
- uint64_t packets;
- uint64_t bytes;
-} counter_t;
-
-typedef uint64_t stat_counter_t;
/**
* A representation of a stat client in VPP
*/
@@ -45,14 +38,9 @@
struct stat_data_t
{
/**
- * stat data constructor
- */
- stat_data_t();
-
- /**
* stat data custom constructor
*/
- stat_data_t(stat_segment_data_t* stat_seg_data);
+ stat_data_t(const stat_segment_data_t& stat_seg_data);
/**
* get name of stat
@@ -67,10 +55,10 @@
/**
* Get pointer to actual data
*/
- double get_stat_segment_scalar_data();
- uint64_t get_stat_segment_error_data();
- uint64_t** get_stat_segment_simple_counter_data();
- counter_t** get_stat_segment_combined_counter_data();
+ double get_stat_segment_scalar_data() const;
+ uint64_t get_stat_segment_error_data() const;
+ uint64_t** get_stat_segment_simple_counter_data() const;
+ vlib_counter_t** get_stat_segment_combined_counter_data() const;
private:
/**
@@ -89,9 +77,9 @@
union
{
double m_scalar_value;
- uint64_t m_error_Value;
- uint64_t** m_simple_counter_vec;
- counter_t** m_combined_counter_vec;
+ uint64_t m_error_value;
+ counter_t** m_simple_counter_vec;
+ vlib_counter_t** m_combined_counter_vec;
};
};
@@ -141,21 +129,6 @@
void disconnect();
/**
- * Get vector length of VPP style vector
- */
- int vec_len(void* vec);
-
- /**
- * Free VPP style vector
- */
- void vec_free(void* vec);
-
- /**
- * ls on the stat directory using given pattern
- */
- void ls();
-
- /**
* dump all the stats for given pattern
*/
const stat_data_vec_t& dump();
@@ -166,9 +139,9 @@
const stat_data_vec_t& dump_entry(uint32_t index);
/**
- * Free stat segment data
+ * Get vector length of VPP style vector
*/
- void data_free();
+ int vec_len(void* vec);
double heartbeat();
@@ -179,6 +152,21 @@
private:
/**
+ * Free VPP style vector
+ */
+ void vec_free(void* vec);
+
+ /**
+ * Free stat segment data
+ */
+ void data_free();
+
+ /**
+ * ls on the stat directory using given pattern
+ */
+ void ls();
+
+ /**
* socket name
*/
std::string m_socket_name;
diff --git a/extras/vom/vom/stat_reader.cpp b/extras/vom/vom/stat_reader.cpp
index 931d088..82e09ae 100644
--- a/extras/vom/vom/stat_reader.cpp
+++ b/extras/vom/vom/stat_reader.cpp
@@ -61,7 +61,7 @@
void
stat_reader::read()
{
- stat_client::stat_data_vec_t sd = m_client.dump();
+ const stat_client::stat_data_vec_t& sd = m_client.dump();
for (auto& sde : sd) {
std::string name;
@@ -74,28 +74,29 @@
case STAT_DIR_TYPE_COUNTER_VECTOR_SIMPLE:
case STAT_DIR_TYPE_ERROR_INDEX:
case STAT_DIR_TYPE_SCALAR_INDEX:
+ case STAT_DIR_TYPE_ILLEGAL:
break;
- case STAT_DIR_TYPE_COUNTER_VECTOR_COMBINED:
+ case STAT_DIR_TYPE_COUNTER_VECTOR_COMBINED: {
+ vlib_counter_t** data;
+
+ data = sde.get_stat_segment_combined_counter_data();
+
if (name.find("/if") != std::string::npos)
name.erase(0, 4);
for (auto& i : m_stat_itf_indexes) {
- counter_t count = {.packets = 0, .bytes = 0 };
- for (int k = 0; k < m_client.vec_len(
- sde.get_stat_segment_combined_counter_data());
- k++) {
- count.packets +=
- sde.get_stat_segment_combined_counter_data()[k][i].packets;
- count.bytes +=
- sde.get_stat_segment_combined_counter_data()[k][i].bytes;
+ counter_t count;
+
+ for (int k = 0; k < m_client.vec_len(data); k++) {
+ count.packets += data[k][i].packets;
+ count.bytes += data[k][i].bytes;
}
std::shared_ptr<interface> itf = interface::find(i);
if (itf)
itf->set(count, name);
}
break;
-
- default:;
+ }
}
}
diff --git a/extras/vom/vom/types.hpp b/extras/vom/vom/types.hpp
index ab21408..e3b21ad 100644
--- a/extras/vom/vom/types.hpp
+++ b/extras/vom/vom/types.hpp
@@ -378,6 +378,17 @@
std::vector<uint8_t> bytes;
};
+struct counter_t
+{
+ counter_t()
+ : packets(0)
+ , bytes(0)
+ {
+ }
+ uint64_t packets;
+ uint64_t bytes;
+};
+
/**
* Ostream operator for a MAC address
*/