Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
8 changes: 5 additions & 3 deletions orchagent/Makefile.am
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
INCLUDES = -I $(top_srcdir)
INCLUDES = -I $(top_srcdir) -I $(top_srcdir)/warmrestart

CFLAGS_SAI = -I /usr/include/sai

Expand Down Expand Up @@ -66,12 +66,14 @@ orchagent_SOURCES = \
switchorch.h \
swssnet.h \
tunneldecaporch.h \
crmorch.h
crmorch.h \
request_parser.h \
vrforch.h \
dtelorch.h \
countercheckorch.h \
flexcounterorch.h
flexcounterorch.h \
$(top_srcdir)/warmrestart/warm_restart.cpp \
$(top_srcdir)/warmrestart/warm_restart.h

orchagent_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)
orchagent_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)
Expand Down
1 change: 1 addition & 0 deletions orchagent/aclorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1694,6 +1694,7 @@ void AclOrch::init(vector<TableConnector>& connectors, PortsOrch *portOrch, Mirr
auto interv = timespec { .tv_sec = COUNTERS_READ_INTERVAL, .tv_nsec = 0 };
auto timer = new SelectableTimer(interv);
auto executor = new ExecutableTimer(timer, this);
executor->setName("ACL_POLL_TIMER");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not just change the constructor to set the name.

not sure if it is useful to introduce setName method, since we do not (probably should not) allow change the name.

Orch::addExecutor("", executor);
timer->start();
}
Expand Down
1 change: 1 addition & 0 deletions orchagent/countercheckorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ CounterCheckOrch::CounterCheckOrch(DBConnector *db, vector<string> &tableNames):
auto interv = timespec { .tv_sec = COUNTER_CHECK_POLL_TIMEOUT_SEC, .tv_nsec = 0 };
auto timer = new SelectableTimer(interv);
auto executor = new ExecutableTimer(timer, this);
executor->setName("MC_COUNTERS_POLL");
Orch::addExecutor("MC_COUNTERS_POLL", executor);
timer->start();
}
Expand Down
6 changes: 5 additions & 1 deletion orchagent/crmorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,11 @@ CrmOrch::CrmOrch(DBConnector *db, string tableName):
m_resourcesMap.emplace(res.first, CrmResourceEntry(res.second, CRM_THRESHOLD_TYPE_DEFAULT, CRM_THRESHOLD_LOW_DEFAULT, CRM_THRESHOLD_HIGH_DEFAULT));
}

// The CRM stats needs to be populated again
m_countersCrmTable->del(CRM_COUNTERS_TABLE_KEY);
Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft Aug 13, 2018

Choose a reason for hiding this comment

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

m_countersCrmTable->del(CRM_COUNTERS_TABLE_KEY); [](start = 4, length = 48)

If it is applied to cold start, let's do it in another PR. #Closed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For cold start, no data exists for CRM stats table, so it has no effect.


