Skip to content

Commit 7126857

Browse files
Junchao-MellanoxJunchao-Mellanox
authored
Port configuration incremental update support (sonic-net#2305)
*portsyncd no longer handle port configuration change and put it to APP DB *Implement incremental configuration change in portmgr *Adjust portsorch to meet incremental configuration change requirement
1 parent 7175245 commit 7126857

File tree

8 files changed

+205
-122
lines changed

8 files changed

+205
-122
lines changed

‎cfgmgr/portmgr.cpp

Lines changed: 46 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -31,29 +31,9 @@ bool PortMgr::setPortMtu(const string &alias, const string &mtu)
3131

3232
// Set the port MTU in application database to update both
3333
// the port MTU and possibly the port based router interface MTU
34-
vector<FieldValueTuple> fvs;
35-
FieldValueTuple fv("mtu", mtu);
36-
fvs.push_back(fv);
37-
m_appPortTable.set(alias, fvs);
38-
39-
return true;
34+
return writeConfigToAppDb(alias, "mtu", mtu);
4035
}
4136

42-
bool PortMgr::setPortTpid(const string &alias, const string &tpid)
43-
{
44-
stringstream cmd;
45-
string res;
46-
47-
// Set the port TPID in application database to update port TPID
48-
vector<FieldValueTuple> fvs;
49-
FieldValueTuple fv("tpid", tpid);
50-
fvs.push_back(fv);
51-
m_appPortTable.set(alias, fvs);
52-
53-
return true;
54-
}
55-
56-
5737
bool PortMgr::setPortAdminStatus(const string &alias, const bool up)
5838
{
5939
stringstream cmd;
@@ -63,23 +43,7 @@ bool PortMgr::setPortAdminStatus(const string &alias, const bool up)
6343
cmd << IP_CMD << " link set dev " << shellquote(alias) << (up ? " up" : " down");
6444
EXEC_WITH_ERROR_THROW(cmd.str(), res);
6545

66-
vector<FieldValueTuple> fvs;
67-
FieldValueTuple fv("admin_status", (up ? "up" : "down"));
68-
fvs.push_back(fv);
69-
m_appPortTable.set(alias, fvs);
70-
71-
return true;
72-
}
73-
74-
bool PortMgr::setPortLearnMode(const string &alias, const string &learn_mode)
75-
{
76-
// Set the port MAC learn mode in application database
77-
vector<FieldValueTuple> fvs;
78-
FieldValueTuple fv("learn_mode", learn_mode);
79-
fvs.push_back(fv);
80-
m_appPortTable.set(alias, fvs);
81-
82-
return true;
46+
return writeConfigToAppDb(alias, "admin_status", (up ? "up" : "down"));
8347
}
8448

8549
bool PortMgr::isPortStateOk(const string &alias)
@@ -117,14 +81,14 @@ void PortMgr::doTask(Consumer &consumer)
11781

11882
if (op == SET_COMMAND)
11983
{
120-
if (!isPortStateOk(alias))
121-
{
122-
SWSS_LOG_INFO("Port %s is not ready, pending...", alias.c_str());
123-
it++;
124-
continue;
125-
}
84+
/* portOk=true indicates that the port has been created in kernel.
85+
* We should not call any ip command if portOk=false. However, it is
86+
* valid to put port configuration to APP DB which will trigger port creation in kernel.
87+
*/
88+
bool portOk = isPortStateOk(alias);
12689

127-
string admin_status, mtu, learn_mode, tpid;
90+
string admin_status, mtu;
91+
std::vector<FieldValueTuple> field_values;
12892

12993
bool configured = (m_portList.find(alias) != m_portList.end());
13094

@@ -138,6 +102,11 @@ void PortMgr::doTask(Consumer &consumer)
138102

139103
m_portList.insert(alias);
140104
}
105+
else if (!portOk)
106+
{
107+
it++;
108+
continue;
109+
}
141110

