-
Notifications
You must be signed in to change notification settings - Fork 543
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
[VRF]: submit vrf feature #943
Changes from 31 commits
f448a22
7f81135
91f1f00
5c2af28
53f2663
fa95f1e
03c0472
e456159
88e3f4f
91fdc69
7bfc853
e772a12
213bca9
771d5f1
90e8000
e47cbc4
2c64599
4874983
6a18696
40755bd
280c6d3
7617947
306a40e
340b125
c79df41
d2b835e
8396833
c584517
78c4001
778f324
8370676
3e762ac
1657824
bb61e52
ebd624d
bd187bf
a53c365
7f8e447
0652e1d
bbca607
d877f23
1618686
854a553
0f55a9b
7987477
2ee979f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ using namespace swss; | |
#define LAG_PREFIX "PortChannel" | ||
#define LOOPBACK_PREFIX "Loopback" | ||
#define VNET_PREFIX "Vnet" | ||
#define VRF_PREFIX "Vrf" | ||
|
||
IntfMgr::IntfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, const vector<string> &tableNames) : | ||
Orch(cfgDb, tableNames), | ||
|
@@ -58,7 +59,7 @@ void IntfMgr::setIntfIp(const string &alias, const string &opCmd, | |
} | ||
} | ||
|
||
void IntfMgr::setIntfVrf(const string &alias, const string vrfName) | ||
void IntfMgr::setIntfVrf(const string &alias, const string &vrfName) | ||
{ | ||
stringstream cmd; | ||
string res; | ||
|
@@ -71,7 +72,94 @@ void IntfMgr::setIntfVrf(const string &alias, const string vrfName) | |
{ | ||
cmd << IP_CMD << " link set " << alias << " nomaster"; | ||
} | ||
EXEC_WITH_ERROR_THROW(cmd.str(), res); | ||
int ret = swss::exec(cmd.str(), res); | ||
if (ret) | ||
{ | ||
SWSS_LOG_ERROR("Command '%s' failed with rc %d", cmd.str().c_str(), ret); | ||
} | ||
} | ||
|
||
void IntfMgr::addLoopbackIntf(const string &alias) | ||
{ | ||
stringstream cmd; | ||
string res; | ||
|
||
cmd << IP_CMD << " link add " << alias << " mtu 65536 type dummy && "; | ||
cmd << IP_CMD << " link set " << alias << " up"; | ||
int ret = swss::exec(cmd.str(), res); | ||
if (ret) | ||
{ | ||
SWSS_LOG_ERROR("Command '%s' failed with rc %d", cmd.str().c_str(), ret); | ||
} | ||
} | ||
|
||
void IntfMgr::delLoopbackIntf(const string &alias) | ||
{ | ||
stringstream cmd; | ||
string res; | ||
|
||
cmd << IP_CMD << " link del " << alias; | ||
int ret = swss::exec(cmd.str(), res); | ||
if (ret) | ||
{ | ||
SWSS_LOG_ERROR("Command '%s' failed with rc %d", cmd.str().c_str(), ret); | ||
} | ||
} | ||
|
||
int IntfMgr::getIntfIpCount(const string &alias) | ||
{ | ||
stringstream cmd; | ||
string res; | ||
|
||
/* query ip address of the device with master name, it is much faster */ | ||
cmd << IP_CMD << " address show " << alias | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please provide the expanded command as an example comment? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok |
||
<< " $(" << IP_CMD << " link show " << alias << " | grep -o 'master [^\\s]*')" | ||
<< " | grep inet | grep -v 'inet6 fe80:' | wc -l"; | ||
|
||
int ret = swss::exec(cmd.str(), res); | ||
if (ret) | ||
{ | ||
SWSS_LOG_ERROR("Command '%s' failed with rc %d", cmd.str().c_str(), ret); | ||
return 0; | ||
} | ||
|
||
return std::stoi(res); | ||
} | ||
|
||
bool IntfMgr::isIntfGeneralDone(const string &alias) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Name doesn't align with functionality. How about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't get a good name all along. Yours is better. |
||
{ | ||
vector<FieldValueTuple> temp; | ||
|
||
if (m_stateIntfTable.get(alias, temp)) | ||
{ | ||
SWSS_LOG_DEBUG("Intf %s is ready", alias.c_str()); | ||
return true; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
bool IntfMgr::isIntfChangeVrf(const string &alias, const string &vrfName) | ||
prsunny marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
vector<FieldValueTuple> temp; | ||
|
||
if (m_stateIntfTable.get(alias, temp)) | ||
{ | ||
for (auto idx : temp) | ||
{ | ||
const auto &field = fvField(idx); | ||
const auto &value = fvValue(idx); | ||
if (field == "vrf") | ||
{ | ||
if (value == vrfName) | ||
return false; | ||
else | ||
return true; | ||
} | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
|
||
bool IntfMgr::isIntfStateOk(const string &alias) | ||
|
@@ -102,6 +190,14 @@ bool IntfMgr::isIntfStateOk(const string &alias) | |
return true; | ||
} | ||
} | ||
else if (!alias.compare(0, strlen(VRF_PREFIX), VRF_PREFIX)) | ||
{ | ||
if (m_stateVrfTable.get(alias, temp)) | ||
{ | ||
SWSS_LOG_DEBUG("Vrf %s is ready", alias.c_str()); | ||
return true; | ||
} | ||
} | ||
else if (m_statePortTable.get(alias, temp)) | ||
{ | ||
SWSS_LOG_DEBUG("Port %s is ready", alias.c_str()); | ||
|
@@ -149,32 +245,38 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys, | |
return false; | ||
} | ||
|
||
// Set Interface VRF except for lo | ||
if (!is_lo) | ||
/* if to change vrf then skip */ | ||
if (isIntfChangeVrf(alias, vrf_name)) | ||
{ | ||
if (!vrf_name.empty()) | ||
{ | ||
setIntfVrf(alias, vrf_name); | ||
} | ||
m_appIntfTableProducer.set(alias, data); | ||
SWSS_LOG_ERROR("%s can not change to %s directly, skipping", alias.c_str(), vrf_name.c_str()); | ||
return true; | ||
} | ||
else | ||
if (is_lo) | ||
{ | ||
m_appIntfTableProducer.set("lo", data); | ||
addLoopbackIntf(alias); | ||
} | ||
if (!vrf_name.empty()) | ||
{ | ||
setIntfVrf(alias, vrf_name); | ||
} | ||
m_appIntfTableProducer.set(alias, data); | ||
m_stateIntfTable.hset(alias, "vrf", vrf_name); | ||
} | ||
else if (op == DEL_COMMAND) | ||
{ | ||
// Set Interface VRF except for lo | ||
if (!is_lo) | ||
/* make sure all ip addresses associated with interface are removed, otherwise these ip address would | ||
be set with global vrf and it may cause ip address confliction. */ | ||
if (getIntfIpCount(alias)) | ||
{ | ||
setIntfVrf(alias, ""); | ||
m_appIntfTableProducer.del(alias); | ||
return false; | ||
} | ||
else | ||
setIntfVrf(alias, ""); | ||
if (is_lo) | ||
{ | ||
m_appIntfTableProducer.del("lo"); | ||
delLoopbackIntf(alias); | ||
} | ||
m_appIntfTableProducer.del(alias); | ||
m_stateIntfTable.del(alias); | ||
} | ||
else | ||
{ | ||
|
@@ -192,26 +294,21 @@ bool IntfMgr::doIntfAddrTask(const vector<string>& keys, | |
|
||
string alias(keys[0]); | ||
IpPrefix ip_prefix(keys[1]); | ||
bool is_lo = !alias.compare(0, strlen(LOOPBACK_PREFIX), LOOPBACK_PREFIX); | ||
string appKey = (is_lo ? "lo" : keys[0]) + ":" + keys[1]; | ||
string appKey = keys[0] + ":" + keys[1]; | ||
|
||
if (op == SET_COMMAND) | ||
{ | ||
/* | ||
* Don't proceed if port/LAG/VLAN is not ready yet. | ||
* Don't proceed if port/LAG/VLAN and intfGeneral are not ready yet. | ||
* The pending task will be checked periodically and retried. | ||
*/ | ||
if (!isIntfStateOk(alias)) | ||
if (!isIntfStateOk(alias) || !isIntfGeneralDone(alias)) | ||
{ | ||
SWSS_LOG_DEBUG("Interface is not ready, skipping %s", alias.c_str()); | ||
return false; | ||
} | ||
|
||
// Set Interface IP except for lo | ||
if (!is_lo) | ||
{ | ||
setIntfIp(alias, "add", ip_prefix); | ||
} | ||
setIntfIp(alias, "add", ip_prefix); | ||
|
||
std::vector<FieldValueTuple> fvVector; | ||
FieldValueTuple f("family", ip_prefix.isV4() ? IPV4_NAME : IPV6_NAME); | ||
|
@@ -224,11 +321,8 @@ bool IntfMgr::doIntfAddrTask(const vector<string>& keys, | |
} | ||
else if (op == DEL_COMMAND) | ||
{ | ||
// Set Interface IP except for lo | ||
if (!is_lo) | ||
{ | ||
setIntfIp(alias, "del", ip_prefix); | ||
} | ||
setIntfIp(alias, "del", ip_prefix); | ||
|
||
m_appIntfTableProducer.del(appKey); | ||
m_stateIntfTable.del(keys[0] + state_db_key_delimiter + keys[1]); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
|
||
#define VRF_TABLE_START 1001 | ||
#define VRF_TABLE_END 2000 | ||
#define OBJ_PREFIX "OBJ" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a new schema name?. I dont find this in HLD. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a new vrf state. To fix vrf delete problem, vrforch set a state entry with prefix OBJ. |
||
|
||
using namespace swss; | ||
|
||
|
@@ -63,6 +64,18 @@ VrfMgr::VrfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, con | |
break; | ||
} | ||
} | ||
|
||
cmd.str(""); | ||
cmd.clear(); | ||
cmd << IP_CMD << " rule | grep '^0:'"; | ||
if (swss::exec(cmd.str(), res) == 0) | ||
{ | ||
cmd.str(""); | ||
cmd.clear(); | ||
cmd << IP_CMD << " rule add pref 1001 table local && " << IP_CMD << " rule del pref 0 && " | ||
prsunny marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you use a variable instead of hardcoding 1001? Suggest to leverage There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think VRF_TABLE_START is not suitabale. I will create a new name. |
||
<< IP_CMD << " -6 rule add pref 1001 table local && " << IP_CMD << " -6 rule del pref 0"; | ||
EXEC_WITH_ERROR_THROW(cmd.str(), res); | ||
} | ||
} | ||
|
||
uint32_t VrfMgr::getFreeTable(void) | ||
|
@@ -139,6 +152,20 @@ bool VrfMgr::setLink(const string& vrfName) | |
return true; | ||
} | ||
|
||
bool VrfMgr::isVrfObjExist(const string& vrfName) | ||
{ | ||
vector<FieldValueTuple> temp; | ||
string key = string(OBJ_PREFIX) + state_db_key_delimiter + vrfName; | ||
|
||
if (m_stateVrfTable.get(key, temp)) | ||
{ | ||
SWSS_LOG_DEBUG("Vrf %s object exist", vrfName.c_str()); | ||
return true; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
void VrfMgr::doTask(Consumer &consumer) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
@@ -173,20 +200,37 @@ void VrfMgr::doTask(Consumer &consumer) | |
} | ||
else if (op == DEL_COMMAND) | ||
{ | ||
if (!delLink(vrfName)) | ||
vector<FieldValueTuple> temp; | ||
|
||
if (m_stateVrfTable.get(vrfName, temp)) | ||
{ | ||
SWSS_LOG_ERROR("Failed to remove vrf netdev %s", vrfName.c_str()); | ||
if (!isVrfObjExist(vrfName)) | ||
{ | ||
it++; | ||
continue; | ||
} | ||
|
||
m_stateVrfTable.del(vrfName); | ||
|
||
if (consumer.getTableName() == CFG_VRF_TABLE_NAME) | ||
{ | ||
m_appVrfTableProducer.del(vrfName); | ||
} | ||
else | ||
{ | ||
m_appVnetTableProducer.del(vrfName); | ||
} | ||
} | ||
|
||
m_stateVrfTable.del(vrfName); | ||
|
||
if (consumer.getTableName() == CFG_VRF_TABLE_NAME) | ||
if (isVrfObjExist(vrfName)) | ||
{ | ||
m_appVrfTableProducer.del(vrfName); | ||
it++; | ||
continue; | ||
} | ||
else | ||
|
||
if (!delLink(vrfName)) | ||
{ | ||
m_appVnetTableProducer.del(vrfName); | ||
SWSS_LOG_ERROR("Failed to remove vrf netdev %s", vrfName.c_str()); | ||
} | ||
|
||
SWSS_LOG_NOTICE("Removed vrf netdev %s", vrfName.c_str()); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please define
65535
and use variable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, maybe always keep the max value.