Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sflow orchagent changes #1012

Merged
merged 10 commits into from
Oct 28, 2019
Merged

Sflow orchagent changes #1012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9974a14
Sflow orchagent changes
Jul 11, 2019
1714af3
Addressing review comments
Sep 6, 2019
5d20b8e
Addressig review comments
Oct 4, 2019
0a784d8
Fixing build
Oct 4, 2019
d4a2b52
Fixing build
Oct 4, 2019
7674af7
Addressing code review comments
Oct 11, 2019
d31ccd1
Renaming variables
Oct 12, 2019
082232d
Merge branch 'master' into sfloworch
lguohan Oct 14, 2019
b34b48e
Change admin_state
GarrickHe Oct 17, 2019
ef110c3
Merge branch 'master' of https://github.com/Azure/sonic-swss into sfl…
Oct 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions cfgmgr/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ CFLAGS_SAI = -I /usr/include/sai
LIBNL_CFLAGS = -I/usr/include/libnl3
LIBNL_LIBS = -lnl-genl-3 -lnl-route-3 -lnl-3

bin_PROGRAMS = vlanmgrd teammgrd portmgrd intfmgrd buffermgrd vrfmgrd nbrmgrd vxlanmgrd
bin_PROGRAMS = vlanmgrd teammgrd portmgrd intfmgrd buffermgrd vrfmgrd nbrmgrd vxlanmgrd sflowmgrd

if DEBUG
DBGFLAGS = -ggdb -DDEBUG
Expand Down Expand Up @@ -49,4 +49,9 @@ nbrmgrd_LDADD = -lswsscommon $(LIBNL_LIBS)
vxlanmgrd_SOURCES = vxlanmgrd.cpp vxlanmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp shellcmd.h
vxlanmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)
vxlanmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)
vxlanmgrd_LDADD = -lswsscommon
vxlanmgrd_LDADD = -lswsscommon

sflowmgrd_SOURCES = sflowmgrd.cpp sflowmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp shellcmd.h
sflowmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)
sflowmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)
sflowmgrd_LDADD = -lswsscommon
376 changes: 376 additions & 0 deletions cfgmgr/sflowmgr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,376 @@
#include "logger.h"
#include "dbconnector.h"
#include "producerstatetable.h"
#include "tokenize.h"
#include "ipprefix.h"
#include "sflowmgr.h"
#include "exec.h"
#include "shellcmd.h"

using namespace std;
using namespace swss;