142111
for (auto i : kfvFieldsValues(t))
143112
{
@@ -149,38 +118,42 @@ void PortMgr::doTask(Consumer &consumer)
149118
{
150119
admin_status = fvValue(i);
151120
}
152-
else if (fvField(i) == "learn_mode")
153-
{
154-
learn_mode = fvValue(i);
155-
}
156-
else if (fvField(i) == "tpid")
121+
else
157122
{
158-
tpid = fvValue(i);
123+
field_values.emplace_back(i);
159124
}
160125
}
161126

162-
if (!mtu.empty())
127+
for (auto &entry : field_values)
163128
{
164-
setPortMtu(alias, mtu);
165-
SWSS_LOG_NOTICE("Configure %s MTU to %s", alias.c_str(), mtu.c_str());
129+
writeConfigToAppDb(alias, fvField(entry), fvValue(entry));
130+
SWSS_LOG_NOTICE("Configure %s %s to %s", alias.c_str(), fvField(entry).c_str(), fvValue(entry).c_str());
166131
}
167132

168-
if (!admin_status.empty())
133+
if (!portOk)
169134
{
170-
setPortAdminStatus(alias, admin_status == "up");
171-
SWSS_LOG_NOTICE("Configure %s admin status to %s", alias.c_str(), admin_status.c_str());
135+
SWSS_LOG_INFO("Port %s is not ready, pending...", alias.c_str());
136+
137+
writeConfigToAppDb(alias, "mtu", mtu);
138+
writeConfigToAppDb(alias, "admin_status", admin_status);
139+
field_values.clear();
140+
field_values.emplace_back("mtu", mtu);
141+
field_values.emplace_back("admin_status", admin_status);
142+
it->second = KeyOpFieldsValuesTuple{alias, SET_COMMAND, field_values};
143+
it++;
144+
continue;
172145
}
173146

174-
if (!learn_mode.empty())
147+
if (!mtu.empty())
175148
{
176-
setPortLearnMode(alias, learn_mode);
177-
SWSS_LOG_NOTICE("Configure %s MAC learn mode to %s", alias.c_str(), learn_mode.c_str());
149+
setPortMtu(alias, mtu);
150+
SWSS_LOG_NOTICE("Configure %s MTU to %s", alias.c_str(), mtu.c_str());
178151
}
179152

180-
if (!tpid.empty())
153+
if (!admin_status.empty())
181154
{
182-
setPortTpid(alias, tpid);
183-
SWSS_LOG_NOTICE("Configure %s TPID to %s", alias.c_str(), tpid.c_str());
155+
setPortAdminStatus(alias, admin_status == "up");
156+
SWSS_LOG_NOTICE("Configure %s admin status to %s", alias.c_str(), admin_status.c_str());
184157
}
185158
}
186159
else if (op == DEL_COMMAND)
@@ -193,3 +166,13 @@ void PortMgr::doTask(Consumer &consumer)
193166
it = consumer.m_toSync.erase(it);
194167
}
195168
}
169+
170+
bool PortMgr::writeConfigToAppDb(const std::string &alias, const std::string &field, const std::string &value)
171+
{
172+
vector<FieldValueTuple> fvs;
173+
FieldValueTuple fv(field, value);
174+
fvs.push_back(fv);
175+
m_appPortTable.set(alias, fvs);
176+
177+
return true;
178+
}

‎cfgmgr/portmgr.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,9 @@ class PortMgr : public Orch
2929
std::set<std::string> m_portList;
3030

3131
void doTask(Consumer &consumer);
32+
bool writeConfigToAppDb(const std::string &alias, const std::string &field, const std::string &value);
3233
bool setPortMtu(const std::string &alias, const std::string &mtu);
33-
bool setPortTpid(const std::string &alias, const std::string &tpid);
3434
bool setPortAdminStatus(const std::string &alias, const bool up);
35-
bool setPortLearnMode(const std::string &alias, const std::string &learn_mode);
3635
bool isPortStateOk(const std::string &alias);
3736
};
3837

