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.

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

Mux/IPTunnel orchagent changes #1497

Merged
merged 11 commits into from
Dec 23, 2020
7 changes: 6 additions & 1 deletion cfgmgr/Makefile.am
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 sflowmgrd natmgrd coppmgrd
bin_PROGRAMS = vlanmgrd teammgrd portmgrd intfmgrd buffermgrd vrfmgrd nbrmgrd vxlanmgrd sflowmgrd natmgrd coppmgrd tunnelmgrd

cfgmgrdir = $(datadir)/swss

Expand Down Expand Up @@ -76,3 +76,8 @@ coppmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)
coppmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)
coppmgrd_LDADD = -lswsscommon

tunnelmgrd_SOURCES = tunnelmgrd.cpp tunnelmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp shellcmd.h
tunnelmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)
tunnelmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)
tunnelmgrd_LDADD = -lswsscommon

87 changes: 87 additions & 0 deletions cfgmgr/tunnelmgr.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
#include <algorithm>
#include <regex>
#include <sstream>
#include <string>
#include <net/if.h>

#include "logger.h"
#include "tunnelmgr.h"

using namespace std;
using namespace swss;

TunnelMgr::TunnelMgr(DBConnector *cfgDb, DBConnector *appDb, std::string tableName) :
Orch(cfgDb, tableName),
m_appIpInIpTunnelTable(appDb, APP_TUNNEL_DECAP_TABLE_NAME)
{
}

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

auto it = consumer.m_toSync.begin();
while (it != consumer.m_toSync.end())
{
bool task_result = false;

KeyOpFieldsValuesTuple t = it->second;
const vector<FieldValueTuple>& data = kfvFieldsValues(t);

const std::string & op = kfvOp(t);

if (op == SET_COMMAND)
{
for (auto idx : data)
{
const auto &field = fvField(idx);
const auto &value = fvValue(idx);
if (field == "tunnel_type")
{
if (value == "IPINIP")
{
task_result = doIpInIpTunnelTask(t);
}
}
}
}
else if (op == DEL_COMMAND)
{
/* TODO: Handle Tunnel delete for other tunnel types */
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a check for value == "IPINIP" before calling doIpInIpTunnelTask(t)? This way it is in parity with how the "SET_COMMAND" is handled that do check for the tunnel type.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the del operation we will not get the value

task_result = doIpInIpTunnelTask(t);
}
else
{
SWSS_LOG_ERROR("Unknown operation: '%s'", op.c_str());
}

if (task_result == true)
{
it = consumer.m_toSync.erase(it);
}
else
{
++it;
}
}
}

bool TunnelMgr::doIpInIpTunnelTask(const KeyOpFieldsValuesTuple & t)
{
SWSS_LOG_ENTER();

const std::string & TunnelName = kfvKey(t);
const std::string & op = kfvOp(t);

if (op == SET_COMMAND)
{
m_appIpInIpTunnelTable.set(TunnelName, kfvFieldsValues(t));
}
else
{
m_appIpInIpTunnelTable.del(TunnelName);
}

SWSS_LOG_NOTICE("Tunnel %s task, op %s", TunnelName.c_str(), op.c_str());
return true;
}
23 changes: 23 additions & 0 deletions cfgmgr/tunnelmgr.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#pragma once

#include "dbconnector.h"
#include "producerstatetable.h"
#include "orch.h"

namespace swss {

class TunnelMgr : public Orch
{
public:
TunnelMgr(DBConnector *cfgDb, DBConnector *appDb, std::string tableName);
Copy link
Contributor

Choose a reason for hiding this comment

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

What about explicitly deleting the default and copy constructor. Also, calling destructor as default

using Orch::doTask;

private:
void doTask(Consumer &consumer);

bool doIpInIpTunnelTask(const KeyOpFieldsValuesTuple & t);

ProducerStateTable m_appIpInIpTunnelTable;
};

}
86 changes: 86 additions & 0 deletions cfgmgr/tunnelmgrd.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
#include <unistd.h>
#include <vector>
#include <sstream>
#include <fstream>
#include <iostream>
#include <mutex>
#include <algorithm>

#include "dbconnector.h"
#include "select.h"
#include "exec.h"
#include "schema.h"
#include "tunnelmgr.h"

using namespace std;
using namespace swss;

/* select() function timeout retry time, in millisecond */
#define SELECT_TIMEOUT 1000
Copy link
Contributor

Choose a reason for hiding this comment

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

What about the time unit to the name SELECT_TIMEOUT_MSEC

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the default value for all mgrs, keeping it for consistency