auto executor = new ExecutableTimer(m_timer.get(), this);
executor->setName("CRM_COUNTERS_POLL");
Orch::addExecutor("CRM_COUNTERS_POLL", executor);
m_timer->start();
}
Expand Down Expand Up @@ -333,7 +337,7 @@ void CrmOrch::decCrmAclUsedCounter(CrmResourceType resource, sai_acl_stage_t sta
{
m_resourcesMap.at(resource).countersMap[getCrmAclKey(stage, point)].usedCounter--;

// Remove ACL table related counters
// Remove ACL table related counters
if (resource == CrmResourceType::CRM_ACL_TABLE)
{
auto & cntMap = m_resourcesMap.at(CrmResourceType::CRM_ACL_TABLE).countersMap;
Expand Down
11 changes: 11 additions & 0 deletions orchagent/fdborch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,17 @@ FdbOrch::FdbOrch(DBConnector *db, string tableName, PortsOrch *port) :
m_portsOrch->attach(this);
m_flushNotificationsConsumer = new NotificationConsumer(db, "FLUSHFDBREQUEST");
auto flushNotifier = new Notifier(m_flushNotificationsConsumer, this);
flushNotifier->setName("FLUSHFDBREQUEST");
Orch::addExecutor("", flushNotifier);

/* Add FDB notifications support from ASIC */
DBConnector *notificationsDb = new DBConnector(ASIC_DB, DBConnector::DEFAULT_UNIXSOCKET, 0);
m_fdbNotificationConsumer = new swss::NotificationConsumer(notificationsDb, "NOTIFICATIONS");
auto fdbNotifier = new Notifier(m_fdbNotificationConsumer, this);
fdbNotifier->setName("FDB_NOTIFICATIONS");
Orch::addExecutor("FDB_NOTIFICATIONS", fdbNotifier);

addExistingData(tableName);
}

void FdbOrch::update(sai_fdb_event_t type, const sai_fdb_entry_t* entry, sai_object_id_t bridge_port_id)
Expand All @@ -53,6 +57,13 @@ void FdbOrch::update(sai_fdb_event_t type, const sai_fdb_entry_t* entry, sai_obj
return;
}

if (m_entries.count(update.entry) != 0) // we already have such entries
{
SWSS_LOG_INFO("FdbOrch notification: mac %s is already in bv_id 0x%lx",
update.entry.mac.to_string().c_str(), entry->bv_id);
break;
}

update.add = true;

{
Expand Down
16 changes: 15 additions & 1 deletion orchagent/intfsorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ IntfsOrch::IntfsOrch(DBConnector *db, string tableName) :
Orch(db, tableName, intfsorch_pri)
{
SWSS_LOG_ENTER();

// Read the pre-existing data for INTF in appDB.
addExistingData(APP_INTF_TABLE_NAME);

}

sai_object_id_t IntfsOrch::getRouterIntfsId(const string &alias)
Expand Down Expand Up @@ -86,7 +90,16 @@ void IntfsOrch::doTask(Consumer &consumer)
{
if (alias == "lo")
{
addIp2MeRoute(ip_prefix);
// set request for lo may come after warm start restore
Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft Aug 13, 2018

Choose a reason for hiding this comment

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

Is it related to warm start? If not, let's do it in another PR. #Closed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is related to warm start. It is noticed that sometime IP data for lo comes after warm start.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The logic is also applied to cold start. Are you fixing a bug also exists in cold start? if yes, we prefer a separate PR just for it.


In reply to: 209787586 [](ancestors = 209787586)

auto it_intfs = m_syncdIntfses.find(alias);
if (it_intfs == m_syncdIntfses.end())
{
IntfsEntry intfs_entry;
intfs_entry.ref_count = 0;
m_syncdIntfses[alias] = intfs_entry;
addIp2MeRoute(ip_prefix);
}

it = consumer.m_toSync.erase(it);
continue;
}
Expand Down Expand Up @@ -171,6 +184,7 @@ void IntfsOrch::doTask(Consumer &consumer)
{
if (alias == "lo")
{
m_syncdIntfses.erase(alias);
Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft Aug 13, 2018

Choose a reason for hiding this comment

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

The same. #Closed

removeIp2MeRoute(ip_prefix);
it = consumer.m_toSync.erase(it);
continue;
Expand Down
32 changes: 22 additions & 10 deletions orchagent/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ extern "C" {
#include "saihelper.h"
#include "notifications.h"
#include <signal.h>
#include <warm_restart.h>
Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft Aug 13, 2018

Choose a reason for hiding this comment

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

#include "warm_restart.h" #Closed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok.


using namespace std;
using namespace swss;
Expand Down Expand Up @@ -77,6 +78,23 @@ void sighup_handler(int signo)
}
}

void syncd_apply_view()
{
SWSS_LOG_NOTICE("Notify syncd APPLY_VIEW");

sai_status_t status;
sai_attribute_t attr;
attr.id = SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD;
attr.value.s32 = SAI_REDIS_NOTIFY_SYNCD_APPLY_VIEW;
status = sai_switch_api->set_switch_attribute(gSwitchId, &attr);

if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to notify syncd APPLY_VIEW %d", status);
exit(EXIT_FAILURE);
}
}

int main(int argc, char **argv)
{
swss::Logger::linkToDbNative("orchagent");
Expand Down Expand Up @@ -251,6 +269,8 @@ int main(int argc, char **argv)
DBConnector config_db(CONFIG_DB, DBConnector::DEFAULT_UNIXSOCKET, 0);
DBConnector state_db(STATE_DB, DBConnector::DEFAULT_UNIXSOCKET, 0);

WarmStart::checkWarmStart("orchagent");

OrchDaemon *orchDaemon = new OrchDaemon(&appl_db, &config_db, &state_db);
if (!orchDaemon->init())
{
Expand All @@ -261,17 +281,9 @@ int main(int argc, char **argv)

try
{
SWSS_LOG_NOTICE("Notify syncd APPLY_VIEW");

attr.id = SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD;
attr.value.s32 = SAI_REDIS_NOTIFY_SYNCD_APPLY_VIEW;
status = sai_switch_api->set_switch_attribute(gSwitchId, &attr);

if (status != SAI_STATUS_SUCCESS)
if (!WarmStart::isWarmStart())
{
SWSS_LOG_ERROR("Failed to notify syncd APPLY_VIEW %d", status);
delete orchDaemon;
exit(EXIT_FAILURE);
syncd_apply_view();
}

orchDaemon->start();
Expand Down
4 changes: 4 additions & 0 deletions orchagent/neighorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ NeighOrch::NeighOrch(DBConnector *db, string tableName, IntfsOrch *intfsOrch) :
Orch(db, tableName, neighorch_pri), m_intfsOrch(intfsOrch)
{
SWSS_LOG_ENTER();

// Read the pre-existing data for neigh in appDB.
addExistingData(tableName);

}

bool NeighOrch::hasNextHop(IpAddress ipAddress)
Expand Down
32 changes: 32 additions & 0 deletions orchagent/orch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,23 @@ void Consumer::drain()
m_orch->doTask(*this);
}

void Consumer::dumpTasks(vector<string> &ts)
{
for (auto &tm :m_toSync)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add one more blank after ':'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok

{
KeyOpFieldsValuesTuple& tuple = tm.second;

string s = getTableName() + ":" + kfvKey(tuple)
+ "|" + kfvOp(tuple);
for (auto i = kfvFieldsValues(tuple).begin(); i != kfvFieldsValues(tuple).end(); i++)
{
s += "|" + fvField(*i) + ":" + fvValue(*i);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do not hardcode the separator.

}

ts.push_back(s);
}
}

bool Orch::addExistingData(const string& tableName)
{
Consumer* consumer = dynamic_cast<Consumer *>(getExecutor(tableName));
Expand Down Expand Up @@ -293,6 +310,21 @@ void Orch::doTask()
}
}

void Orch::dumpTasks(vector<string> &ts)
{
for(auto &it : m_consumerMap)
{
Consumer* consumer = dynamic_cast<Consumer *>(it.second.get());
if (consumer == NULL)
{
SWSS_LOG_DEBUG("Executor is not a Consumer");
continue;
}

consumer->dumpTasks(ts);
}
}

void Orch::logfileReopen()
{
gRecordOfs.close();
Expand Down
28 changes: 28 additions & 0 deletions orchagent/orch.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,22 @@ class Executor : public Selectable
virtual void execute() { }
virtual void drain() { }

virtual string getName() const
{
return m_name;
}
virtual void setName(string name)
{
m_name = name;
}

protected:
Selectable *m_selectable;
Orch *m_orch;

// Name for Executor
string m_name;

// Get the underlying selectable
Selectable *getSelectable() const { return m_selectable; }
};
Expand All @@ -116,10 +128,24 @@ class Consumer : public Executor {
return getConsumerTable()->getTableName();
}


string getName() const
{
return getConsumerTable()->getTableName();
}

int getDbId() const
{
return getConsumerTable()->getDbId();
}

void dumpTasks(vector<string> &ts);

void addToSync(std::deque<KeyOpFieldsValuesTuple> &entries);
void refillToSync();
void refillToSync(Table* table);
void execute();
Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft Aug 13, 2018

Choose a reason for hiding this comment

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

void execute(); [](start = 4, length = 15)

No need to move this line. #Closed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok


void drain();

/* Store the latest 'golden' status */
Expand Down Expand Up @@ -166,6 +192,8 @@ class Orch

/* TODO: refactor recording */
static void recordTuple(Consumer &consumer, KeyOpFieldsValuesTuple &tuple);

void dumpTasks(vector<string> &ts);
protected:
ConsumerMap m_consumerMap;

Expand Down
Loading