Skip to content

Commit

Permalink
Added Max Nexthopgroup/ECMP Count supported by device into State DB. (s…
Browse files Browse the repository at this point in the history
…onic-net#1383)

* Added Max Nexthopgroup/ECMP Count supported by device
to State DB as part of Swicth Capability Table.

Motivation for adding this to StateDB was to make sure
we can query this value (n) from external test case and make sure
we add (n) number of ECMP Group successfully and (n+1) should fail
without termination orchagent.

Since we are using SWITCH_CAPABILTY Table moved the setting of table
field from aclorch to switchorch

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>

* Fix

* Fix VS mock_tables gtest failure.
  • Loading branch information
abdosi authored Aug 8, 2020
1 parent 66eb894 commit 3052fb5
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 22 deletions.
10 changes: 5 additions & 5 deletions orchagent/aclorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2220,7 +2220,7 @@ void AclOrch::init(vector<TableConnector>& connectors, PortsOrch *portOrch, Mirr
break;
}
}
m_switchTable.set("switch", fvVector);
m_switchOrch->set_switch_capability(fvVector);

sai_attribute_t attrs[2];
attrs[0].id = SAI_SWITCH_ATTR_ACL_ENTRY_MINIMUM_PRIORITY;
Expand Down Expand Up @@ -2362,7 +2362,7 @@ void AclOrch::putAclActionCapabilityInDB(acl_stage_type_t stage)
}

fvVector.emplace_back(field, acl_action_value_stream.str());
m_switchTable.set("switch", fvVector);
m_switchOrch->set_switch_capability(fvVector);
}

void AclOrch::initDefaultAclActionCapabilities(acl_stage_type_t stage)
Expand Down Expand Up @@ -2464,7 +2464,7 @@ void AclOrch::queryAclActionAttrEnumValues(const string &action_name,
fvVector.emplace_back(field, acl_action_value_stream.str());
}

m_switchTable.set("switch", fvVector);
m_switchOrch->set_switch_capability(fvVector);
}

sai_acl_action_type_t AclOrch::getAclActionFromAclEntry(sai_acl_entry_attr_t attr)
Expand All @@ -2477,10 +2477,10 @@ sai_acl_action_type_t AclOrch::getAclActionFromAclEntry(sai_acl_entry_attr_t att
return static_cast<sai_acl_action_type_t>(attr - SAI_ACL_ENTRY_ATTR_ACTION_START);
};