‎orchagent/port.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,12 @@ class Port
8787
UNKNOWN
8888
} ;
8989

90+
enum AutoNegMode {
91+
AUTONEG_NOT_SET = -1,
92+
AUTONEG_OFF = 0,
93+
AUTONEG_ON = 1
94+
};
95+
9096
Port() {};
9197
Port(std::string alias, Type type) :
9298
m_alias(alias), m_type(type) {};
@@ -112,7 +118,7 @@ class Port
112118
uint32_t m_mtu = DEFAULT_MTU;
113119
uint32_t m_speed = 0; // Mbps
114120
std::string m_learn_mode = "hardware";
115-
int m_autoneg = -1; // -1 means not set, 0 = disabled, 1 = enabled
121+
AutoNegMode m_autoneg = Port::AutoNegMode::AUTONEG_NOT_SET;
116122
bool m_admin_state_up = false;
117123
bool m_init = false;
118124
bool m_l3_vni = false;
@@ -148,7 +154,7 @@ class Port
148154
uint32_t m_up_member_count = 0;
149155
uint32_t m_maximum_headroom = 0;
150156
std::vector<uint32_t> m_adv_speeds;
151-
sai_port_interface_type_t m_interface_type;
157+
sai_port_interface_type_t m_interface_type = SAI_PORT_INTERFACE_TYPE_NONE;
152158
std::vector<uint32_t> m_adv_interface_types;
153159
bool m_mpls = false;
154160
/*

‎orchagent/portsorch.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2966,6 +2966,11 @@ void PortsOrch::doPortTask(Consumer &consumer)
29662966
}
29672967
else
29682968
{
2969+
if (admin_status.empty())
2970+
{
2971+
admin_status = p.m_admin_state_up ? "up" : "down";
2972+
}
2973+
29692974
if (!an_str.empty())
29702975
{
29712976
if (autoneg_mode_map.find(an_str) == autoneg_mode_map.end())
@@ -3008,7 +3013,7 @@ void PortsOrch::doPortTask(Consumer &consumer)
30083013
continue;
30093014
}
30103015
SWSS_LOG_NOTICE("Set port %s AutoNeg from %d to %d", alias.c_str(), p.m_autoneg, an);
3011-
p.m_autoneg = an;
3016+
p.m_autoneg = static_cast<swss::Port::AutoNegMode>(an);
30123017
m_portList[alias] = p;
30133018
}
30143019
}

‎portsyncd/portsyncd.cpp

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,12 @@ void usage()
4545
void handlePortConfigFile(ProducerStateTable &p, string file, bool warm);
4646
void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb, bool warm);
4747
void handleVlanIntfFile(string file);
48-
void handlePortConfig(ProducerStateTable &p, map<string, KeyOpFieldsValuesTuple> &port_cfg_map);
4948
void checkPortInitDone(DBConnector *appl_db);
5049

5150
int main(int argc, char **argv)
5251
{
5352
Logger::linkToDbNative("portsyncd");
5453
int opt;
55-
map<string, KeyOpFieldsValuesTuple> port_cfg_map;
5654

5755
while ((opt = getopt(argc, argv, "v:h")) != -1 )
5856
{
@@ -71,7 +69,6 @@ int main(int argc, char **argv)
7169
DBConnector appl_db("APPL_DB", 0);
7270
DBConnector state_db("STATE_DB", 0);
7371
ProducerStateTable p(&appl_db, APP_PORT_TABLE_NAME);
74-
SubscriberStateTable portCfg(&cfgDb, CFG_PORT_TABLE_NAME);
7572

7673
WarmStart::initialize("portsyncd", "swss");
7774
WarmStart::checkWarmStart("portsyncd", "swss");
@@ -93,7 +90,6 @@ int main(int argc, char **argv)
9390
NetDispatcher::getInstance().registerMessageHandler(RTM_DELLINK, &sync);
9491

9592
s.addSelectable(&netlink);
96-
s.addSelectable(&portCfg);
9793

9894
while (true)
9995
{
@@ -135,28 +131,6 @@ int main(int argc, char **argv)
135131

136132
g_init = true;
137133
}
138-
if (!port_cfg_map.empty())
139-
{
140-
handlePortConfig(p, port_cfg_map);
141-
}
142-
}
143-
else if (temps == (Selectable *)&portCfg)
144-
{
145-
std::deque<KeyOpFieldsValuesTuple> entries;
146-
portCfg.pops(entries);
147-
148-
for (auto entry: entries)
149-
{
150-
string key = kfvKey(entry);
151-
152-
if (port_cfg_map.find(key) != port_cfg_map.end())
153-
{
154-
/* For now we simply drop previous pending port config */
155-
port_cfg_map.erase(key);
156-
}
157-
port_cfg_map[key] = entry;
158-
}
159-
handlePortConfig(p, port_cfg_map);
160134
}
161135
else
162136
{
@@ -225,31 +199,3 @@ void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb, boo
225199
}
226200

