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

Port configuration incremental update support #2305

Merged
merged 16 commits into from
Jul 7, 2022
+205 −122
Merged

Port configuration incremental update support #2305

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
0273978
Implement incremental port configuration update
Junchao-Mellanox Mar 8, 2022
623b64a
Improve unit test coverage
Junchao-Mellanox Apr 2, 2022
400ee4b
Merge remote-tracking branch 'origin/master' into incre-port-update
Junchao-Mellanox Apr 11, 2022
24697b2
Merge remote-tracking branch 'origin/master' into incre-port-update
Junchao-Mellanox Apr 18, 2022
c104b77
Fix vs test failure
Junchao-Mellanox Apr 25, 2022
1cca0aa
Merge remote-tracking branch 'origin/master' into incre-port-update
Junchao-Mellanox Apr 27, 2022
97fae06
Do not check configuration value since there could be case that someo…
Junchao-Mellanox Apr 28, 2022
9dce0ca
Merge branch 'master' into incre-port-update
Junchao-Mellanox May 5, 2022
9c5990d
Merge commit '910bfd4' into incre-port-update
Junchao-Mellanox Jun 1, 2022
0b9b2e1
Port incremental configuration support
Junchao-Mellanox Jun 2, 2022
0b18221
Merge remote-tracking branch 'origin/master' into incre-port-update-1
Junchao-Mellanox Jun 2, 2022
bbaadc5
Remove unused code
Junchao-Mellanox Jun 2, 2022
55a18d9
Fix review comment
Junchao-Mellanox Jun 21, 2022
ebeb091
Fix review comment
Junchao-Mellanox Jun 21, 2022
5b99253
Fix review comment
Junchao-Mellanox Jun 28, 2022
25cb579
Merge remote-tracking branch 'origin/master' into incre-port-update-1
Junchao-Mellanox Jul 1, 2022
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
Prev Previous commit
Next Next commit
Remove unused code
@Junchao-Mellanox
Junchao-Mellanox committed Jun 2, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
commit bbaadc5963a57dc5d3c90bce5057249938c259b4
5 changes: 0 additions & 5 deletions cfgmgr/portmgr.cpp
View file
Original file line number Diff line number Diff line change
@@ -11,11 +11,6 @@
using namespace std;
using namespace swss;

map<std::string, std::string> portDefaultConfig = {
{"mtu", DEFAULT_MTU_STR},
{"admin_status", DEFAULT_ADMIN_STATUS_STR}
};

PortMgr::PortMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, const vector<string> &tableNames) :
Orch(cfgDb, tableNames),
m_cfgPortTable(cfgDb, CFG_PORT_TABLE_NAME),
5 changes: 0 additions & 5 deletions tests/mock_tests/Makefile.am
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
FLEX_CTR_DIR = $(top_srcdir)/orchagent/flex_counter
DEBUG_CTR_DIR = $(top_srcdir)/orchagent/debug_counter
P4_ORCH_DIR = $(top_srcdir)/orchagent/p4orch
CFG_MGR_DIR = $(top_srcdir)/cfgmgr

<<<<<<< HEAD
INCLUDES = -I $(FLEX_CTR_DIR) -I $(DEBUG_CTR_DIR) -I $(top_srcdir)/lib -I $(CFG_MGR_DIR)
=======
INCLUDES = -I $(FLEX_CTR_DIR) -I $(DEBUG_CTR_DIR) -I $(top_srcdir)/lib -I $(top_srcdir)/cfgmgr
>>>>>>> origin/master

CFLAGS_SAI = -I /usr/include/sai

253 changes: 0 additions & 253 deletions tests/mock_tests/portsorch_ut.cpp
View file
Original file line number Diff line number Diff line change
@@ -970,259 +970,6 @@ namespace portsorch_test
sai_port_api = orig_port_api;
}