/*
* Following global variables are defined here for the purpose of
* using existing Orch class which is to be refactored soon to
* eliminate the direct exposure of the global variables.
*
* Once Orch class refactoring is done, these global variables
* should be removed from here.
*/
int gBatchSize = 0;
bool gSwssRecord = false;
bool gLogRotate = false;
ofstream gRecordOfs;
string gRecordFile;
/* Global database mutex */
mutex gDbMutex;

int main(int argc, char **argv)
{
Logger::linkToDbNative("tunnelmgrd");

SWSS_LOG_NOTICE("--- Starting Tunnelmgrd ---");

try
{

DBConnector cfgDb("CONFIG_DB", 0);
DBConnector appDb("APPL_DB", 0);

TunnelMgr tunnelmgr(&cfgDb, &appDb, CFG_TUNNEL_TABLE_NAME);

std::vector<Orch *> cfgOrchList = {&tunnelmgr};
Copy link
Contributor

Choose a reason for hiding this comment

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

curious cfg_tunnel_tables is a list of one and cfgOrchList is a list of one. Why not use variable?

nit: style mismatch camelCase vs hyphened_variables.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Addressed


swss::Select s;
for (Orch *o : cfgOrchList)
{
s.addSelectables(o->getSelectables());
}

SWSS_LOG_NOTICE("starting main loop");
while (true)
{
Selectable *sel;
int ret;

ret = s.select(&sel, SELECT_TIMEOUT);
if (ret == Select::ERROR)
{
SWSS_LOG_NOTICE("Error: %s!", strerror(errno));
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to use SWSS_LOG_ERROR instead of NOTICE here for the error handling?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same for all managers, keeping for consistency

continue;
}
if (ret == Select::TIMEOUT)
{
tunnelmgr.doTask();
continue;
}

auto *c = (Executor *)sel;
c->execute();
}
}
catch(const std::exception &e)
{
SWSS_LOG_ERROR("Runtime error: %s", e.what());
}
return -1;
}
3 changes: 2 additions & 1 deletion orchagent/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ orchagent_SOURCES = \
sfloworch.cpp \
chassisorch.cpp \
debugcounterorch.cpp \
natorch.cpp
natorch.cpp \
muxorch.cpp

orchagent_SOURCES += flex_counter/flex_counter_manager.cpp flex_counter/flex_counter_stat_manager.cpp
orchagent_SOURCES += debug_counter/debug_counter.cpp debug_counter/drop_counter.cpp
Expand Down
10 changes: 10 additions & 0 deletions orchagent/aclorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,16 @@ bool AclRulePfcwd::validateAddMatch(string attr_name, string attr_value)
return AclRule::validateAddMatch(attr_name, attr_value);
}

AclRuleMux::AclRuleMux(AclOrch *aclOrch, string rule, string table, acl_table_type_t type, bool createCounter) :
AclRuleL3(aclOrch, rule, table, type, createCounter)
{
}

bool AclRuleMux::validateAddMatch(string attr_name, string attr_value)
{
return AclRule::validateAddMatch(attr_name, attr_value);
}

AclRuleL3V6::AclRuleL3V6(AclOrch *aclOrch, string rule, string table, acl_table_type_t type) :
AclRuleL3(aclOrch, rule, table, type)
{
Expand Down
10 changes: 9 additions & 1 deletion orchagent/aclorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#define TABLE_TYPE_DTEL_FLOW_WATCHLIST "DTEL_FLOW_WATCHLIST"
#define TABLE_TYPE_DTEL_DROP_WATCHLIST "DTEL_DROP_WATCHLIST"
#define TABLE_TYPE_MCLAG "MCLAG"
#define TABLE_TYPE_MUX "MUX"

#define RULE_PRIORITY "PRIORITY"
#define MATCH_IN_PORTS "IN_PORTS"
Expand Down Expand Up @@ -115,7 +116,8 @@ typedef enum
ACL_TABLE_CTRLPLANE,
ACL_TABLE_DTEL_FLOW_WATCHLIST,
ACL_TABLE_DTEL_DROP_WATCHLIST,
ACL_TABLE_MCLAG
ACL_TABLE_MCLAG,
ACL_TABLE_MUX
} acl_table_type_t;

typedef map<string, acl_table_type_t> acl_table_type_lookup_t;
Expand Down Expand Up @@ -272,6 +274,12 @@ class AclRulePfcwd: public AclRuleL3
bool validateAddMatch(string attr_name, string attr_value);
};

class AclRuleMux: public AclRuleL3
{
public:
AclRuleMux(AclOrch *m_pAclOrch, string rule, string table, acl_table_type_t type, bool createCounter = false);
bool validateAddMatch(string attr_name, string attr_value);
};

class AclRuleMirror: public AclRule
{
Expand Down
Loading