227201
}
228-
229-
void handlePortConfig(ProducerStateTable &p, map<string, KeyOpFieldsValuesTuple> &port_cfg_map)
230-
{
231-
auto it = port_cfg_map.begin();
232-
while (it != port_cfg_map.end())
233-
{
234-
KeyOpFieldsValuesTuple entry = it->second;
235-
string key = kfvKey(entry);
236-
string op = kfvOp(entry);
237-
auto values = kfvFieldsValues(entry);
238-
239-
/* only push down port config when port is not in hostif create pending state */
240-
if (g_portSet.find(key) == g_portSet.end())
241-
{
242-
/* No support for port delete yet */
243-
if (op == SET_COMMAND)
244-
{
245-
p.set(key, values);
246-
}
247-
248-
it = port_cfg_map.erase(it);
249-
}
250-
else
251-
{
252-
it++;
253-
}
254-
}
255-
}

‎tests/mock_tests/Makefile.am

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ tests_SOURCES = aclorch_ut.cpp \
3939
mock_table.cpp \
4040
mock_hiredis.cpp \
4141
mock_redisreply.cpp \
42+
mock_shell_command.cpp \
4243
bulker_ut.cpp \
44+
portmgr_ut.cpp \
4345
fake_response_publisher.cpp \
4446
swssnet_ut.cpp \
4547
flowcounterrouteorch_ut.cpp \
@@ -98,6 +100,7 @@ tests_SOURCES = aclorch_ut.cpp \
98100
$(top_srcdir)/orchagent/bfdorch.cpp \
99101
$(top_srcdir)/orchagent/srv6orch.cpp \
100102
$(top_srcdir)/orchagent/nvgreorch.cpp \
103+
$(top_srcdir)/cfgmgr/portmgr.cpp \
101104
$(top_srcdir)/cfgmgr/buffermgrdyn.cpp
102105

103106
tests_SOURCES += $(FLEX_CTR_DIR)/flex_counter_manager.cpp $(FLEX_CTR_DIR)/flex_counter_stat_manager.cpp $(FLEX_CTR_DIR)/flow_counter_handler.cpp $(FLEX_CTR_DIR)/flowcounterrouteorch.cpp

‎tests/mock_tests/mock_shell_command.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#include <string>
2+
#include <vector>
3+
4+
int mockCmdReturn = 0;
5+
std::string mockCmdStdcout = "";
6+
std::vector<std::string> mockCallArgs;
7+
8+
namespace swss {
9+
int exec(const std::string &cmd, std::string &stdout)
10+
{
11+
mockCallArgs.push_back(cmd);
12+
stdout = mockCmdStdcout;
13+
return mockCmdReturn;
14+
}
15+
}

0 commit comments

Comments
 (0)