/*
* The scope of this test is to verify that auto negotiation related configuration
*/
/*
TEST_F(PortsOrchTest, PortAutoNegConfig)
{
Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME);

// Get SAI default ports to populate DB
auto ports = ut_helper::getInitialSaiPorts();

// Populate port table with SAI ports
for (const auto &it : ports)
{
portTable.set(it.first, it.second);
}

// Set PortConfigDone, PortInitDone
portTable.set("PortConfigDone", { { "count", to_string(ports.size()) } });
portTable.set("PortInitDone", { { "lanes", "0" } });

// refill consumer
gPortsOrch->addExistingData(&portTable);
// Apply configuration : create ports
static_cast<Orch *>(gPortsOrch)->doTask();

// save original api since we will spy
auto orig_port_api = sai_port_api;
sai_port_api = new sai_port_api_t();
memcpy(sai_port_api, orig_port_api, sizeof(*sai_port_api));

// mock SAI API sai_port_api->get_port_attribute
auto portSpy = SpyOn<SAI_API_PORT, SAI_OBJECT_TYPE_PORT>(&sai_port_api->set_port_attribute);
std::vector<shared_ptr<sai_attribute_t>> actual_attrs;
std::vector<uint32_t> adv_speeds;
std::vector<uint32_t> adv_types;
portSpy->callFake([&](sai_object_id_t oid, const sai_attribute_t * attr) -> sai_status_t {
if (attr->id == SAI_PORT_ATTR_AUTO_NEG_MODE ||
attr->id == SAI_PORT_ATTR_SPEED ||
attr->id == SAI_PORT_ATTR_ADVERTISED_SPEED ||
attr->id == SAI_PORT_ATTR_INTERFACE_TYPE ||
attr->id == SAI_PORT_ATTR_ADVERTISED_INTERFACE_TYPE)
{
// attr is a stack variable, need clone it
auto attr_copy = make_shared<sai_attribute_t>();
attr_copy->id = attr->id;
attr_copy->value = attr->value;
if (attr->id == SAI_PORT_ATTR_ADVERTISED_SPEED)
{
adv_speeds.assign(attr_copy->value.u32list.list, attr_copy->value.u32list.list + attr_copy->value.u32list.count);
}
else if (attr->id == SAI_PORT_ATTR_ADVERTISED_INTERFACE_TYPE)
{
adv_types.assign(attr_copy->value.u32list.list, attr_copy->value.u32list.list + attr_copy->value.u32list.count);
}
actual_attrs.push_back(attr_copy);
}
return (sai_status_t)SAI_STATUS_SUCCESS;
});

// Enable autoneg, verify autoneg, advertised speeds, advertised interface types are set to SAI
portTable.set("Ethernet0", { { "autoneg", "on" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_EQ(size_t(3), actual_attrs.size());
ASSERT_EQ(SAI_PORT_ATTR_AUTO_NEG_MODE, actual_attrs[0]->id);
ASSERT_TRUE(actual_attrs[0]->value.booldata);
ASSERT_EQ(SAI_PORT_ATTR_ADVERTISED_INTERFACE_TYPE, actual_attrs[1]->id);
ASSERT_EQ(0, actual_attrs[1]->value.u32list.count);
ASSERT_EQ(SAI_PORT_ATTR_ADVERTISED_SPEED, actual_attrs[2]->id);
ASSERT_EQ(0, actual_attrs[2]->value.u32list.count);

// Set speed/interface type while autoneg is enabled, verify the SAI API is not called
actual_attrs.clear();
portTable.set("Ethernet0", { { "interface_type", "CR" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_TRUE(actual_attrs.empty());
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
portTable.set("Ethernet0", { { "speed", "100000" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_TRUE(actual_attrs.empty());

// Set advertised speeds, advertised interface types while autoneg is enabled, verify the SAI API is called
portTable.set("Ethernet0", { { "adv_speeds", "50000,100000" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_FALSE(actual_attrs.empty());
ASSERT_EQ(SAI_PORT_ATTR_ADVERTISED_SPEED, actual_attrs[0]->id);
ASSERT_EQ(2, actual_attrs[0]->value.u32list.count);
ASSERT_EQ(size_t(2), adv_speeds.size());
ASSERT_EQ(50000, adv_speeds[0]);
ASSERT_EQ(100000, adv_speeds[1]);

actual_attrs.clear();
portTable.set("Ethernet0", { { "adv_interface_types", "CR,CR2" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_FALSE(actual_attrs.empty());
ASSERT_EQ(SAI_PORT_ATTR_ADVERTISED_INTERFACE_TYPE, actual_attrs[0]->id);
ASSERT_EQ(2, actual_attrs[0]->value.u32list.count);
ASSERT_EQ(size_t(2), adv_types.size());
ASSERT_EQ(static_cast<uint32_t>(SAI_PORT_INTERFACE_TYPE_CR), adv_types[0]);
ASSERT_EQ(static_cast<uint32_t>(SAI_PORT_INTERFACE_TYPE_CR2), adv_types[1]);

// Disable autoneg, verify interface type, speed are set to SAI
actual_attrs.clear();
portTable.set("Ethernet0", { { "autoneg", "off" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_EQ(size_t(3), actual_attrs.size());
ASSERT_EQ(SAI_PORT_ATTR_AUTO_NEG_MODE, actual_attrs[0]->id);
ASSERT_FALSE(actual_attrs[0]->value.booldata);
ASSERT_EQ(SAI_PORT_ATTR_INTERFACE_TYPE, actual_attrs[1]->id);
ASSERT_EQ(static_cast<uint32_t>(SAI_PORT_INTERFACE_TYPE_CR), actual_attrs[1]->value.u32);
ASSERT_EQ(SAI_PORT_ATTR_SPEED, actual_attrs[2]->id);
ASSERT_EQ(100000, actual_attrs[2]->value.u32);

// Set advertised speeds, advertised interface types while autoneg is disabled, verify the SAI API is not called
actual_attrs.clear();
portTable.set("Ethernet0", { { "adv_speeds", "50000" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_TRUE(actual_attrs.empty());
portTable.set("Ethernet0", { { "adv_interface_types", "CR4" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_TRUE(actual_attrs.empty());

// Set speed/interface type while autoneg is disabled, verify the SAI API is called
portTable.set("Ethernet0", { { "speed", "50000" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_FALSE(actual_attrs.empty());
ASSERT_EQ(SAI_PORT_ATTR_SPEED, actual_attrs[0]->id);
ASSERT_EQ(50000, actual_attrs[0]->value.u32);
actual_attrs.clear();
portTable.set("Ethernet0", { { "interface_type", "CR2" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_FALSE(actual_attrs.empty());
ASSERT_EQ(SAI_PORT_ATTR_INTERFACE_TYPE, actual_attrs[0]->id);
ASSERT_EQ(static_cast<uint32_t>(SAI_PORT_INTERFACE_TYPE_CR2), actual_attrs[0]->value.u32);

sai_port_api = orig_port_api;
}*/

