From 16fc11a4e593f656167b02ba3146fc496c3115c8 Mon Sep 17 00:00:00 2001
From: Vivek Reddy Karri <vkarri@nvidia.com>
Date: Fri, 14 Apr 2023 21:47:40 +0000
Subject: [PATCH 1/9] Added def sflow sampling rates for 800g
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
---
cfgmgr/sflowmgr.cpp | 1 +
cfgmgr/sflowmgr.h | 2 ++
2 files changed, 3 insertions(+)
diff --git a/cfgmgr/sflowmgr.cpp b/cfgmgr/sflowmgr.cpp
index bb732e83d5..aa89003e23 100644
--- a/cfgmgr/sflowmgr.cpp
+++ b/cfgmgr/sflowmgr.cpp
@@ -12,6 +12,7 @@ using namespace swss;
map<string,string> sflowSpeedRateInitMap =
{
+ {SFLOW_SAMPLE_RATE_KEY_800G, SFLOW_SAMPLE_RATE_VALUE_800G},
{SFLOW_SAMPLE_RATE_KEY_400G, SFLOW_SAMPLE_RATE_VALUE_400G},
{SFLOW_SAMPLE_RATE_KEY_200G, SFLOW_SAMPLE_RATE_VALUE_200G},
{SFLOW_SAMPLE_RATE_KEY_100G, SFLOW_SAMPLE_RATE_VALUE_100G},
diff --git a/cfgmgr/sflowmgr.h b/cfgmgr/sflowmgr.h
index eb35ec2125..7ed8e9fc11 100644
--- a/cfgmgr/sflowmgr.h
+++ b/cfgmgr/sflowmgr.h
@@ -10,6 +10,7 @@
namespace swss {
+#define SFLOW_SAMPLE_RATE_KEY_800G "800000"
#define SFLOW_SAMPLE_RATE_KEY_400G "400000"
#define SFLOW_SAMPLE_RATE_KEY_200G "200000"
#define SFLOW_SAMPLE_RATE_KEY_100G "100000"
@@ -19,6 +20,7 @@ namespace swss {
#define SFLOW_SAMPLE_RATE_KEY_10G "10000"
#define SFLOW_SAMPLE_RATE_KEY_1G "1000"
+#define SFLOW_SAMPLE_RATE_VALUE_800G "800000"
#define SFLOW_SAMPLE_RATE_VALUE_400G "400000"
#define SFLOW_SAMPLE_RATE_VALUE_200G "200000"
#define SFLOW_SAMPLE_RATE_VALUE_100G "100000"
From 3430ba99486e81c8a4e425cf16268273f54e2f50 Mon Sep 17 00:00:00 2001
From: Vivek Reddy Karri <vkarri@nvidia.com>
Date: Tue, 23 May 2023 23:03:55 +0000
Subject: [PATCH 2/9] Infer sampling rate for any speed
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
---
cfgmgr/sflowmgr.cpp | 53 +++++++++++++++------------------------------
cfgmgr/sflowmgr.h | 25 +++------------------
2 files changed, 20 insertions(+), 58 deletions(-)
diff --git a/cfgmgr/sflowmgr.cpp b/cfgmgr/sflowmgr.cpp
index aa89003e23..d97aee1835 100644
--- a/cfgmgr/sflowmgr.cpp
+++ b/cfgmgr/sflowmgr.cpp
@@ -10,19 +10,6 @@
using namespace std;
using namespace swss;
-map<string,string> sflowSpeedRateInitMap =
-{
- {SFLOW_SAMPLE_RATE_KEY_800G, SFLOW_SAMPLE_RATE_VALUE_800G},
- {SFLOW_SAMPLE_RATE_KEY_400G, SFLOW_SAMPLE_RATE_VALUE_400G},
- {SFLOW_SAMPLE_RATE_KEY_200G, SFLOW_SAMPLE_RATE_VALUE_200G},
- {SFLOW_SAMPLE_RATE_KEY_100G, SFLOW_SAMPLE_RATE_VALUE_100G},
- {SFLOW_SAMPLE_RATE_KEY_50G, SFLOW_SAMPLE_RATE_VALUE_50G},
- {SFLOW_SAMPLE_RATE_KEY_40G, SFLOW_SAMPLE_RATE_VALUE_40G},
- {SFLOW_SAMPLE_RATE_KEY_25G, SFLOW_SAMPLE_RATE_VALUE_25G},
- {SFLOW_SAMPLE_RATE_KEY_10G, SFLOW_SAMPLE_RATE_VALUE_10G},
- {SFLOW_SAMPLE_RATE_KEY_1G, SFLOW_SAMPLE_RATE_VALUE_1G}
-};
-
SflowMgr::SflowMgr(DBConnector *cfgDb, DBConnector *appDb, const vector<string> &tableNames) :
Orch(cfgDb, tableNames),
m_cfgSflowTable(cfgDb, CFG_SFLOW_TABLE_NAME),
@@ -113,7 +100,7 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer)
if (new_port || (speed_change && !m_sflowPortConfMap[key].local_rate_cfg))
{
vector<FieldValueTuple> fvs;
- sflowGetGlobalInfo(fvs, m_sflowPortConfMap[key].speed);
+ sflowGetGlobalInfo(fvs, key);
m_appSflowSessionTable.set(key, fvs);
}
}
@@ -155,7 +142,7 @@ void SflowMgr::sflowHandleSessionAll(bool enable)
}
else
{
- sflowGetGlobalInfo(fvs, it.second.speed);
+ sflowGetGlobalInfo(fvs, it.first);
}
m_appSflowSessionTable.set(it.first, fvs);
}
@@ -186,21 +173,13 @@ void SflowMgr::sflowHandleSessionLocal(bool enable)
}
}
-void SflowMgr::sflowGetGlobalInfo(vector<FieldValueTuple> &fvs, string speed)
+void SflowMgr::sflowGetGlobalInfo(vector<FieldValueTuple> &fvs, const string& alias)
{
string rate;
FieldValueTuple fv1("admin_state", "up");
fvs.push_back(fv1);
- if (speed != SFLOW_ERROR_SPEED_STR && sflowSpeedRateInitMap.find(speed) != sflowSpeedRateInitMap.end())
- {
- rate = sflowSpeedRateInitMap[speed];
- }
- else
- {
- rate = SFLOW_ERROR_SPEED_STR;
- }
- FieldValueTuple fv2("sample_rate",rate);
+ FieldValueTuple fv2("sample_rate", findSamplingRate(alias));
fvs.push_back(fv2);
}
@@ -256,16 +235,7 @@ void SflowMgr::sflowCheckAndFillValues(string alias, vector<FieldValueTuple> &va
m_sflowPortConfMap[alias].local_rate_cfg)
{
string speed = m_sflowPortConfMap[alias].speed;
-
- if (speed != SFLOW_ERROR_SPEED_STR && sflowSpeedRateInitMap.find(speed) != sflowSpeedRateInitMap.end())
- {
- rate = sflowSpeedRateInitMap[speed];
- }
- else
- {
- rate = SFLOW_ERROR_SPEED_STR;
- }
- m_sflowPortConfMap[alias].rate = rate;
+ m_sflowPortConfMap[alias].rate = findSamplingRate(alias);
}
m_sflowPortConfMap[alias].local_rate_cfg = false;
FieldValueTuple fv("sample_rate", m_sflowPortConfMap[alias].rate);
@@ -285,6 +255,17 @@ void SflowMgr::sflowCheckAndFillValues(string alias, vector<FieldValueTuple> &va
}
}
+string SflowMgr::findSamplingRate(const string& alias)
+{
+ /* Default sampling rate is equal to the speed in Gbps or error */
+ if (m_sflowPortConfMap.find(alias) == m_sflowPortConfMap.end())
+ {
+ SWSS_LOG_ERROR("%s not found in port configuration map", alias.c_str());
+ return SFLOW_ERROR_SPEED_STR;
+ }
+ return m_sflowPortConfMap[alias].speed;
+}
+
void SflowMgr::doTask(Consumer &consumer)
{
SWSS_LOG_ENTER();
@@ -412,7 +393,7 @@ void SflowMgr::doTask(Consumer &consumer)
if (m_intfAllConf)
{
vector<FieldValueTuple> fvs;
- sflowGetGlobalInfo(fvs, m_sflowPortConfMap[key].speed);
+ sflowGetGlobalInfo(fvs, key);
m_appSflowSessionTable.set(key,fvs);
}
}
diff --git a/cfgmgr/sflowmgr.h b/cfgmgr/sflowmgr.h
index 7ed8e9fc11..a1377ccbc7 100644
--- a/cfgmgr/sflowmgr.h
+++ b/cfgmgr/sflowmgr.h
@@ -10,26 +10,6 @@
namespace swss {
-#define SFLOW_SAMPLE_RATE_KEY_800G "800000"
-#define SFLOW_SAMPLE_RATE_KEY_400G "400000"
-#define SFLOW_SAMPLE_RATE_KEY_200G "200000"
-#define SFLOW_SAMPLE_RATE_KEY_100G "100000"
-#define SFLOW_SAMPLE_RATE_KEY_50G "50000"
-#define SFLOW_SAMPLE_RATE_KEY_40G "40000"
-#define SFLOW_SAMPLE_RATE_KEY_25G "25000"
-#define SFLOW_SAMPLE_RATE_KEY_10G "10000"
-#define SFLOW_SAMPLE_RATE_KEY_1G "1000"
-
-#define SFLOW_SAMPLE_RATE_VALUE_800G "800000"
-#define SFLOW_SAMPLE_RATE_VALUE_400G "400000"
-#define SFLOW_SAMPLE_RATE_VALUE_200G "200000"
-#define SFLOW_SAMPLE_RATE_VALUE_100G "100000"
-#define SFLOW_SAMPLE_RATE_VALUE_50G "50000"
-#define SFLOW_SAMPLE_RATE_VALUE_40G "40000"
-#define SFLOW_SAMPLE_RATE_VALUE_25G "25000"
-#define SFLOW_SAMPLE_RATE_VALUE_10G "10000"
-#define SFLOW_SAMPLE_RATE_VALUE_1G "1000"
-
#define SFLOW_ERROR_SPEED_STR "error"
struct SflowPortInfo
@@ -55,7 +35,7 @@ class SflowMgr : public Orch
Table m_cfgSflowSessionTable;
ProducerStateTable m_appSflowTable;
ProducerStateTable m_appSflowSessionTable;
- SflowPortConfMap m_sflowPortConfMap;
+ SflowPortConfMap m_sflowPortConfMap;
bool m_intfAllConf;
bool m_gEnable;
@@ -66,7 +46,8 @@ class SflowMgr : public Orch
void sflowHandleSessionLocal(bool enable);
void sflowCheckAndFillValues(std::string alias, std::vector<FieldValueTuple> &values, std::vector<FieldValueTuple> &fvs);
void sflowGetPortInfo(std::vector<FieldValueTuple> &fvs, SflowPortInfo &local_info);
- void sflowGetGlobalInfo(std::vector<FieldValueTuple> &fvs, std::string speed);
+ void sflowGetGlobalInfo(std::vector<FieldValueTuple> &fvs, const std::string& alias);
+ std::string findSamplingRate(const std::string& speed);
};
}
From 57f76699c862cc77537de1e6907f9c8a9b433ead Mon Sep 17 00:00:00 2001
From: Vivek Reddy Karri <vkarri@nvidia.com>
Date: Thu, 25 May 2023 03:17:36 +0000
Subject: [PATCH 3/9] listen to oper speed
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
---
cfgmgr/sflowmgr.cpp | 83 +++++++++++--
cfgmgr/sflowmgr.h | 8 +-
cfgmgr/sflowmgrd.cpp | 25 ++--
tests/mock_tests/Makefile.am | 2 +
tests/mock_tests/sflowmgrd_ut.cpp | 199 ++++++++++++++++++++++++++++++
5 files changed, 297 insertions(+), 20 deletions(-)
create mode 100644 tests/mock_tests/sflowmgrd_ut.cpp
diff --git a/cfgmgr/sflowmgr.cpp b/cfgmgr/sflowmgr.cpp
index d97aee1835..b372135abf 100644
--- a/cfgmgr/sflowmgr.cpp
+++ b/cfgmgr/sflowmgr.cpp
@@ -10,10 +10,8 @@
using namespace std;
using namespace swss;
-SflowMgr::SflowMgr(DBConnector *cfgDb, DBConnector *appDb, const vector<string> &tableNames) :
- Orch(cfgDb, tableNames),
- m_cfgSflowTable(cfgDb, CFG_SFLOW_TABLE_NAME),
- m_cfgSflowSessionTable(cfgDb, CFG_SFLOW_SESSION_TABLE_NAME),
+SflowMgr::SflowMgr(DBConnector *appDb, const std::vector<TableConnector>& tableNames) :
+ Orch(tableNames),
m_appSflowTable(appDb, APP_SFLOW_TABLE_NAME),
m_appSflowSessionTable(appDb, APP_SFLOW_SESSION_TABLE_NAME)
{
@@ -57,7 +55,6 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer)
while (it != consumer.m_toSync.end())
{
KeyOpFieldsValuesTuple t = it->second;
-
string key = kfvKey(t);
string op = kfvOp(t);
auto values = kfvFieldsValues(t);
@@ -73,7 +70,9 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer)
new_port = true;
port_info.local_rate_cfg = false;
port_info.local_admin_cfg = false;
+ port_info.autoneg_enabled = false;
port_info.speed = SFLOW_ERROR_SPEED_STR;
+ port_info.oper_speed = SFLOW_NA_SPEED_STR;
port_info.rate = "";
port_info.admin = "";
m_sflowPortConfMap[key] = port_info;
@@ -87,6 +86,15 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer)
{
new_speed = fvValue(i);
}
+ else if (fvField(i) == "autoneg")
+ {
+ m_sflowPortConfMap[key].autoneg_enabled = fvValue(i) == "on" ? true : false;
+ if (fvValue(i) != "on")
+ {
+ /* If auto-neg is disabled, update sample_rate based on configured speed */
+ speed_change = true;
+ }
+ }
}
if (m_sflowPortConfMap[key].speed != new_speed)
{
@@ -124,6 +132,53 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer)
}
}
+void SflowMgr::sflowProcessOperSpeed(Consumer &consumer)
+{
+ auto it = consumer.m_toSync.begin();
+
+ while (it != consumer.m_toSync.end())
+ {
+ KeyOpFieldsValuesTuple t = it->second;
+ string alias = kfvKey(t);
+ string op = kfvOp(t);
+ auto values = kfvFieldsValues(t);
+ string oper_speed = "";
+ bool iface_found = true;
+
+ for (auto i : values)
+ {
+ if (fvField(i) == "speed")
+ {
+ oper_speed = fvValue(i);
+ }
+ }
+
+ if (m_sflowPortConfMap.find(alias) == m_sflowPortConfMap.end())
+ {
+ SWSS_LOG_NOTICE("Port %s not found in port conf map", alias.c_str());
+ iface_found = false;
+ }
+
+ if (oper_speed != m_sflowPortConfMap[alias].oper_speed &&
+ iface_found && op == SET_COMMAND)
+ {
+ m_sflowPortConfMap[alias].oper_speed = oper_speed;
+ if (!m_sflowPortConfMap[alias].local_rate_cfg && m_gEnable &&
+ m_intfAllConf && m_sflowPortConfMap[alias].autoneg_enabled)
+ {
+ auto rate = findSamplingRate(alias);
+ FieldValueTuple fv("sample_rate", rate);
+ vector<FieldValueTuple> fvs = {fv};
+ m_appSflowSessionTable.set(alias, fvs);
+ SWSS_LOG_INFO("Default sampling rate for %s updated to %s", alias.c_str(), rate.c_str());
+ }
+ }
+ /* Do nothing for DEL as the SflowPortConfMap will already be cleared by the DEL from CONFIG_DB
+ Also don't act on the notifications from the STATE_DB if the speed is not set, happens during init by portsyncd */
+ it = consumer.m_toSync.erase(it);
+ }
+}
+
void SflowMgr::sflowHandleSessionAll(bool enable)
{
for (auto it: m_sflowPortConfMap)
@@ -234,7 +289,6 @@ void SflowMgr::sflowCheckAndFillValues(string alias, vector<FieldValueTuple> &va
if (m_sflowPortConfMap[alias].rate == "" ||
m_sflowPortConfMap[alias].local_rate_cfg)
{
- string speed = m_sflowPortConfMap[alias].speed;
m_sflowPortConfMap[alias].rate = findSamplingRate(alias);
}
m_sflowPortConfMap[alias].local_rate_cfg = false;
@@ -257,13 +311,21 @@ void SflowMgr::sflowCheckAndFillValues(string alias, vector<FieldValueTuple> &va
string SflowMgr::findSamplingRate(const string& alias)
{
- /* Default sampling rate is equal to the speed in Gbps or error */
+ /* Default sampling rate is equal to the oper_speed in Gbps or error
+ if auto_neg is not configured, use the configured speed */
if (m_sflowPortConfMap.find(alias) == m_sflowPortConfMap.end())
{
SWSS_LOG_ERROR("%s not found in port configuration map", alias.c_str());
return SFLOW_ERROR_SPEED_STR;
}
- return m_sflowPortConfMap[alias].speed;
+ string oper_speed = m_sflowPortConfMap[alias].oper_speed;
+ string cfg_speed = m_sflowPortConfMap[alias].speed;
+ bool autoneg = m_sflowPortConfMap[alias].autoneg_enabled;
+ if (autoneg && !oper_speed.empty() && oper_speed != SFLOW_NA_SPEED_STR)
+ {
+ return oper_speed;
+ }
+ return cfg_speed;
}
void SflowMgr::doTask(Consumer &consumer)
@@ -277,6 +339,11 @@ void SflowMgr::doTask(Consumer &consumer)
sflowUpdatePortInfo(consumer);
return;
}
+ else if (table == STATE_PORT_TABLE_NAME)
+ {
+ sflowProcessOperSpeed(consumer);
+ return;
+ }
auto it = consumer.m_toSync.begin();
while (it != consumer.m_toSync.end())
diff --git a/cfgmgr/sflowmgr.h b/cfgmgr/sflowmgr.h
index a1377ccbc7..d36d29533d 100644
--- a/cfgmgr/sflowmgr.h
+++ b/cfgmgr/sflowmgr.h
@@ -11,12 +11,15 @@
namespace swss {
#define SFLOW_ERROR_SPEED_STR "error"
+#define SFLOW_NA_SPEED_STR "N/A"
struct SflowPortInfo
{
bool local_rate_cfg;
bool local_admin_cfg;
+ bool autoneg_enabled;
std::string speed;
+ std::string oper_speed;
std::string rate;
std::string admin;
};
@@ -27,12 +30,10 @@ typedef std::map<std::string, SflowPortInfo> SflowPortConfMap;
class SflowMgr : public Orch
{
public:
- SflowMgr(DBConnector *cfgDb, DBConnector *appDb, const std::vector<std::string> &tableNames);
+ SflowMgr(DBConnector *appDb, const std::vector<TableConnector>& tableNames);
using Orch::doTask;
private:
- Table m_cfgSflowTable;
- Table m_cfgSflowSessionTable;
ProducerStateTable m_appSflowTable;
ProducerStateTable m_appSflowSessionTable;
SflowPortConfMap m_sflowPortConfMap;
@@ -42,6 +43,7 @@ class SflowMgr : public Orch
void doTask(Consumer &consumer);
void sflowHandleService(bool enable);
void sflowUpdatePortInfo(Consumer &consumer);
+ void sflowProcessOperSpeed(Consumer &consumer);
void sflowHandleSessionAll(bool enable);
void sflowHandleSessionLocal(bool enable);
void sflowCheckAndFillValues(std::string alias, std::vector<FieldValueTuple> &values, std::vector<FieldValueTuple> &fvs);
diff --git a/cfgmgr/sflowmgrd.cpp b/cfgmgr/sflowmgrd.cpp
index 7de5f15a2d..39def61811 100644
--- a/cfgmgr/sflowmgrd.cpp
+++ b/cfgmgr/sflowmgrd.cpp
@@ -44,21 +44,28 @@ int main(int argc, char **argv)
try
{
- vector<string> cfg_sflow_tables = {
- CFG_SFLOW_TABLE_NAME,
- CFG_SFLOW_SESSION_TABLE_NAME,
- CFG_PORT_TABLE_NAME
- };
-
DBConnector cfgDb("CONFIG_DB", 0);
DBConnector appDb("APPL_DB", 0);
+ DBConnector stateDb("STATE_DB", 0);
+
+ TableConnector conf_port_table(&cfgDb, CFG_PORT_TABLE_NAME);
+ TableConnector state_port_table(&stateDb, STATE_PORT_TABLE_NAME);
+ TableConnector conf_sflow_table(&cfgDb, CFG_SFLOW_TABLE_NAME);
+ TableConnector conf_sflow_session_table(&cfgDb, CFG_SFLOW_SESSION_TABLE_NAME);
+
+ vector<TableConnector> sflow_tables = {
+ conf_port_table,
+ state_port_table,
+ conf_sflow_table,
+ conf_sflow_session_table
+ };
- SflowMgr sflowmgr(&cfgDb, &appDb, cfg_sflow_tables);
+ SflowMgr sflowmgr(&appDb, sflow_tables);
- vector<Orch *> cfgOrchList = {&sflowmgr};
+ vector<Orch *> orchList = {&sflowmgr};
swss::Select s;
- for (Orch *o : cfgOrchList)
+ for (Orch *o : orchList)
{
s.addSelectables(o->getSelectables());
}
diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am
index 3741e7da27..fd8982eae8 100644
--- a/tests/mock_tests/Makefile.am
+++ b/tests/mock_tests/Makefile.am
@@ -45,6 +45,7 @@ tests_SOURCES = aclorch_ut.cpp \
mock_redisreply.cpp \
bulker_ut.cpp \
portmgr_ut.cpp \
+ sflowmgrd_ut.cpp \
fake_response_publisher.cpp \
swssnet_ut.cpp \
flowcounterrouteorch_ut.cpp \
@@ -109,6 +110,7 @@ tests_SOURCES = aclorch_ut.cpp \
$(top_srcdir)/orchagent/srv6orch.cpp \
$(top_srcdir)/orchagent/nvgreorch.cpp \
$(top_srcdir)/cfgmgr/portmgr.cpp \
+ $(top_srcdir)/cfgmgr/sflowmgr.cpp \
$(top_srcdir)/cfgmgr/buffermgrdyn.cpp \
$(top_srcdir)/warmrestart/warmRestartAssist.cpp
diff --git a/tests/mock_tests/sflowmgrd_ut.cpp b/tests/mock_tests/sflowmgrd_ut.cpp
new file mode 100644
index 0000000000..71c6a31a4f
--- /dev/null
+++ b/tests/mock_tests/sflowmgrd_ut.cpp
@@ -0,0 +1,199 @@
+#include "gtest/gtest.h"
+#include "mock_table.h"
+#include "redisutility.h"
+#define private public
+#include "sflowmgr.h"
+#undef private
+
+namespace sflowmgr_ut
+{
+ using namespace swss;
+ using namespace std;
+
+ struct SflowMgrTest : public ::testing::Test
+ {
+ shared_ptr<swss::DBConnector> m_app_db;
+ shared_ptr<swss::DBConnector> m_config_db;
+ shared_ptr<swss::DBConnector> m_state_db;
+ shared_ptr<SflowMgr> m_sflowMgr;
+ SflowMgrTest()
+ {
+ m_app_db = make_shared<swss::DBConnector>(
+ "APPL_DB", 0);
+ m_config_db = make_shared<swss::DBConnector>(
+ "CONFIG_DB", 0);
+ m_state_db = make_shared<swss::DBConnector>(
+ "STATE_DB", 0);
+ }
+
+ virtual void SetUp() override
+ {
+ ::testing_db::reset();
+ TableConnector conf_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
+ TableConnector state_port_table(m_state_db.get(), STATE_PORT_TABLE_NAME);
+ TableConnector conf_sflow_table(m_config_db.get(), CFG_SFLOW_TABLE_NAME);
+ TableConnector conf_sflow_session_table(m_config_db.get(), CFG_SFLOW_SESSION_TABLE_NAME);
+
+ vector<TableConnector> sflow_tables = {
+ conf_port_table,
+ state_port_table,
+ conf_sflow_table,
+ conf_sflow_session_table
+ };
+ m_sflowMgr.reset(new SflowMgr(m_app_db.get(), sflow_tables));
+ m_sflowMgr->m_gEnable = true;
+ }
+ };
+
+ TEST_F(SflowMgrTest, test_RateConfiguration)
+ {
+ Table state_port_table(m_state_db.get(), STATE_PORT_TABLE_NAME);
+ Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
+ Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
+
+ cfg_port_table.set("Ethernet0", {
+ {"speed", "100000"},
+ {"autoneg", "on"}
+ });
+
+ m_sflowMgr->addExistingData(&cfg_port_table);
+ m_sflowMgr->doTask();
+
+ std::vector<FieldValueTuple> values;
+ appl_sflow_table.get("Ethernet0", values);
+ auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate.get() == "100000");
+
+ /* Scenario: Operational Speed Changes to 25000 */
+ state_port_table.set("Ethernet0", {
+ {"speed", "25000"}
+ });
+
+ m_sflowMgr->addExistingData(&state_port_table);
+ m_sflowMgr->doTask();
+
+ values.clear();
+ appl_sflow_table.get("Ethernet0", values);
+ value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate.get() == "25000");
+ }
+
+ TEST_F(SflowMgrTest, test_OnlyStateDbNotif)
+ {
+ Table state_port_table(m_state_db.get(), STATE_PORT_TABLE_NAME);
+ Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
+ Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
+
+ state_port_table.set("Ethernet0", {
+ {"speed", "100000"}
+ });
+
+ m_sflowMgr->addExistingData(&cfg_port_table);
+ m_sflowMgr->doTask();
+
+ std::vector<FieldValueTuple> values;
+ appl_sflow_table.get("Ethernet0", values);
+ auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_FALSE(value_rate);
+ }
+
+ TEST_F(SflowMgrTest, tets_LocalRateConfiguration)
+ {
+ Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
+ Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
+ Table cfg_sflow_table(m_config_db.get(), CFG_SFLOW_SESSION_TABLE_NAME);
+
+ cfg_port_table.set("Ethernet0", {
+ {"speed", "100000"}
+ });
+
+ m_sflowMgr->addExistingData(&cfg_port_table);
+ m_sflowMgr->doTask();
+
+ std::vector<FieldValueTuple> values;
+ appl_sflow_table.get("Ethernet0", values);
+ auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate.get() == "100000");
+
+ cfg_sflow_table.set("Ethernet0", {
+ {"sample_rate", "12345"}
+ });
+
+ m_sflowMgr->addExistingData(&cfg_sflow_table);
+ m_sflowMgr->doTask();
+
+ appl_sflow_table.get("Ethernet0", values);
+ value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate.get() == "12345");
+ }
+
+ TEST_F(SflowMgrTest, tets_LocalRateConfigurationWithOperSpeed)
+ {
+ Table state_port_table(m_state_db.get(), STATE_PORT_TABLE_NAME);
+ Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
+ Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
+ Table cfg_sflow_table(m_config_db.get(), CFG_SFLOW_SESSION_TABLE_NAME);
+
+ cfg_port_table.set("Ethernet0", {
+ {"speed", "100000"},
+ {"autoneg", "on"}
+ });
+
+ /* Scenario: Operational Speed Changes to 25000 */
+ state_port_table.set("Ethernet0", {
+ {"speed", "25000"}
+ });
+
+ m_sflowMgr->addExistingData(&cfg_port_table);
+ m_sflowMgr->addExistingData(&state_port_table);
+ m_sflowMgr->doTask();
+
+ std::vector<FieldValueTuple> values;
+ appl_sflow_table.get("Ethernet0", values);
+ auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate.get() == "25000");
+
+ cfg_sflow_table.set("Ethernet0", {
+ {"sample_rate", "12345"}
+ });
+
+ m_sflowMgr->addExistingData(&cfg_sflow_table);
+ m_sflowMgr->doTask();
+
+ appl_sflow_table.get("Ethernet0", values);
+ value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate.get() == "12345");
+
+ /* Operational Speed Changes again to 50000 */
+ state_port_table.set("Ethernet0", {
+ {"speed", "50000"}
+ });
+
+ m_sflowMgr->addExistingData(&state_port_table);
+ m_sflowMgr->doTask();
+
+ appl_sflow_table.get("Ethernet0", values);
+ value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ /* Local config wouldn't change */
+ ASSERT_TRUE(value_rate.get() == "12345");
+ }
+
+ TEST_F(SflowMgrTest, tets_800g)
+ {
+ Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
+ Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
+ Table cfg_sflow_table(m_config_db.get(), CFG_SFLOW_SESSION_TABLE_NAME);
+
+ cfg_port_table.set("Ethernet0", {
+ {"speed", "800000"}
+ });
+
+ m_sflowMgr->addExistingData(&cfg_port_table);
+ m_sflowMgr->doTask();
+
+ std::vector<FieldValueTuple> values;
+ appl_sflow_table.get("Ethernet0", values);
+ auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate.get() == "800000");
+ }
+}
From 431d3f8f4779bc66697728134219099dfd35cefa Mon Sep 17 00:00:00 2001
From: Vivek Reddy Karri <vkarri@nvidia.com>
Date: Thu, 25 May 2023 21:21:27 +0000
Subject: [PATCH 4/9] handle auto-neg disabled flow
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
---
cfgmgr/sflowmgr.cpp | 14 +++++---------
tests/mock_tests/sflowmgrd_ut.cpp | 14 ++++++++++++++
2 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/cfgmgr/sflowmgr.cpp b/cfgmgr/sflowmgr.cpp
index b372135abf..1fee8bcc12 100644
--- a/cfgmgr/sflowmgr.cpp
+++ b/cfgmgr/sflowmgr.cpp
@@ -78,7 +78,7 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer)
m_sflowPortConfMap[key] = port_info;
}
- bool speed_change = false;
+ bool rate_update = false;
string new_speed = SFLOW_ERROR_SPEED_STR;
for (auto i : values)
{
@@ -89,23 +89,20 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer)
else if (fvField(i) == "autoneg")
{
m_sflowPortConfMap[key].autoneg_enabled = fvValue(i) == "on" ? true : false;
- if (fvValue(i) != "on")
- {
- /* If auto-neg is disabled, update sample_rate based on configured speed */
- speed_change = true;
- }
+ /* If autoneg is diabled, should set the rate back to configured speed */
+ rate_update = !m_sflowPortConfMap[key].autoneg_enabled;
}
}
if (m_sflowPortConfMap[key].speed != new_speed)
{
m_sflowPortConfMap[key].speed = new_speed;
- speed_change = true;
+ rate_update = true;
}
if (m_gEnable && m_intfAllConf)
{
// If the Local rate Conf is already present, dont't override it even though the speed is changed
- if (new_port || (speed_change && !m_sflowPortConfMap[key].local_rate_cfg))
+ if (new_port || (rate_update && !m_sflowPortConfMap[key].local_rate_cfg))
{
vector<FieldValueTuple> fvs;
sflowGetGlobalInfo(fvs, key);
@@ -230,7 +227,6 @@ void SflowMgr::sflowHandleSessionLocal(bool enable)
void SflowMgr::sflowGetGlobalInfo(vector<FieldValueTuple> &fvs, const string& alias)
{
- string rate;
FieldValueTuple fv1("admin_state", "up");
fvs.push_back(fv1);
diff --git a/tests/mock_tests/sflowmgrd_ut.cpp b/tests/mock_tests/sflowmgrd_ut.cpp
index 71c6a31a4f..f38aa6b69d 100644
--- a/tests/mock_tests/sflowmgrd_ut.cpp
+++ b/tests/mock_tests/sflowmgrd_ut.cpp
@@ -76,6 +76,20 @@ namespace sflowmgr_ut
appl_sflow_table.get("Ethernet0", values);
value_rate = swss::fvsGetValue(values, "sample_rate", true);
ASSERT_TRUE(value_rate.get() == "25000");
+
+ cfg_port_table.set("Ethernet0", {
+ {"speed", "100000"},
+ {"autoneg", "off"}
+ });
+
+ m_sflowMgr->addExistingData(&cfg_port_table);
+ m_sflowMgr->doTask();
+
+ /* Sample rate should be back to configured speed if auto-neg is disabled */
+ values.clear();
+ appl_sflow_table.get("Ethernet0", values);
+ value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate.get() == "100000");
}
TEST_F(SflowMgrTest, test_OnlyStateDbNotif)
From 8f439d7f19f33770320d856616394ee9eab5d792 Mon Sep 17 00:00:00 2001
From: Vivek Reddy Karri <vkarri@nvidia.com>
Date: Thu, 25 May 2023 21:42:19 +0000
Subject: [PATCH 5/9] cleanup tests
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
---
tests/mock_tests/sflowmgrd_ut.cpp | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/tests/mock_tests/sflowmgrd_ut.cpp b/tests/mock_tests/sflowmgrd_ut.cpp
index f38aa6b69d..25dbf972cf 100644
--- a/tests/mock_tests/sflowmgrd_ut.cpp
+++ b/tests/mock_tests/sflowmgrd_ut.cpp
@@ -1,9 +1,7 @@
#include "gtest/gtest.h"
#include "mock_table.h"
#include "redisutility.h"
-#define private public
#include "sflowmgr.h"
-#undef private
namespace sflowmgr_ut
{
@@ -41,7 +39,17 @@ namespace sflowmgr_ut
conf_sflow_session_table
};
m_sflowMgr.reset(new SflowMgr(m_app_db.get(), sflow_tables));
- m_sflowMgr->m_gEnable = true;
+ enableSflow();
+ }
+
+ void enableSflow()
+ {
+ Table cfg_sflow(m_config_db.get(), CFG_SFLOW_TABLE_NAME);
+ cfg_sflow.set("global", {
+ {"admin_state", "up"}
+ });
+ m_sflowMgr->addExistingData(&cfg_sflow);
+ m_sflowMgr->doTask();
}
};
@@ -62,6 +70,7 @@ namespace sflowmgr_ut
std::vector<FieldValueTuple> values;
appl_sflow_table.get("Ethernet0", values);
auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate);
ASSERT_TRUE(value_rate.get() == "100000");
/* Scenario: Operational Speed Changes to 25000 */
@@ -75,6 +84,7 @@ namespace sflowmgr_ut
values.clear();
appl_sflow_table.get("Ethernet0", values);
value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate);
ASSERT_TRUE(value_rate.get() == "25000");
cfg_port_table.set("Ethernet0", {
@@ -89,6 +99,7 @@ namespace sflowmgr_ut
values.clear();
appl_sflow_table.get("Ethernet0", values);
value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate);
ASSERT_TRUE(value_rate.get() == "100000");
}
@@ -111,7 +122,7 @@ namespace sflowmgr_ut
ASSERT_FALSE(value_rate);
}
- TEST_F(SflowMgrTest, tets_LocalRateConfiguration)
+ TEST_F(SflowMgrTest, test_LocalRateConfiguration)
{
Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
@@ -127,6 +138,7 @@ namespace sflowmgr_ut
std::vector<FieldValueTuple> values;
appl_sflow_table.get("Ethernet0", values);
auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate);
ASSERT_TRUE(value_rate.get() == "100000");
cfg_sflow_table.set("Ethernet0", {
@@ -138,10 +150,11 @@ namespace sflowmgr_ut
appl_sflow_table.get("Ethernet0", values);
value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate);
ASSERT_TRUE(value_rate.get() == "12345");
}
- TEST_F(SflowMgrTest, tets_LocalRateConfigurationWithOperSpeed)
+ TEST_F(SflowMgrTest, test_LocalRateConfigurationWithOperSpeed)
{
Table state_port_table(m_state_db.get(), STATE_PORT_TABLE_NAME);
Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
@@ -165,6 +178,7 @@ namespace sflowmgr_ut
std::vector<FieldValueTuple> values;
appl_sflow_table.get("Ethernet0", values);
auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate);
ASSERT_TRUE(value_rate.get() == "25000");
cfg_sflow_table.set("Ethernet0", {
@@ -176,6 +190,7 @@ namespace sflowmgr_ut
appl_sflow_table.get("Ethernet0", values);
value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate);
ASSERT_TRUE(value_rate.get() == "12345");
/* Operational Speed Changes again to 50000 */
@@ -189,10 +204,11 @@ namespace sflowmgr_ut
appl_sflow_table.get("Ethernet0", values);
value_rate = swss::fvsGetValue(values, "sample_rate", true);
/* Local config wouldn't change */
+ ASSERT_TRUE(value_rate);
ASSERT_TRUE(value_rate.get() == "12345");
}
- TEST_F(SflowMgrTest, tets_800g)
+ TEST_F(SflowMgrTest, test_800g)
{
Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
@@ -208,6 +224,7 @@ namespace sflowmgr_ut
std::vector<FieldValueTuple> values;
appl_sflow_table.get("Ethernet0", values);
auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate);
ASSERT_TRUE(value_rate.get() == "800000");
}
}
From 020747af4f10913d257bbdf468d83073140ba6bd Mon Sep 17 00:00:00 2001
From: Vivek Reddy Karri <vkarri@nvidia.com>
Date: Fri, 26 May 2023 18:48:39 +0000
Subject: [PATCH 6/9] Remove autoneg parameter
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
---
cfgmgr/sflowmgr.cpp | 60 +++++++++-------
cfgmgr/sflowmgr.h | 2 +-
cfgmgr/sflowmgrd.cpp | 2 +
tests/mock_tests/sflowmgrd_ut.cpp | 111 +++++++++++++++++++++---------
4 files changed, 120 insertions(+), 55 deletions(-)
diff --git a/cfgmgr/sflowmgr.cpp b/cfgmgr/sflowmgr.cpp
index 1fee8bcc12..fecff136ed 100644
--- a/cfgmgr/sflowmgr.cpp
+++ b/cfgmgr/sflowmgr.cpp
@@ -19,6 +19,20 @@ SflowMgr::SflowMgr(DBConnector *appDb, const std::vector<TableConnector>& tableN
m_gEnable = false;
}
+void SflowMgr::readPortConfig()
+{
+ auto consumer_it = m_consumerMap.find(CFG_PORT_TABLE_NAME);
+ if (consumer_it != m_consumerMap.end())
+ {
+ consumer_it->second->drain();
+ SWSS_LOG_INFO("Port Configuration Read..");
+ }
+ else
+ {
+ throw std::runtime_error("Consumer for config db PORT_TABLE not found");
+ }
+}
+
void SflowMgr::sflowHandleService(bool enable)
{
stringstream cmd;
@@ -70,7 +84,6 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer)
new_port = true;
port_info.local_rate_cfg = false;
port_info.local_admin_cfg = false;
- port_info.autoneg_enabled = false;
port_info.speed = SFLOW_ERROR_SPEED_STR;
port_info.oper_speed = SFLOW_NA_SPEED_STR;
port_info.rate = "";
@@ -86,12 +99,6 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer)
{
new_speed = fvValue(i);
}
- else if (fvField(i) == "autoneg")
- {
- m_sflowPortConfMap[key].autoneg_enabled = fvValue(i) == "on" ? true : false;
- /* If autoneg is diabled, should set the rate back to configured speed */
- rate_update = !m_sflowPortConfMap[key].autoneg_enabled;
- }
}
if (m_sflowPortConfMap[key].speed != new_speed)
{
@@ -139,29 +146,38 @@ void SflowMgr::sflowProcessOperSpeed(Consumer &consumer)
string alias = kfvKey(t);
string op = kfvOp(t);
auto values = kfvFieldsValues(t);
- string oper_speed = "";
- bool iface_found = true;
-
+ string oper_speed = SFLOW_NA_SPEED_STR;
+ bool oper_status = false;
+
for (auto i : values)
{
if (fvField(i) == "speed")
{
oper_speed = fvValue(i);
}
+ else if (fvField(i) == "netdev_oper_status")
+ {
+ oper_status = fvValue(i) == "up" ? true : false;
+ }
}
+ /* When the oper_status is down, the speed (if present) is just a stale entry and hence discard */
+ oper_speed = oper_status ? oper_speed : SFLOW_NA_SPEED_STR;
+
if (m_sflowPortConfMap.find(alias) == m_sflowPortConfMap.end())
{
- SWSS_LOG_NOTICE("Port %s not found in port conf map", alias.c_str());
- iface_found = false;
+ SWSS_LOG_ERROR("Port %s not found in port conf map", alias.c_str());
}
-
- if (oper_speed != m_sflowPortConfMap[alias].oper_speed &&
- iface_found && op == SET_COMMAND)
+ else
{
- m_sflowPortConfMap[alias].oper_speed = oper_speed;
- if (!m_sflowPortConfMap[alias].local_rate_cfg && m_gEnable &&
- m_intfAllConf && m_sflowPortConfMap[alias].autoneg_enabled)
+ bool speed_change = false;
+ if (m_sflowPortConfMap[alias].oper_speed != oper_speed)
+ {
+ speed_change = true;
+ m_sflowPortConfMap[alias].oper_speed = oper_speed;
+ }
+ if (speed_change && m_gEnable && m_intfAllConf &&
+ !m_sflowPortConfMap[alias].local_rate_cfg)
{
auto rate = findSamplingRate(alias);
FieldValueTuple fv("sample_rate", rate);
@@ -170,8 +186,7 @@ void SflowMgr::sflowProcessOperSpeed(Consumer &consumer)
SWSS_LOG_INFO("Default sampling rate for %s updated to %s", alias.c_str(), rate.c_str());
}
}
- /* Do nothing for DEL as the SflowPortConfMap will already be cleared by the DEL from CONFIG_DB
- Also don't act on the notifications from the STATE_DB if the speed is not set, happens during init by portsyncd */
+ /* Do nothing for DEL as the SflowPortConfMap will already be cleared by the DEL from CONFIG_DB */
it = consumer.m_toSync.erase(it);
}
}
@@ -308,7 +323,7 @@ void SflowMgr::sflowCheckAndFillValues(string alias, vector<FieldValueTuple> &va
string SflowMgr::findSamplingRate(const string& alias)
{
/* Default sampling rate is equal to the oper_speed in Gbps or error
- if auto_neg is not configured, use the configured speed */
+ if oper_speed is not found, use the configured speed */
if (m_sflowPortConfMap.find(alias) == m_sflowPortConfMap.end())
{
SWSS_LOG_ERROR("%s not found in port configuration map", alias.c_str());
@@ -316,8 +331,7 @@ string SflowMgr::findSamplingRate(const string& alias)
}
string oper_speed = m_sflowPortConfMap[alias].oper_speed;
string cfg_speed = m_sflowPortConfMap[alias].speed;
- bool autoneg = m_sflowPortConfMap[alias].autoneg_enabled;
- if (autoneg && !oper_speed.empty() && oper_speed != SFLOW_NA_SPEED_STR)
+ if (!oper_speed.empty() && oper_speed != SFLOW_NA_SPEED_STR)
{
return oper_speed;
}
diff --git a/cfgmgr/sflowmgr.h b/cfgmgr/sflowmgr.h
index d36d29533d..51dae92d7b 100644
--- a/cfgmgr/sflowmgr.h
+++ b/cfgmgr/sflowmgr.h
@@ -17,7 +17,6 @@ struct SflowPortInfo
{
bool local_rate_cfg;
bool local_admin_cfg;
- bool autoneg_enabled;
std::string speed;
std::string oper_speed;
std::string rate;
@@ -31,6 +30,7 @@ class SflowMgr : public Orch
{
public:
SflowMgr(DBConnector *appDb, const std::vector<TableConnector>& tableNames);
+ void readPortConfig();
using Orch::doTask;
private:
diff --git a/cfgmgr/sflowmgrd.cpp b/cfgmgr/sflowmgrd.cpp
index 39def61811..83fb134c82 100644
--- a/cfgmgr/sflowmgrd.cpp
+++ b/cfgmgr/sflowmgrd.cpp
@@ -62,6 +62,8 @@ int main(int argc, char **argv)
SflowMgr sflowmgr(&appDb, sflow_tables);
+ sflowmgr.readPortConfig();
+
vector<Orch *> orchList = {&sflowmgr};
swss::Select s;
diff --git a/tests/mock_tests/sflowmgrd_ut.cpp b/tests/mock_tests/sflowmgrd_ut.cpp
index 25dbf972cf..5c3e291e4a 100644
--- a/tests/mock_tests/sflowmgrd_ut.cpp
+++ b/tests/mock_tests/sflowmgrd_ut.cpp
@@ -61,7 +61,6 @@ namespace sflowmgr_ut
cfg_port_table.set("Ethernet0", {
{"speed", "100000"},
- {"autoneg", "on"}
});
m_sflowMgr->addExistingData(&cfg_port_table);
@@ -75,7 +74,8 @@ namespace sflowmgr_ut
/* Scenario: Operational Speed Changes to 25000 */
state_port_table.set("Ethernet0", {
- {"speed", "25000"}
+ {"speed", "25000"},
+ {"netdev_oper_status", "up"}
});
m_sflowMgr->addExistingData(&state_port_table);
@@ -87,20 +87,82 @@ namespace sflowmgr_ut
ASSERT_TRUE(value_rate);
ASSERT_TRUE(value_rate.get() == "25000");
+ /* oper status is down since rate_neg is disabled */
+ state_port_table.set("Ethernet0", {
+ {"speed", "25000"},
+ {"netdev_oper_status", "down"}
+ });
+
+ m_sflowMgr->addExistingData(&state_port_table);
+ m_sflowMgr->doTask();
+
+ values.clear();
+ appl_sflow_table.get("Ethernet0", values);
+ value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate);
+ /* Sample rate is restored to original value */
+ ASSERT_TRUE(value_rate.get() == "100000");
+ }
+
+ TEST_F(SflowMgrTest, test_RateConfigurationOperUpDown)
+ {
+ /* Simulates a realistic scenario of oper up down events mixed with auto neg */
+ Table state_port_table(m_state_db.get(), STATE_PORT_TABLE_NAME);
+ Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
+ Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
+ std::vector<FieldValueTuple> values;
+
+ /* Configure the Speed to 100G */
cfg_port_table.set("Ethernet0", {
{"speed", "100000"},
- {"autoneg", "off"}
+ });
+
+ /* Scenario: Operational Speed Changes to 100G, no autoneg */
+ state_port_table.set("Ethernet0", {
+ {"speed", "100000"},
+ {"netdev_oper_status", "up"}
});
+ m_sflowMgr->addExistingData(&cfg_port_table);
+ m_sflowMgr->addExistingData(&state_port_table);
+ m_sflowMgr->doTask();
+
+ /* Scenario: Operational Speed Changes to 25G, with autoneg */
+ state_port_table.set("Ethernet0", {
+ {"speed", "25000"},
+ {"netdev_oper_status", "up"}
+ });
+
+ m_sflowMgr->addExistingData(&state_port_table);
+ m_sflowMgr->doTask();
+
+ values.clear();
+ appl_sflow_table.get("Ethernet0", values);
+ auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate);
+ /* Rate is supposed to be updated */
+ ASSERT_TRUE(value_rate.get() == "25000");
+
+ /* rate_neg is disabled and oper status happen to get disabled */
+ state_port_table.set("Ethernet0", {
+ {"speed", "25000"},
+ {"netdev_oper_status", "down"}
+ });
+
+ /* Configured speed is updated by user */
+ cfg_port_table.set("Ethernet0", {
+ {"speed", "200000"},
+ });
+
+ m_sflowMgr->addExistingData(&state_port_table);
m_sflowMgr->addExistingData(&cfg_port_table);
m_sflowMgr->doTask();
- /* Sample rate should be back to configured speed if auto-neg is disabled */
values.clear();
appl_sflow_table.get("Ethernet0", values);
value_rate = swss::fvsGetValue(values, "sample_rate", true);
ASSERT_TRUE(value_rate);
- ASSERT_TRUE(value_rate.get() == "100000");
+ ASSERT_TRUE(value_rate.get() == "200000");
}
TEST_F(SflowMgrTest, test_OnlyStateDbNotif)
@@ -132,29 +194,22 @@ namespace sflowmgr_ut
{"speed", "100000"}
});
- m_sflowMgr->addExistingData(&cfg_port_table);
- m_sflowMgr->doTask();
-
- std::vector<FieldValueTuple> values;
- appl_sflow_table.get("Ethernet0", values);
- auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
- ASSERT_TRUE(value_rate);
- ASSERT_TRUE(value_rate.get() == "100000");
-
cfg_sflow_table.set("Ethernet0", {
{"sample_rate", "12345"}
});
+ m_sflowMgr->addExistingData(&cfg_port_table);
m_sflowMgr->addExistingData(&cfg_sflow_table);
m_sflowMgr->doTask();
+ std::vector<FieldValueTuple> values;
appl_sflow_table.get("Ethernet0", values);
- value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
ASSERT_TRUE(value_rate);
ASSERT_TRUE(value_rate.get() == "12345");
}
- TEST_F(SflowMgrTest, test_LocalRateConfigurationWithOperSpeed)
+ TEST_F(SflowMgrTest, test_LocalRateConfWithOperSpeed)
{
Table state_port_table(m_state_db.get(), STATE_PORT_TABLE_NAME);
Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
@@ -163,39 +218,33 @@ namespace sflowmgr_ut
cfg_port_table.set("Ethernet0", {
{"speed", "100000"},
- {"autoneg", "on"}
});
/* Scenario: Operational Speed Changes to 25000 */
state_port_table.set("Ethernet0", {
- {"speed", "25000"}
+ {"speed", "25000"},
+ {"netdev_oper_status", "up"}
});
- m_sflowMgr->addExistingData(&cfg_port_table);
- m_sflowMgr->addExistingData(&state_port_table);
- m_sflowMgr->doTask();
-
- std::vector<FieldValueTuple> values;
- appl_sflow_table.get("Ethernet0", values);
- auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
- ASSERT_TRUE(value_rate);
- ASSERT_TRUE(value_rate.get() == "25000");
-
cfg_sflow_table.set("Ethernet0", {
{"sample_rate", "12345"}
});
+ m_sflowMgr->addExistingData(&cfg_port_table);
m_sflowMgr->addExistingData(&cfg_sflow_table);
+ m_sflowMgr->addExistingData(&state_port_table);
m_sflowMgr->doTask();
+ std::vector<FieldValueTuple> values;
appl_sflow_table.get("Ethernet0", values);
- value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
ASSERT_TRUE(value_rate);
ASSERT_TRUE(value_rate.get() == "12345");
/* Operational Speed Changes again to 50000 */
state_port_table.set("Ethernet0", {
- {"speed", "50000"}
+ {"speed", "50000"},
+ {"netdev_oper_status", "up"}
});
m_sflowMgr->addExistingData(&state_port_table);
@@ -208,7 +257,7 @@ namespace sflowmgr_ut
ASSERT_TRUE(value_rate.get() == "12345");
}
- TEST_F(SflowMgrTest, test_800g)
+ TEST_F(SflowMgrTest, test_new_speed)
{
Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
From 2fe2160b1342ce4c61f1b5683a29fbe0826dc60b Mon Sep 17 00:00:00 2001
From: Vivek Reddy Karri <vkarri@nvidia.com>
Date: Sat, 27 May 2023 00:42:40 +0000
Subject: [PATCH 7/9] Handle comments
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
---
cfgmgr/sflowmgr.cpp | 53 ++++++++++++++--------------
cfgmgr/sflowmgr.h | 4 +--
cfgmgr/sflowmgrd.cpp | 3 +-
tests/mock_tests/sflowmgrd_ut.cpp | 58 +++++++++++++------------------
4 files changed, 54 insertions(+), 64 deletions(-)
diff --git a/cfgmgr/sflowmgr.cpp b/cfgmgr/sflowmgr.cpp
index fecff136ed..88a37f53e5 100644
--- a/cfgmgr/sflowmgr.cpp
+++ b/cfgmgr/sflowmgr.cpp
@@ -25,11 +25,11 @@ void SflowMgr::readPortConfig()
if (consumer_it != m_consumerMap.end())
{
consumer_it->second->drain();
- SWSS_LOG_INFO("Port Configuration Read..");
+ SWSS_LOG_NOTICE("Port Configuration Read..");
}
else
{
- throw std::runtime_error("Consumer for config db PORT_TABLE not found");
+ SWSS_LOG_ERROR("Consumer object for PORT_TABLE not found");
}
}
@@ -84,15 +84,15 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer)
new_port = true;
port_info.local_rate_cfg = false;
port_info.local_admin_cfg = false;
- port_info.speed = SFLOW_ERROR_SPEED_STR;
- port_info.oper_speed = SFLOW_NA_SPEED_STR;
+ port_info.speed = ERROR_SPEED;
+ port_info.oper_speed = NA_SPEED;
port_info.rate = "";
port_info.admin = "";
m_sflowPortConfMap[key] = port_info;
}
bool rate_update = false;
- string new_speed = SFLOW_ERROR_SPEED_STR;
+ string new_speed = ERROR_SPEED;
for (auto i : values)
{
if (fvField(i) == "speed")
@@ -146,8 +146,8 @@ void SflowMgr::sflowProcessOperSpeed(Consumer &consumer)
string alias = kfvKey(t);
string op = kfvOp(t);
auto values = kfvFieldsValues(t);
- string oper_speed = SFLOW_NA_SPEED_STR;
- bool oper_status = false;
+ string oper_speed = "";
+ bool rate_update = false;
for (auto i : values)
{
@@ -155,35 +155,34 @@ void SflowMgr::sflowProcessOperSpeed(Consumer &consumer)
{
oper_speed = fvValue(i);
}
- else if (fvField(i) == "netdev_oper_status")
- {
- oper_status = fvValue(i) == "up" ? true : false;
- }
}
- /* When the oper_status is down, the speed (if present) is just a stale entry and hence discard */
- oper_speed = oper_status ? oper_speed : SFLOW_NA_SPEED_STR;
-
- if (m_sflowPortConfMap.find(alias) == m_sflowPortConfMap.end())
- {
- SWSS_LOG_ERROR("Port %s not found in port conf map", alias.c_str());
- }
- else
+ if (m_sflowPortConfMap.find(alias) != m_sflowPortConfMap.end() && op == SET_COMMAND)
{
- bool speed_change = false;
- if (m_sflowPortConfMap[alias].oper_speed != oper_speed)
+ SWSS_LOG_DEBUG("STATE_DB update: iface: %s, oper_speed: %s, cfg_speed: %s, new_speed: %s",
+ alias.c_str(), m_sflowPortConfMap[alias].oper_speed.c_str(),
+ m_sflowPortConfMap[alias].speed.c_str(),
+ oper_speed.c_str());
+ /* oper_speed is updated by orchagent if the vendor supports and oper status is up */
+ if (m_sflowPortConfMap[alias].oper_speed != oper_speed && !oper_speed.empty())
{
- speed_change = true;
+ rate_update = true;
+ if (oper_speed == m_sflowPortConfMap[alias].speed && m_sflowPortConfMap[alias].oper_speed == NA_SPEED)
+ {
+ /* if oper_speed is equal to cfg_speed, avoid the write to APP_DB
+ Can happen if auto-neg is not set */
+ rate_update = false;
+ }
m_sflowPortConfMap[alias].oper_speed = oper_speed;
}
- if (speed_change && m_gEnable && m_intfAllConf &&
- !m_sflowPortConfMap[alias].local_rate_cfg)
+
+ if (rate_update && m_gEnable && m_intfAllConf && !m_sflowPortConfMap[alias].local_rate_cfg )
{
auto rate = findSamplingRate(alias);
FieldValueTuple fv("sample_rate", rate);
vector<FieldValueTuple> fvs = {fv};
m_appSflowSessionTable.set(alias, fvs);
- SWSS_LOG_INFO("Default sampling rate for %s updated to %s", alias.c_str(), rate.c_str());
+ SWSS_LOG_NOTICE("Default sampling rate for %s updated to %s", alias.c_str(), rate.c_str());
}
}
/* Do nothing for DEL as the SflowPortConfMap will already be cleared by the DEL from CONFIG_DB */
@@ -327,11 +326,11 @@ string SflowMgr::findSamplingRate(const string& alias)
if (m_sflowPortConfMap.find(alias) == m_sflowPortConfMap.end())
{
SWSS_LOG_ERROR("%s not found in port configuration map", alias.c_str());
- return SFLOW_ERROR_SPEED_STR;
+ return ERROR_SPEED;
}
string oper_speed = m_sflowPortConfMap[alias].oper_speed;
string cfg_speed = m_sflowPortConfMap[alias].speed;
- if (!oper_speed.empty() && oper_speed != SFLOW_NA_SPEED_STR)
+ if (!oper_speed.empty() && oper_speed != NA_SPEED)
{
return oper_speed;
}
diff --git a/cfgmgr/sflowmgr.h b/cfgmgr/sflowmgr.h
index 51dae92d7b..c1c1f92b60 100644
--- a/cfgmgr/sflowmgr.h
+++ b/cfgmgr/sflowmgr.h
@@ -10,8 +10,8 @@
namespace swss {
-#define SFLOW_ERROR_SPEED_STR "error"
-#define SFLOW_NA_SPEED_STR "N/A"
+#define ERROR_SPEED "error"
+#define NA_SPEED "N/A"
struct SflowPortInfo
{
diff --git a/cfgmgr/sflowmgrd.cpp b/cfgmgr/sflowmgrd.cpp
index 83fb134c82..ebe489dfdc 100644
--- a/cfgmgr/sflowmgrd.cpp
+++ b/cfgmgr/sflowmgrd.cpp
@@ -61,7 +61,8 @@ int main(int argc, char **argv)
};
SflowMgr sflowmgr(&appDb, sflow_tables);
-
+ /* During process startup, the ordering of cfg_db followed by state_db notifications cannot be guaranteed
+ and so handle the cfg notifs manually */
sflowmgr.readPortConfig();
vector<Orch *> orchList = {&sflowmgr};
diff --git a/tests/mock_tests/sflowmgrd_ut.cpp b/tests/mock_tests/sflowmgrd_ut.cpp
index 5c3e291e4a..e07cf04359 100644
--- a/tests/mock_tests/sflowmgrd_ut.cpp
+++ b/tests/mock_tests/sflowmgrd_ut.cpp
@@ -74,8 +74,7 @@ namespace sflowmgr_ut
/* Scenario: Operational Speed Changes to 25000 */
state_port_table.set("Ethernet0", {
- {"speed", "25000"},
- {"netdev_oper_status", "up"}
+ {"speed", "25000"}
});
m_sflowMgr->addExistingData(&state_port_table);
@@ -86,22 +85,6 @@ namespace sflowmgr_ut
value_rate = swss::fvsGetValue(values, "sample_rate", true);
ASSERT_TRUE(value_rate);
ASSERT_TRUE(value_rate.get() == "25000");
-
- /* oper status is down since rate_neg is disabled */
- state_port_table.set("Ethernet0", {
- {"speed", "25000"},
- {"netdev_oper_status", "down"}
- });
-
- m_sflowMgr->addExistingData(&state_port_table);
- m_sflowMgr->doTask();
-
- values.clear();
- appl_sflow_table.get("Ethernet0", values);
- value_rate = swss::fvsGetValue(values, "sample_rate", true);
- ASSERT_TRUE(value_rate);
- /* Sample rate is restored to original value */
- ASSERT_TRUE(value_rate.get() == "100000");
}
TEST_F(SflowMgrTest, test_RateConfigurationOperUpDown)
@@ -117,38 +100,45 @@ namespace sflowmgr_ut
{"speed", "100000"},
});
- /* Scenario: Operational Speed Changes to 100G, no autoneg */
+ /* Scenario: Operational Speed Changes to 100G with autoneg */
state_port_table.set("Ethernet0", {
- {"speed", "100000"},
- {"netdev_oper_status", "up"}
+ {"speed", "100000"}
});
m_sflowMgr->addExistingData(&cfg_port_table);
m_sflowMgr->addExistingData(&state_port_table);
m_sflowMgr->doTask();
- /* Scenario: Operational Speed Changes to 25G, with autoneg */
- state_port_table.set("Ethernet0", {
- {"speed", "25000"},
- {"netdev_oper_status", "up"}
+ /* User changes the config speed to 10G */
+ cfg_port_table.set("Ethernet0", {
+ {"speed", "10000"},
});
- m_sflowMgr->addExistingData(&state_port_table);
+ m_sflowMgr->addExistingData(&cfg_port_table);
m_sflowMgr->doTask();
values.clear();
appl_sflow_table.get("Ethernet0", values);
auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
ASSERT_TRUE(value_rate);
- /* Rate is supposed to be updated */
- ASSERT_TRUE(value_rate.get() == "25000");
+ /* Rate Should still adhere to oper_speed */
+ ASSERT_TRUE(value_rate.get() == "100000");
- /* rate_neg is disabled and oper status happen to get disabled */
+ /* Scenario: Operational Speed Changes to 10G, with autoneg */
state_port_table.set("Ethernet0", {
- {"speed", "25000"},
- {"netdev_oper_status", "down"}
+ {"speed", "10000"}
});
+ m_sflowMgr->addExistingData(&state_port_table);
+ m_sflowMgr->doTask();
+
+ values.clear();
+ appl_sflow_table.get("Ethernet0", values);
+ value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ ASSERT_TRUE(value_rate);
+ /* Rate is supposed to be updated */
+ ASSERT_TRUE(value_rate.get() == "10000");
+
/* Configured speed is updated by user */
cfg_port_table.set("Ethernet0", {
{"speed", "200000"},
@@ -162,7 +152,8 @@ namespace sflowmgr_ut
appl_sflow_table.get("Ethernet0", values);
value_rate = swss::fvsGetValue(values, "sample_rate", true);
ASSERT_TRUE(value_rate);
- ASSERT_TRUE(value_rate.get() == "200000");
+ /* Sampling Rate will not be updated */
+ ASSERT_TRUE(value_rate.get() == "10000");
}
TEST_F(SflowMgrTest, test_OnlyStateDbNotif)
@@ -243,8 +234,7 @@ namespace sflowmgr_ut
/* Operational Speed Changes again to 50000 */
state_port_table.set("Ethernet0", {
- {"speed", "50000"},
- {"netdev_oper_status", "up"}
+ {"speed", "50000"}
});
m_sflowMgr->addExistingData(&state_port_table);
From 0bf48266480c1e80b17c2b0dedad9cf1db2274fc Mon Sep 17 00:00:00 2001
From: Vivek Reddy Karri <vkarri@nvidia.com>
Date: Sat, 27 May 2023 08:24:59 +0000
Subject: [PATCH 8/9] Handled comments, Refactored and added new tests
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
---
cfgmgr/sflowmgr.cpp | 25 ++-
cfgmgr/sflowmgr.h | 1 +
tests/mock_tests/sflowmgrd_ut.cpp | 321 ++++++++++++++----------------
3 files changed, 168 insertions(+), 179 deletions(-)
diff --git a/cfgmgr/sflowmgr.cpp b/cfgmgr/sflowmgr.cpp
index 88a37f53e5..d9d46f59b2 100644
--- a/cfgmgr/sflowmgr.cpp
+++ b/cfgmgr/sflowmgr.cpp
@@ -33,6 +33,19 @@ void SflowMgr::readPortConfig()
}
}
+bool SflowMgr::isPortEnabled(const std::string& alias)
+{
+ /* Checks if the sflow is enabled on the port */
+ auto it = m_sflowPortConfMap.find(alias);
+ if (it == m_sflowPortConfMap.end())
+ {
+ return false;
+ }
+ bool local_admin = it->second.local_admin_cfg;
+ bool status = it->second.admin == "up" ? true : false;
+ return m_gEnable && (m_intfAllConf || (local_admin && status));
+}
+
void SflowMgr::sflowHandleService(bool enable)
{
stringstream cmd;
@@ -103,12 +116,16 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer)
if (m_sflowPortConfMap[key].speed != new_speed)
{
m_sflowPortConfMap[key].speed = new_speed;
- rate_update = true;
+ /* if oper_speed is set, no need to write to APP_DB */
+ if (m_sflowPortConfMap[key].oper_speed == NA_SPEED)
+ {
+ rate_update = true;
+ }
}
- if (m_gEnable && m_intfAllConf)
+ if (isPortEnabled(key))
{
- // If the Local rate Conf is already present, dont't override it even though the speed is changed
+ // If the Local rate conf is already present, dont't override it even though the speed is changed
if (new_port || (rate_update && !m_sflowPortConfMap[key].local_rate_cfg))
{
vector<FieldValueTuple> fvs;
@@ -176,7 +193,7 @@ void SflowMgr::sflowProcessOperSpeed(Consumer &consumer)
m_sflowPortConfMap[alias].oper_speed = oper_speed;
}
- if (rate_update && m_gEnable && m_intfAllConf && !m_sflowPortConfMap[alias].local_rate_cfg )
+ if (isPortEnabled(alias) && rate_update && !m_sflowPortConfMap[alias].local_rate_cfg)
{
auto rate = findSamplingRate(alias);
FieldValueTuple fv("sample_rate", rate);
diff --git a/cfgmgr/sflowmgr.h b/cfgmgr/sflowmgr.h
index c1c1f92b60..0a3df13652 100644
--- a/cfgmgr/sflowmgr.h
+++ b/cfgmgr/sflowmgr.h
@@ -49,6 +49,7 @@ class SflowMgr : public Orch
void sflowCheckAndFillValues(std::string alias, std::vector<FieldValueTuple> &values, std::vector<FieldValueTuple> &fvs);
void sflowGetPortInfo(std::vector<FieldValueTuple> &fvs, SflowPortInfo &local_info);
void sflowGetGlobalInfo(std::vector<FieldValueTuple> &fvs, const std::string& alias);
+ bool isPortEnabled(const std::string& alias);
std::string findSamplingRate(const std::string& speed);
};
diff --git a/tests/mock_tests/sflowmgrd_ut.cpp b/tests/mock_tests/sflowmgrd_ut.cpp
index e07cf04359..96f79daa41 100644
--- a/tests/mock_tests/sflowmgrd_ut.cpp
+++ b/tests/mock_tests/sflowmgrd_ut.cpp
@@ -51,219 +51,190 @@ namespace sflowmgr_ut
m_sflowMgr->addExistingData(&cfg_sflow);
m_sflowMgr->doTask();
}
+
+ void cfgSflowSession(string alias, bool status, string sample_rate)
+ {
+ Table cfg_sflow_table(m_config_db.get(), CFG_SFLOW_SESSION_TABLE_NAME);
+ vector<FieldValueTuple> values;
+ values.emplace_back("admin_state", status ? "up" : "down");
+ if (!sample_rate.empty())
+ {
+ values.emplace_back("sample_rate", sample_rate);
+ }
+ cfg_sflow_table.set(alias, values);
+ m_sflowMgr->addExistingData(&cfg_sflow_table);
+ m_sflowMgr->doTask();
+ }
+
+ void cfgSflowSessionAll(bool status)
+ {
+ Table cfg_sflow_table(m_config_db.get(), CFG_SFLOW_SESSION_TABLE_NAME);
+ cfg_sflow_table.set("all", {
+ {"admin_state", status ? "up" : "down"},
+ });
+ m_sflowMgr->addExistingData(&cfg_sflow_table);
+ m_sflowMgr->doTask();
+ }
+
+ void cfgPortSpeed(string alias, string speed)
+ {
+ Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
+ cfg_port_table.set(alias, {
+ {"speed", speed}
+ });
+ m_sflowMgr->addExistingData(&cfg_port_table);
+ m_sflowMgr->doTask();
+ }
+
+ void statePortSpeed(string alias, string speed)
+ {
+ Table state_port_table(m_config_db.get(), STATE_PORT_TABLE_NAME);
+ state_port_table.set(alias, {
+ {"speed", speed}
+ });
+ m_sflowMgr->addExistingData(&state_port_table);
+ m_sflowMgr->doTask();
+ }
+
+ string getSflowSampleRate(string alias)
+ {
+ Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
+ std::vector<FieldValueTuple> values;
+ appl_sflow_table.get("Ethernet0", values);
+ auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
+ if (value_rate)
+ {
+ string ret = value_rate.get();
+ return ret;
+ }
+ return "";
+ }
+
+ string getSflowAdminStatus(string alias)
+ {
+ Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
+ std::vector<FieldValueTuple> values;
+ appl_sflow_table.get("Ethernet0", values);
+ auto value_rate = swss::fvsGetValue(values, "admin_state", true);
+ if (value_rate)
+ {
+ string ret = value_rate.get();
+ return ret;
+ }
+ return "down";
+ }
};
TEST_F(SflowMgrTest, test_RateConfiguration)
{
- Table state_port_table(m_state_db.get(), STATE_PORT_TABLE_NAME);
- Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
- Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
-
- cfg_port_table.set("Ethernet0", {
- {"speed", "100000"},
- });
-
- m_sflowMgr->addExistingData(&cfg_port_table);
- m_sflowMgr->doTask();
-
- std::vector<FieldValueTuple> values;
- appl_sflow_table.get("Ethernet0", values);
- auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
- ASSERT_TRUE(value_rate);
- ASSERT_TRUE(value_rate.get() == "100000");
+ cfgPortSpeed("Ethernet0", "100000");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "100000");
/* Scenario: Operational Speed Changes to 25000 */
- state_port_table.set("Ethernet0", {
- {"speed", "25000"}
- });
-
- m_sflowMgr->addExistingData(&state_port_table);
- m_sflowMgr->doTask();
-
- values.clear();
- appl_sflow_table.get("Ethernet0", values);
- value_rate = swss::fvsGetValue(values, "sample_rate", true);
- ASSERT_TRUE(value_rate);
- ASSERT_TRUE(value_rate.get() == "25000");
+ statePortSpeed("Ethernet0", "25000");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "25000");
}
- TEST_F(SflowMgrTest, test_RateConfigurationOperUpDown)
+ TEST_F(SflowMgrTest, test_RateConfigurationCfgSpeed)
{
- /* Simulates a realistic scenario of oper up down events mixed with auto neg */
- Table state_port_table(m_state_db.get(), STATE_PORT_TABLE_NAME);
- Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
- Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
- std::vector<FieldValueTuple> values;
-
/* Configure the Speed to 100G */
- cfg_port_table.set("Ethernet0", {
- {"speed", "100000"},
- });
+ cfgPortSpeed("Ethernet0", "100000");
/* Scenario: Operational Speed Changes to 100G with autoneg */
- state_port_table.set("Ethernet0", {
- {"speed", "100000"}
- });
-
- m_sflowMgr->addExistingData(&cfg_port_table);
- m_sflowMgr->addExistingData(&state_port_table);
- m_sflowMgr->doTask();
+ statePortSpeed("Ethernet0", "100000");
/* User changes the config speed to 10G */
- cfg_port_table.set("Ethernet0", {
- {"speed", "10000"},
- });
-
- m_sflowMgr->addExistingData(&cfg_port_table);
- m_sflowMgr->doTask();
+ cfgPortSpeed("Ethernet0", "10000");
- values.clear();
- appl_sflow_table.get("Ethernet0", values);
- auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
- ASSERT_TRUE(value_rate);
- /* Rate Should still adhere to oper_speed */
- ASSERT_TRUE(value_rate.get() == "100000");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "100000");
/* Scenario: Operational Speed Changes to 10G, with autoneg */
- state_port_table.set("Ethernet0", {
- {"speed", "10000"}
- });
-
- m_sflowMgr->addExistingData(&state_port_table);
- m_sflowMgr->doTask();
-
- values.clear();
- appl_sflow_table.get("Ethernet0", values);
- value_rate = swss::fvsGetValue(values, "sample_rate", true);
- ASSERT_TRUE(value_rate);
- /* Rate is supposed to be updated */
- ASSERT_TRUE(value_rate.get() == "10000");
+ statePortSpeed("Ethernet0", "10000");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "10000");
/* Configured speed is updated by user */
- cfg_port_table.set("Ethernet0", {
- {"speed", "200000"},
- });
-
- m_sflowMgr->addExistingData(&state_port_table);
- m_sflowMgr->addExistingData(&cfg_port_table);
- m_sflowMgr->doTask();
-
- values.clear();
- appl_sflow_table.get("Ethernet0", values);
- value_rate = swss::fvsGetValue(values, "sample_rate", true);
- ASSERT_TRUE(value_rate);
+ cfgPortSpeed("Ethernet0", "200000");
+
/* Sampling Rate will not be updated */
- ASSERT_TRUE(value_rate.get() == "10000");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "10000");
}
TEST_F(SflowMgrTest, test_OnlyStateDbNotif)
{
- Table state_port_table(m_state_db.get(), STATE_PORT_TABLE_NAME);
- Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
- Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
-
- state_port_table.set("Ethernet0", {
- {"speed", "100000"}
- });
-
- m_sflowMgr->addExistingData(&cfg_port_table);
- m_sflowMgr->doTask();
-
- std::vector<FieldValueTuple> values;
- appl_sflow_table.get("Ethernet0", values);
- auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
- ASSERT_FALSE(value_rate);
+ statePortSpeed("Ethernet0", "100000");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "");
}
TEST_F(SflowMgrTest, test_LocalRateConfiguration)
{
- Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
- Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
- Table cfg_sflow_table(m_config_db.get(), CFG_SFLOW_SESSION_TABLE_NAME);
-
- cfg_port_table.set("Ethernet0", {
- {"speed", "100000"}
- });
-
- cfg_sflow_table.set("Ethernet0", {
- {"sample_rate", "12345"}
- });
-
- m_sflowMgr->addExistingData(&cfg_port_table);
- m_sflowMgr->addExistingData(&cfg_sflow_table);
- m_sflowMgr->doTask();
-
- std::vector<FieldValueTuple> values;
- appl_sflow_table.get("Ethernet0", values);
- auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
- ASSERT_TRUE(value_rate);
- ASSERT_TRUE(value_rate.get() == "12345");
+ cfgPortSpeed("Ethernet0", "100000");
+ cfgSflowSession("Ethernet0", true, "12345");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "12345");
}
TEST_F(SflowMgrTest, test_LocalRateConfWithOperSpeed)
{
- Table state_port_table(m_state_db.get(), STATE_PORT_TABLE_NAME);
- Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
- Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
- Table cfg_sflow_table(m_config_db.get(), CFG_SFLOW_SESSION_TABLE_NAME);
-
- cfg_port_table.set("Ethernet0", {
- {"speed", "100000"},
- });
+ cfgPortSpeed("Ethernet0", "100000");
/* Scenario: Operational Speed Changes to 25000 */
- state_port_table.set("Ethernet0", {
- {"speed", "25000"},
- {"netdev_oper_status", "up"}
- });
-
- cfg_sflow_table.set("Ethernet0", {
- {"sample_rate", "12345"}
- });
-
- m_sflowMgr->addExistingData(&cfg_port_table);
- m_sflowMgr->addExistingData(&cfg_sflow_table);
- m_sflowMgr->addExistingData(&state_port_table);
- m_sflowMgr->doTask();
-
- std::vector<FieldValueTuple> values;
- appl_sflow_table.get("Ethernet0", values);
- auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
- ASSERT_TRUE(value_rate);
- ASSERT_TRUE(value_rate.get() == "12345");
+ statePortSpeed("Ethernet0", "25000");
+
+ /* Set per interface sampling rate*/
+ cfgSflowSession("Ethernet0", true, "12345");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "12345");
/* Operational Speed Changes again to 50000 */
- state_port_table.set("Ethernet0", {
- {"speed", "50000"}
- });
-
- m_sflowMgr->addExistingData(&state_port_table);
- m_sflowMgr->doTask();
-
- appl_sflow_table.get("Ethernet0", values);
- value_rate = swss::fvsGetValue(values, "sample_rate", true);
- /* Local config wouldn't change */
- ASSERT_TRUE(value_rate);
- ASSERT_TRUE(value_rate.get() == "12345");
+ statePortSpeed("Ethernet0", "50000");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "12345");
+ }
+
+ TEST_F(SflowMgrTest, test_newSpeed)
+ {
+ cfgPortSpeed("Ethernet0", "800000");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "800000");
+ }
+
+ TEST_F(SflowMgrTest, test_CfgSpeedAdminCfg)
+ {
+ cfgPortSpeed("Ethernet0", "100000");
+ cfgSflowSessionAll(false); /* Disable sflow on all interfaces*/
+ ASSERT_TRUE(getSflowAdminStatus("Ethernet0") == "down");
+ cfgSflowSession("Ethernet0", true, ""); /* Set local admin up with no rate */
+ ASSERT_TRUE(getSflowAdminStatus("Ethernet0") == "up");
+
+ /* Sampling rate should adhere to config speed*/
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "100000");
+
+ cfgPortSpeed("Ethernet0", "25000"); /* Change cfg speed */
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "25000");
}
- TEST_F(SflowMgrTest, test_new_speed)
+ TEST_F(SflowMgrTest, test_OperSpeedAdminCfg)
{
- Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
- Table cfg_port_table(m_config_db.get(), CFG_PORT_TABLE_NAME);
- Table cfg_sflow_table(m_config_db.get(), CFG_SFLOW_SESSION_TABLE_NAME);
-
- cfg_port_table.set("Ethernet0", {
- {"speed", "800000"}
- });
-
- m_sflowMgr->addExistingData(&cfg_port_table);
- m_sflowMgr->doTask();
-
- std::vector<FieldValueTuple> values;
- appl_sflow_table.get("Ethernet0", values);
- auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
- ASSERT_TRUE(value_rate);
- ASSERT_TRUE(value_rate.get() == "800000");
+ cfgPortSpeed("Ethernet0", "100000");
+ cfgSflowSessionAll(false); /* Disable sflow on all interfaces*/
+ cfgSflowSession("Ethernet0", true, ""); /* Set local admin up with no rate */
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "100000");
+ ASSERT_TRUE(getSflowAdminStatus("Ethernet0") == "up");
+
+ statePortSpeed("Ethernet0", "50000");
+ /* Sampling rate should adhere to oper speed*/
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "50000");
+ ASSERT_TRUE(getSflowAdminStatus("Ethernet0") == "up");
+
+ /* Change cfg speed */
+ cfgPortSpeed("Ethernet0", "25000");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "50000");
+
+ statePortSpeed("Ethernet0", "1000");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "1000");
+
+ cfgSflowSession("Ethernet0", true, "12345"); /* Set local sampling rate */
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "12345");
+ ASSERT_TRUE(getSflowAdminStatus("Ethernet0") == "up");
+
+ /* Change oper speed now */
+ statePortSpeed("Ethernet0", "12345");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "12345");
}
}
From 0009f3d3b0cc703ae88683be89072a39d6d5238f Mon Sep 17 00:00:00 2001
From: Vivek Reddy Karri <vkarri@nvidia.com>
Date: Thu, 1 Jun 2023 22:29:38 +0000
Subject: [PATCH 9/9] Handle merge problem and ut's to cover direction case
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
---
cfgmgr/sflowmgr.cpp | 11 ++--
tests/mock_tests/sflowmgrd_ut.cpp | 88 +++++++++++++++++++++++++++++--
2 files changed, 89 insertions(+), 10 deletions(-)
diff --git a/cfgmgr/sflowmgr.cpp b/cfgmgr/sflowmgr.cpp
index 72043a3f6c..122ffc0780 100644
--- a/cfgmgr/sflowmgr.cpp
+++ b/cfgmgr/sflowmgr.cpp
@@ -206,11 +206,10 @@ void SflowMgr::sflowProcessOperSpeed(Consumer &consumer)
if (isPortEnabled(alias) && rate_update && !m_sflowPortConfMap[alias].local_rate_cfg)
{
- auto rate = findSamplingRate(alias);
- FieldValueTuple fv("sample_rate", rate);
- vector<FieldValueTuple> fvs = {fv};
+ vector<FieldValueTuple> fvs;
+ sflowGetGlobalInfo(fvs, alias, m_sflowPortConfMap[alias].dir);
m_appSflowSessionTable.set(alias, fvs);
- SWSS_LOG_NOTICE("Default sampling rate for %s updated to %s", alias.c_str(), rate.c_str());
+ SWSS_LOG_NOTICE("Default sampling rate for %s updated to %s", alias.c_str(), findSamplingRate(alias).c_str());
}
}
/* Do nothing for DEL as the SflowPortConfMap will already be cleared by the DEL from CONFIG_DB */
@@ -242,7 +241,7 @@ void SflowMgr::sflowHandleSessionAll(bool enable, string direction)
}
else
{
- sflowGetGlobalInfo(fvs, it.second.speed, direction);
+ sflowGetGlobalInfo(fvs, it.first, direction);
}
m_appSflowSessionTable.set(it.first, fvs);
}
@@ -577,7 +576,7 @@ void SflowMgr::doTask(Consumer &consumer)
if (m_intfAllConf)
{
vector<FieldValueTuple> fvs;
- sflowGetGlobalInfo(fvs, m_sflowPortConfMap[key].speed, m_intfAllDir);
+ sflowGetGlobalInfo(fvs, key, m_intfAllDir);
m_appSflowSessionTable.set(key,fvs);
}
}
diff --git a/tests/mock_tests/sflowmgrd_ut.cpp b/tests/mock_tests/sflowmgrd_ut.cpp
index 96f79daa41..7e47b162f2 100644
--- a/tests/mock_tests/sflowmgrd_ut.cpp
+++ b/tests/mock_tests/sflowmgrd_ut.cpp
@@ -39,7 +39,6 @@ namespace sflowmgr_ut
conf_sflow_session_table
};
m_sflowMgr.reset(new SflowMgr(m_app_db.get(), sflow_tables));
- enableSflow();
}
void enableSflow()
@@ -52,7 +51,7 @@ namespace sflowmgr_ut
m_sflowMgr->doTask();
}
- void cfgSflowSession(string alias, bool status, string sample_rate)
+ void cfgSflowSession(string alias, bool status, string sample_rate, string direction = "")
{
Table cfg_sflow_table(m_config_db.get(), CFG_SFLOW_SESSION_TABLE_NAME);
vector<FieldValueTuple> values;
@@ -61,6 +60,10 @@ namespace sflowmgr_ut
{
values.emplace_back("sample_rate", sample_rate);
}
+ if (!direction.empty())
+ {
+ values.emplace_back("sample_direction", direction);
+ }
cfg_sflow_table.set(alias, values);
m_sflowMgr->addExistingData(&cfg_sflow_table);
m_sflowMgr->doTask();
@@ -100,7 +103,7 @@ namespace sflowmgr_ut
{
Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
std::vector<FieldValueTuple> values;
- appl_sflow_table.get("Ethernet0", values);
+ appl_sflow_table.get(alias, values);
auto value_rate = swss::fvsGetValue(values, "sample_rate", true);
if (value_rate)
{
@@ -110,11 +113,25 @@ namespace sflowmgr_ut
return "";
}
+ string getSflowSampleDir(string alias)
+ {
+ Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
+ std::vector<FieldValueTuple> values;
+ appl_sflow_table.get(alias, values);
+ auto value_rate = swss::fvsGetValue(values, "sample_direction", true);
+ if (value_rate)
+ {
+ string ret = value_rate.get();
+ return ret;
+ }
+ return "";
+ }
+
string getSflowAdminStatus(string alias)
{
Table appl_sflow_table(m_app_db.get(), APP_SFLOW_SESSION_TABLE_NAME);
std::vector<FieldValueTuple> values;
- appl_sflow_table.get("Ethernet0", values);
+ appl_sflow_table.get(alias, values);
auto value_rate = swss::fvsGetValue(values, "admin_state", true);
if (value_rate)
{
@@ -127,16 +144,19 @@ namespace sflowmgr_ut
TEST_F(SflowMgrTest, test_RateConfiguration)
{
+ enableSflow();
cfgPortSpeed("Ethernet0", "100000");
ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "100000");
/* Scenario: Operational Speed Changes to 25000 */
statePortSpeed("Ethernet0", "25000");
ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "25000");
+ ASSERT_TRUE(getSflowSampleDir("Ethernet0") == "rx");
}
TEST_F(SflowMgrTest, test_RateConfigurationCfgSpeed)
{
+ enableSflow();
/* Configure the Speed to 100G */
cfgPortSpeed("Ethernet0", "100000");
@@ -161,12 +181,14 @@ namespace sflowmgr_ut
TEST_F(SflowMgrTest, test_OnlyStateDbNotif)
{
+ enableSflow();
statePortSpeed("Ethernet0", "100000");
ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "");
}
TEST_F(SflowMgrTest, test_LocalRateConfiguration)
{
+ enableSflow();
cfgPortSpeed("Ethernet0", "100000");
cfgSflowSession("Ethernet0", true, "12345");
ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "12345");
@@ -174,6 +196,7 @@ namespace sflowmgr_ut
TEST_F(SflowMgrTest, test_LocalRateConfWithOperSpeed)
{
+ enableSflow();
cfgPortSpeed("Ethernet0", "100000");
/* Scenario: Operational Speed Changes to 25000 */
@@ -190,12 +213,14 @@ namespace sflowmgr_ut
TEST_F(SflowMgrTest, test_newSpeed)
{
+ enableSflow();
cfgPortSpeed("Ethernet0", "800000");
ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "800000");
}
TEST_F(SflowMgrTest, test_CfgSpeedAdminCfg)
{
+ enableSflow();
cfgPortSpeed("Ethernet0", "100000");
cfgSflowSessionAll(false); /* Disable sflow on all interfaces*/
ASSERT_TRUE(getSflowAdminStatus("Ethernet0") == "down");
@@ -211,6 +236,7 @@ namespace sflowmgr_ut
TEST_F(SflowMgrTest, test_OperSpeedAdminCfg)
{
+ enableSflow();
cfgPortSpeed("Ethernet0", "100000");
cfgSflowSessionAll(false); /* Disable sflow on all interfaces*/
cfgSflowSession("Ethernet0", true, ""); /* Set local admin up with no rate */
@@ -237,4 +263,58 @@ namespace sflowmgr_ut
statePortSpeed("Ethernet0", "12345");
ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "12345");
}
+
+ TEST_F(SflowMgrTest, test_SflowCfgAfterPortCfg)
+ {
+ cfgPortSpeed("Ethernet0", "100000");
+ /* Nothing is written yet since cfg is not enabled */
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "");
+ ASSERT_TRUE(getSflowAdminStatus("Ethernet0") == "down");
+
+ /* State DB is updated with oper speed */
+ statePortSpeed("Ethernet0", "100000");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "");
+ ASSERT_TRUE(getSflowAdminStatus("Ethernet0") == "down");
+
+ /* enable sflow */
+ enableSflow();
+ cfgSflowSessionAll(true);
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "100000");
+ ASSERT_TRUE(getSflowAdminStatus("Ethernet0") == "up");
+ ASSERT_TRUE(getSflowSampleDir("Ethernet0") == "rx");
+ }
+
+ TEST_F(SflowMgrTest, test_SflowCfgAfterOperSpeed)
+ {
+ cfgPortSpeed("Ethernet0", "100000");
+ /* Nothing is written yet since cfg is not enabled */
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "");
+ ASSERT_TRUE(getSflowAdminStatus("Ethernet0") == "down");
+
+ /* State DB is updated with oper speed */
+ statePortSpeed("Ethernet0", "50000");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "");
+ ASSERT_TRUE(getSflowAdminStatus("Ethernet0") == "down");
+
+ /* enable sflow */
+ cfgSflowSessionAll(true);
+ enableSflow();
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "50000");
+ ASSERT_TRUE(getSflowAdminStatus("Ethernet0") == "up");
+ ASSERT_TRUE(getSflowSampleDir("Ethernet0") == "rx");
+ }
+
+ TEST_F(SflowMgrTest, test_RateConfigEgressDir)
+ {
+ enableSflow();
+ cfgPortSpeed("Ethernet0", "100000");
+ /* Set local admin up with no rate and no egress direction */
+ cfgSflowSession("Ethernet0", true, "", "tx");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "100000");
+
+ /* Scenario: Operational Speed Changes to 25000 */
+ statePortSpeed("Ethernet0", "25000");
+ ASSERT_TRUE(getSflowSampleRate("Ethernet0") == "25000");
+ ASSERT_TRUE(getSflowSampleDir("Ethernet0") == "tx");
+ }
}