AclOrch::AclOrch(vector<TableConnector>& connectors, TableConnector switchTable,
AclOrch::AclOrch(vector<TableConnector>& connectors, SwitchOrch *switchOrch,
PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch, DTelOrch *dtelOrch) :
Orch(connectors),
m_switchTable(switchTable.first, switchTable.second),
m_switchOrch(switchOrch),
m_mirrorOrch(mirrorOrch),
m_neighOrch(neighOrch),
m_routeOrch(routeOrch),
Expand Down
5 changes: 3 additions & 2 deletions orchagent/aclorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <map>
#include <condition_variable>
#include "orch.h"
#include "switchorch.h"
#include "portsorch.h"
#include "mirrororch.h"
#include "dtelorch.h"
Expand Down Expand Up @@ -391,7 +392,7 @@ class AclOrch : public Orch, public Observer
{
public:
AclOrch(vector<TableConnector>& connectors,
TableConnector switchTable,
SwitchOrch *m_switchOrch,
PortsOrch *portOrch,
MirrorOrch *mirrorOrch,
NeighOrch *neighOrch,
Expand All @@ -408,7 +409,6 @@ class AclOrch : public Orch, public Observer
return m_countersTable;
}

Table m_switchTable;

// FIXME: Add getters for them? I'd better to add a common directory of orch objects and use it everywhere
MirrorOrch *m_mirrorOrch;
Expand All @@ -432,6 +432,7 @@ class AclOrch : public Orch, public Observer
static sai_acl_action_type_t getAclActionFromAclEntry(sai_acl_entry_attr_t attr);

private:
SwitchOrch *m_switchOrch;
void doTask(Consumer &consumer);
void doAclTableTask(Consumer &consumer);
void doAclRuleTask(Consumer &consumer);
Expand Down
8 changes: 4 additions & 4 deletions orchagent/orchdaemon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@ bool OrchDaemon::init()
SWSS_LOG_ENTER();

string platform = getenv("platform") ? getenv("platform") : "";
TableConnector stateDbSwitchTable(m_stateDb, "SWITCH_CAPABILITY");

gSwitchOrch = new SwitchOrch(m_applDb, APP_SWITCH_TABLE_NAME);
gSwitchOrch = new SwitchOrch(m_applDb, APP_SWITCH_TABLE_NAME, stateDbSwitchTable);

const int portsorch_base_pri = 40;

Expand Down Expand Up @@ -125,7 +126,7 @@ bool OrchDaemon::init()

gIntfsOrch = new IntfsOrch(m_applDb, APP_INTF_TABLE_NAME, vrf_orch);
gNeighOrch = new NeighOrch(m_applDb, APP_NEIGH_TABLE_NAME, gIntfsOrch);
gRouteOrch = new RouteOrch(m_applDb, APP_ROUTE_TABLE_NAME, gNeighOrch, gIntfsOrch, vrf_orch);
gRouteOrch = new RouteOrch(m_applDb, APP_ROUTE_TABLE_NAME, gSwitchOrch, gNeighOrch, gIntfsOrch, vrf_orch);

TableConnector confDbSflowTable(m_configDb, CFG_SFLOW_TABLE_NAME);
TableConnector appCoppTable(m_applDb, APP_COPP_TABLE_NAME);
Expand Down Expand Up @@ -267,8 +268,7 @@ bool OrchDaemon::init()
dtel_orch = new DTelOrch(m_configDb, dtel_tables, gPortsOrch);
m_orchList.push_back(dtel_orch);
}
TableConnector stateDbSwitchTable(m_stateDb, "SWITCH_CAPABILITY");
gAclOrch = new AclOrch(acl_table_connectors, stateDbSwitchTable, gPortsOrch, mirror_orch, gNeighOrch, gRouteOrch, dtel_orch);
gAclOrch = new AclOrch(acl_table_connectors, gSwitchOrch, gPortsOrch, mirror_orch, gNeighOrch, gRouteOrch, dtel_orch);

m_orchList.push_back(gFdbOrch);
m_orchList.push_back(mirror_orch);
Expand Down
7 changes: 6 additions & 1 deletion orchagent/routeorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ extern CrmOrch *gCrmOrch;

const int routeorch_pri = 5;

RouteOrch::RouteOrch(DBConnector *db, string tableName, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch) :
RouteOrch::RouteOrch(DBConnector *db, string tableName, SwitchOrch *switchOrch, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch) :
gRouteBulker(sai_route_api),
gNextHopGroupMemberBulker(sai_next_hop_group_api, gSwitchId),
Orch(db, tableName, routeorch_pri),
m_switchOrch(switchOrch),
m_neighOrch(neighOrch),
m_intfsOrch(intfsOrch),
m_vrfOrch(vrfOrch),
Expand Down Expand Up @@ -63,6 +64,10 @@ RouteOrch::RouteOrch(DBConnector *db, string tableName, NeighOrch *neighOrch, In
m_maxNextHopGroupCount /= DEFAULT_MAX_ECMP_GROUP_SIZE;
}
}
vector<FieldValueTuple> fvTuple;
fvTuple.emplace_back("MAX_NEXTHOP_GROUP_COUNT", to_string(m_maxNextHopGroupCount));
m_switchOrch->set_switch_capability(fvTuple);

SWSS_LOG_NOTICE("Maximum number of ECMP groups supported is %d", m_maxNextHopGroupCount);

IpPrefix default_ip_prefix("0.0.0.0/0");
Expand Down
4 changes: 3 additions & 1 deletion orchagent/routeorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "orch.h"
#include "observer.h"
#include "switchorch.h"
#include "intfsorch.h"
#include "neighorch.h"

Expand Down Expand Up @@ -88,7 +89,7 @@ struct RouteBulkContext
class RouteOrch : public Orch, public Subject
{
public:
RouteOrch(DBConnector *db, string tableName, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch);
RouteOrch(DBConnector *db, string tableName, SwitchOrch *switchOrch, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch);

bool hasNextHopGroup(const NextHopGroupKey&) const;
sai_object_id_t getNextHopGroupId(const NextHopGroupKey&);
Expand All @@ -108,6 +109,7 @@ class RouteOrch : public Orch, public Subject

void notifyNextHopChangeObservers(sai_object_id_t, const IpPrefix&, const NextHopGroupKey&, bool);
private:
SwitchOrch *m_switchOrch;
NeighOrch *m_neighOrch;
IntfsOrch *m_intfsOrch;
VRFOrch *m_vrfOrch;
Expand Down
8 changes: 7 additions & 1 deletion orchagent/switchorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ const map<string, sai_packet_action_t> packet_action_map =
{"trap", SAI_PACKET_ACTION_TRAP}
};

SwitchOrch::SwitchOrch(DBConnector *db, string tableName) :
SwitchOrch::SwitchOrch(DBConnector *db, string tableName, TableConnector switchTable):
Orch(db, tableName),
m_switchTable(switchTable.first, switchTable.second),
m_db(db)
{
m_restartCheckNotificationConsumer = new NotificationConsumer(db, "RESTARTCHECK");
Expand Down Expand Up @@ -207,3 +208,8 @@ bool SwitchOrch::setAgingFDB(uint32_t sec)
SWSS_LOG_NOTICE("Set switch %" PRIx64 " fdb_aging_time %u sec", gSwitchId, sec);
return true;
}