/*
* The scope of this test is to verify that auto negotiation related configuration failed due to SAI return value
*/
/*
TEST_F(PortsOrchTest, PortAutoNegConfigNeg)
{
Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME);

// Get SAI default ports to populate DB
auto ports = ut_helper::getInitialSaiPorts();

// Populate port table with SAI ports
for (const auto &it : ports)
{
portTable.set(it.first, it.second);
}

// Set PortConfigDone, PortInitDone
portTable.set("PortConfigDone", { { "count", to_string(ports.size()) } });
portTable.set("PortInitDone", { { "lanes", "0" } });

// refill consumer
gPortsOrch->addExistingData(&portTable);
// Apply configuration : create ports
static_cast<Orch *>(gPortsOrch)->doTask();

// save original api since we will spy
auto orig_port_api = sai_port_api;
sai_port_api = new sai_port_api_t();
memcpy(sai_port_api, orig_port_api, sizeof(*sai_port_api));

// mock SAI API sai_port_api->get_port_attribute
auto portSpy = SpyOn<SAI_API_PORT, SAI_OBJECT_TYPE_PORT>(&sai_port_api->set_port_attribute);
auto set_autoneg_mode_return = SAI_STATUS_INVALID_ATTR_VALUE_0;
auto set_adv_speeds_return = SAI_STATUS_INVALID_ATTR_VALUE_0;
auto set_adv_types_return = SAI_STATUS_INVALID_ATTR_VALUE_0;
auto set_speed_return = SAI_STATUS_INVALID_ATTR_VALUE_0;
auto set_type_return = SAI_STATUS_INVALID_ATTR_VALUE_0;
std::map<sai_status_t, size_t> call_stats;
portSpy->callFake([&](sai_object_id_t oid, const sai_attribute_t * attr) -> sai_status_t {
switch(attr->id)
{
case SAI_PORT_ATTR_AUTO_NEG_MODE:
call_stats[SAI_PORT_ATTR_AUTO_NEG_MODE] += 1;
return (sai_status_t)set_autoneg_mode_return;
case SAI_PORT_ATTR_ADVERTISED_SPEED:
call_stats[SAI_PORT_ATTR_ADVERTISED_SPEED] += 1;
return (sai_status_t)set_adv_speeds_return;
case SAI_PORT_ATTR_ADVERTISED_INTERFACE_TYPE:
call_stats[SAI_PORT_ATTR_ADVERTISED_INTERFACE_TYPE] += 1;
return (sai_status_t)set_adv_types_return;
case SAI_PORT_ATTR_INTERFACE_TYPE:
call_stats[SAI_PORT_ATTR_INTERFACE_TYPE] += 1;
return (sai_status_t)set_type_return;
case SAI_PORT_ATTR_SPEED:
call_stats[SAI_PORT_ATTR_SPEED] += 1;
return (sai_status_t)set_speed_return;
default:
return (sai_status_t)SAI_STATUS_SUCCESS;
}
});

portTable.set("Ethernet0", { { "autoneg", "on" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_EQ(1, call_stats[SAI_PORT_ATTR_AUTO_NEG_MODE]);
ASSERT_FALSE(call_stats.count(SAI_PORT_ATTR_ADVERTISED_INTERFACE_TYPE));
ASSERT_FALSE(call_stats.count(SAI_PORT_ATTR_ADVERTISED_SPEED));

set_autoneg_mode_return = SAI_STATUS_SUCCESS;
portTable.set("Ethernet0", { { "autoneg", "on" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_EQ(1, call_stats[SAI_PORT_ATTR_ADVERTISED_INTERFACE_TYPE]);
ASSERT_FALSE(call_stats.count(SAI_PORT_ATTR_ADVERTISED_SPEED));

set_adv_types_return = SAI_STATUS_SUCCESS;
portTable.set("Ethernet0", { { "autoneg", "on" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_EQ(1, call_stats[SAI_PORT_ATTR_ADVERTISED_SPEED]);

set_autoneg_mode_return = SAI_STATUS_INVALID_ATTR_VALUE_0;
portTable.set("Ethernet0", { { "autoneg", "off" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_FALSE(call_stats.count(SAI_PORT_ATTR_INTERFACE_TYPE));
ASSERT_FALSE(call_stats.count(SAI_PORT_ATTR_SPEED));

set_autoneg_mode_return = SAI_STATUS_SUCCESS;
portTable.set("Ethernet0", { { "autoneg", "off" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_EQ(1, call_stats[SAI_PORT_ATTR_INTERFACE_TYPE]);
ASSERT_FALSE(call_stats.count(SAI_PORT_ATTR_SPEED));

set_type_return = SAI_STATUS_SUCCESS;
portTable.set("Ethernet0", { { "autoneg", "off" } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();
ASSERT_EQ(1, call_stats[SAI_PORT_ATTR_SPEED]);
}
*/

/*
* The scope of this test is to verify that LAG member is
* added to a LAG before any other object on LAG is created, like RIF, bridge port in warm mode.