map<string,string> sflowSpeedRateInitMap =
{
{SFLOW_SAMPLE_RATE_KEY_400G, SFLOW_SAMPLE_RATE_VALUE_400G},
{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),
m_cfgSflowSessionTable(cfgDb, CFG_SFLOW_SESSION_TABLE_NAME),
m_appSflowTable(appDb, APP_SFLOW_TABLE_NAME),
m_appSflowSessionTable(appDb, APP_SFLOW_SESSION_TABLE_NAME)
{
m_intfAllConf = true;
m_gEnable = false;
}

void SflowMgr::sflowHandleService(bool enable)
{
stringstream cmd;
string res;

SWSS_LOG_ENTER();

if (enable)
{
cmd << "service hsflowd restart";
}
else
{
cmd << "service hsflowd stop";
}

int ret = swss::exec(cmd.str(), res);
if (ret)
{
SWSS_LOG_ERROR("Command '%s' failed with rc %d", cmd.str().c_str(), ret);
}
else
{
SWSS_LOG_NOTICE("Starting hsflowd service");
SWSS_LOG_INFO("Command '%s' succeeded", cmd.str().c_str());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to put is as NOTICE stating "Starting hsflowd service"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}

}

void SflowMgr::sflowUpdatePortInfo(Consumer &consumer)
{
auto it = consumer.m_toSync.begin();

while (it != consumer.m_toSync.end())
{
KeyOpFieldsValuesTuple t = it->second;

string key = kfvKey(t);
string op = kfvOp(t);
auto values = kfvFieldsValues(t);

if (op == SET_COMMAND)
{
SflowPortInfo port_info;
bool new_port = false;

auto sflowPortConf = m_sflowPortConfMap.find(key);
if (sflowPortConf == m_sflowPortConfMap.end())
{
new_port = true;
port_info.local_conf = false;
port_info.speed = SFLOW_ERROR_SPEED_STR;
port_info.rate = "";
port_info.admin = "";
m_sflowPortConfMap[key] = port_info;
}
for (auto i : values)
{
if (fvField(i) == "speed")
{
m_sflowPortConfMap[key].speed = fvValue(i);
}
}

if (new_port)
{
if (m_gEnable && m_intfAllConf)
{
vector<FieldValueTuple> fvs;
sflowGetGlobalInfo(fvs, m_sflowPortConfMap[key].speed);
m_appSflowSessionTable.set(key, fvs);
}
}
}
else if (op == DEL_COMMAND)
{
auto sflowPortConf = m_sflowPortConfMap.find(key);
if (sflowPortConf != m_sflowPortConfMap.end())
{
bool local_cfg = m_sflowPortConfMap[key].local_conf;

m_sflowPortConfMap.erase(key);
if ((m_intfAllConf && m_gEnable) || local_cfg)
{
m_appSflowSessionTable.del(key);
}
}
}
it = consumer.m_toSync.erase(it);
}
}

void SflowMgr::sflowHandleSessionAll(bool enable)
{
for (auto it: m_sflowPortConfMap)
{
if (!it.second.local_conf)
{
vector<FieldValueTuple> fvs;
sflowGetGlobalInfo(fvs, it.second.speed);
if (enable)
{
m_appSflowSessionTable.set(it.first, fvs);
}
else
{
m_appSflowSessionTable.del(it.first);
}
}
}
}

void SflowMgr::sflowHandleSessionLocal(bool enable)
{
for (auto it: m_sflowPortConfMap)
{
if (it.second.local_conf)
{
vector<FieldValueTuple> fvs;
sflowGetPortInfo(fvs, it.second);
if (enable)
{
m_appSflowSessionTable.set(it.first, fvs);
}
else
{
m_appSflowSessionTable.del(it.first);
}
}
}
}

void SflowMgr::sflowGetGlobalInfo(vector<FieldValueTuple> &fvs, string speed)
{
string rate;
FieldValueTuple fv1("admin_state", "up");
fvs.push_back(fv1);

if (speed != SFLOW_ERROR_SPEED_STR)
{
rate = sflowSpeedRateInitMap[speed];
}
else
{
rate = SFLOW_ERROR_SPEED_STR;
}
FieldValueTuple fv2("sample_rate",rate);
fvs.push_back(fv2);
}

void SflowMgr::sflowGetPortInfo(vector<FieldValueTuple> &fvs, SflowPortInfo &local_info)
{
if (local_info.admin.length() > 0)
{
FieldValueTuple fv1("admin_state", local_info.admin);
fvs.push_back(fv1);
}

FieldValueTuple fv2("sample_rate", local_info.rate);
fvs.push_back(fv2);
}

void SflowMgr::sflowCheckAndFillValues(string alias, vector<FieldValueTuple> &fvs)
{
string rate;
bool admin_present = false;
bool rate_present = false;

for (auto i : fvs)
{
if (fvField(i) == "sample_rate")
{
rate_present = true;
m_sflowPortConfMap[alias].rate = fvValue(i);
}
if (fvField(i) == "admin_state")
{
admin_present = true;
m_sflowPortConfMap[alias].admin = fvValue(i);
}
}

if (!rate_present)
{
if (m_sflowPortConfMap[alias].rate == "")
{
string speed = m_sflowPortConfMap[alias].speed;

if (speed != SFLOW_ERROR_SPEED_STR)
{
rate = sflowSpeedRateInitMap[speed];
}
else
{
rate = SFLOW_ERROR_SPEED_STR;
}
m_sflowPortConfMap[alias].rate = rate;
}
FieldValueTuple fv("sample_rate", m_sflowPortConfMap[alias].rate);
fvs.push_back(fv);
}

if (!admin_present)
{
if (m_sflowPortConfMap[alias].admin == "")
{
/* By default admin state is enable if not set explicitely */
m_sflowPortConfMap[alias].admin = "up";
}
FieldValueTuple fv("admin_state", m_sflowPortConfMap[alias].admin);
fvs.push_back(fv);
}
}

void SflowMgr::doTask(Consumer &consumer)
{
SWSS_LOG_ENTER();

auto table = consumer.getTableName();

if (table == CFG_PORT_TABLE_NAME)
{
sflowUpdatePortInfo(consumer);
return;
}

auto it = consumer.m_toSync.begin();
while (it != consumer.m_toSync.end())
prsunny marked this conversation as resolved.
Show resolved Hide resolved
{
KeyOpFieldsValuesTuple t = it->second;

string key = kfvKey(t);
string op = kfvOp(t);
auto values = kfvFieldsValues(t);

if (op == SET_COMMAND)
{
if (table == CFG_SFLOW_TABLE_NAME)
{
for (auto i : values)
{
if (fvField(i) == "admin_state")
{
bool enable = false;
if (fvValue(i) == "up")
{
prsunny marked this conversation as resolved.
Show resolved Hide resolved
enable = true;
}
if (enable == m_gEnable)
{
break;
}
m_gEnable = enable;
sflowHandleService(enable);
if (m_intfAllConf)
{
sflowHandleSessionAll(enable);
}
sflowHandleSessionLocal(enable);
prsunny marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +291 to +293
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typical practice for admin_state toggle is just to propagate the field-value tuple change to APPL_DB. Here, when looking into these two function internals, admin_state: down triggers deleting the entire key, say Ethernet1, in APPL_DB SFLOW_SESSION table instead of an update to field admin_state under the key. This is consistent with the HLD. But quite curious about the design thought behind it.

Copy link
Collaborator Author

@dgsudharsan dgsudharsan Sep 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wendani : Sflow has the following commands

  1. config sflow enable ------------> Enable/disable feature as a whole. When this is disabled, it means all sflow configurations need to be removed (Feature level disable).
  2. config sflow interface all enable/disable ----> Global level sflow command to control the enabling/disabling sflow globally. By default when sflow feature is enabled, all ports will have flow enabled. The main usage of this command is in 'disable' configuration. When users need to enable sflow on only one interface or very few interfaces, without global interface disable command, the user has to configure individual interface disable on the rest of the interfaces except for the interfaces of interest. Thus to avoid this, users can have sflow global interface all disable and then do individual sflow enable on interested interfaces which would override the global setting.
  3. config sflow interface Ethernetx enable/disable ------> Local interface level command which would override global interface all command when configured on specific interfaces

}
}
m_appSflowTable.set(key, values);
}
else if (table == CFG_SFLOW_SESSION_TABLE_NAME)
{
if (key == "all")
{
for (auto i : values)
{
if (fvField(i) == "admin_state")
{
bool enable = false;

if (fvValue(i) == "up")
{
enable = true;
}
if ((enable != m_intfAllConf) && (m_gEnable))
{
sflowHandleSessionAll(enable);
}
m_intfAllConf = enable;
}
}
}
else
{
auto sflowPortConf = m_sflowPortConfMap.find(key);

if (sflowPortConf == m_sflowPortConfMap.end())
{
it++;
continue;
}
sflowCheckAndFillValues(key,values);
m_sflowPortConfMap[key].local_conf = true;
m_appSflowSessionTable.set(key, values);
}
}
}
else if (op == DEL_COMMAND)
{
if (table == CFG_SFLOW_TABLE_NAME)
{
if (m_gEnable)
{
sflowHandleService(false);
sflowHandleSessionAll(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also call sflowHandleSessionLocal(false); here to disable individually configured port(s); otherwise redis-cli -n 4 del "SFLOW|global" command will not remove APPL_DB SFLOW_SESSION_TABLE:EthernetX for individually configured port EthernetX.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. This needs to be fixed.

}
m_gEnable = false;
m_appSflowTable.del(key);
}
else if (table == CFG_SFLOW_SESSION_TABLE_NAME)
{
if (key == "all")
{
if (!m_intfAllConf)
{
sflowHandleSessionAll(true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this called with true?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When DEL is issued for "config sflow interface all" command and it is currently not enabled, it means we are removing "config sflow interface disable all" CLI. Once it is removed, the default state is all interfaces are enabled and thus the API is called with true

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should feed m_gEnable instead of true. Consider the following sequence of CLIs:

$ sudo config sflow interface disable all

m_intfAllConf is false

$ redis-cli -n 4 del "SFLOW|global"

m_gEnable is false

$ redis-cli -n 4 del "SFLOW_SESSION|all"

APPL_DB SFLOW_SESSION_TABLE:EthernetX will show up for non-individually configured port EthernetX under the condition that m_gEnable is false

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that APP_DB would be populated but there is a check in orchagent which will ensure that ports are not programmed when sflow feature is disabled. Hence there will not be any functional issue.
I saw another scenario too where similarly appDB would be populated but it wouldn't be pushed to ASIC DB. When feature is disabled and when config sflow interface Ethernetx sample-rate/enable command is executed we can find them present in APP_DB.
These scenarios need to be cleaned up.

}
m_intfAllConf = true;
}
else
{
m_appSflowSessionTable.del(key);
m_sflowPortConfMap[key].local_conf = false;
m_sflowPortConfMap[key].rate = "";
m_sflowPortConfMap[key].admin = "";

/* If Global configured, set global session on port after local config is deleted */
if (m_intfAllConf)
{
vector<FieldValueTuple> fvs;
sflowGetGlobalInfo(fvs, m_sflowPortConfMap[key].speed);
m_appSflowSessionTable.set(key,fvs);
}
}
}
}
it = consumer.m_toSync.erase(it);
}
}
Loading