void SwitchOrch::set_switch_capability(const std::vector<FieldValueTuple>& values)
{
m_switchTable.set("switch", values);
}
5 changes: 3 additions & 2 deletions orchagent/switchorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,21 @@ struct WarmRestartCheck
class SwitchOrch : public Orch
{
public:
SwitchOrch(swss::DBConnector *db, std::string tableName);

SwitchOrch(swss::DBConnector *db, std::string tableName, TableConnector switchTable);
bool checkRestartReady() { return m_warmRestartCheck.checkRestartReadyState; }
bool checkRestartNoFreeze() { return m_warmRestartCheck.noFreeze; }
bool skipPendingTaskCheck() { return m_warmRestartCheck.skipPendingTaskCheck; }
void checkRestartReadyDone() { m_warmRestartCheck.checkRestartReadyState = false; }
void restartCheckReply(const std::string &op, const std::string &data, std::vector<swss::FieldValueTuple> &values);
bool setAgingFDB(uint32_t sec);
void set_switch_capability(const std::vector<swss::FieldValueTuple>& values);
private:
void doTask(Consumer &consumer);

swss::NotificationConsumer* m_restartCheckNotificationConsumer;
void doTask(swss::NotificationConsumer& consumer);
swss::DBConnector *m_db;
swss::Table m_switchTable;

// Information contained in the request from
// external program for orchagent pre-shutdown state check
Expand Down
17 changes: 11 additions & 6 deletions tests/mock_tests/aclorch_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

extern sai_object_id_t gSwitchId;

extern SwitchOrch *gSwitchOrch;
extern CrmOrch *gCrmOrch;
extern PortsOrch *gPortsOrch;
extern RouteOrch *gRouteOrch;
Expand Down Expand Up @@ -122,7 +123,7 @@ namespace aclorch_test
AclOrch *m_aclOrch;
swss::DBConnector *config_db;

MockAclOrch(swss::DBConnector *config_db, swss::DBConnector *state_db,
MockAclOrch(swss::DBConnector *config_db, swss::DBConnector *state_db, SwitchOrch *switchOrch,
PortsOrch *portsOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch) :
config_db(config_db)
{
Expand All @@ -131,9 +132,7 @@ namespace aclorch_test

vector<TableConnector> acl_table_connectors = { confDbAclTable, confDbAclRuleTable };

TableConnector stateDbSwitchTable(state_db, "SWITCH_CAPABILITY");

m_aclOrch = new AclOrch(acl_table_connectors, stateDbSwitchTable, portsOrch, mirrorOrch,
m_aclOrch = new AclOrch(acl_table_connectors, switchOrch, portsOrch, mirrorOrch,
neighOrch, routeOrch);
}

Expand Down Expand Up @@ -285,6 +284,10 @@ namespace aclorch_test

gVirtualRouterId = attr.value.oid;

TableConnector stateDbSwitchTable(m_state_db.get(), "SWITCH_CAPABILITY");
ASSERT_EQ(gSwitchOrch, nullptr);
gSwitchOrch = new SwitchOrch(m_app_db.get(), APP_SWITCH_TABLE_NAME, stateDbSwitchTable);

// Create dependencies ...

const int portsorch_base_pri = 40;
Expand Down Expand Up @@ -313,7 +316,7 @@ namespace aclorch_test
gNeighOrch = new NeighOrch(m_app_db.get(), APP_NEIGH_TABLE_NAME, gIntfsOrch);

ASSERT_EQ(gRouteOrch, nullptr);
gRouteOrch = new RouteOrch(m_app_db.get(), APP_ROUTE_TABLE_NAME, gNeighOrch, gIntfsOrch, gVrfOrch);
gRouteOrch = new RouteOrch(m_app_db.get(), APP_ROUTE_TABLE_NAME, gSwitchOrch, gNeighOrch, gIntfsOrch, gVrfOrch);

TableConnector applDbFdb(m_app_db.get(), APP_FDB_TABLE_NAME);
TableConnector stateDbFdb(m_state_db.get(), STATE_FDB_TABLE_NAME);
Expand Down Expand Up @@ -341,6 +344,8 @@ namespace aclorch_test
{
AclTestBase::TearDown();

delete gSwitchOrch;
gSwitchOrch = nullptr;
delete gFdbOrch;
gFdbOrch = nullptr;
delete gMirrorOrch;
Expand Down Expand Up @@ -374,7 +379,7 @@ namespace aclorch_test

shared_ptr<MockAclOrch> createAclOrch()
{
return make_shared<MockAclOrch>(m_config_db.get(), m_state_db.get(), gPortsOrch, gMirrorOrch,
return make_shared<MockAclOrch>(m_config_db.get(), m_state_db.get(), gSwitchOrch, gPortsOrch, gMirrorOrch,
gNeighOrch, gRouteOrch);
}

Expand Down

0 comments on commit 3052fb5

Please sign in to comment.