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

Added support for EVPN L3 VXLAN as described in the PR Azure/SONiC#437 #1267

Merged
merged 9 commits into from
Dec 18, 2020
Merged

Added support for EVPN L3 VXLAN as described in the PR Azure/SONiC#437 #1267

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
16c7f0a
Added support for EVPN L3 VXLAN as described in the PR Azure/SONiC#437
tapashdas Apr 21, 2020
11cf178
Added Unit Test Fixes and code to handle overlay ECMP group
tapashdas Jun 24, 2020
2171210
Merge branch 'master' into evpn_L3_vxlan
tapashdas Oct 10, 2020
b0c9342
Updated Review Comments. And fixed master branch merge issues.
tapashdas Oct 10, 2020
9b3fe9d
Merge branch 'master' into evpn_L3_vxlan
tapashdas Dec 2, 2020
8419ee2
Resolved Code merge issues after PR1264 merge.
tapashdas Dec 2, 2020
cd8c5df
Fix for mirror VS test failures.
tapashdas Dec 6, 2020
f542d8f
Updated Review Comments.
tapashdas Dec 15, 2020
c9ccfe4
Updated review comments.
tapashdas Dec 18, 2020
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
217 changes: 201 additions & 16 deletions cfgmgr/vrfmgr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ VrfMgr::VrfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, con
Orch(cfgDb, tableNames),
m_appVrfTableProducer(appDb, APP_VRF_TABLE_NAME),
m_appVnetTableProducer(appDb, APP_VNET_TABLE_NAME),
m_appVxlanVrfTableProducer(appDb, APP_VXLAN_VRF_TABLE_NAME),
m_stateVrfTable(stateDb, STATE_VRF_TABLE_NAME),
m_stateVrfObjectTable(stateDb, STATE_VRF_OBJECT_TABLE_NAME)
{
Expand Down Expand Up @@ -203,23 +204,32 @@ void VrfMgr::doTask(Consumer &consumer)
string op = kfvOp(t);
if (op == SET_COMMAND)
{
if (!setLink(vrfName))
if (consumer.getTableName() == CFG_VXLAN_EVPN_NVO_TABLE_NAME)
{
SWSS_LOG_ERROR("Failed to create vrf netdev %s", vrfName.c_str());
}

vector<FieldValueTuple> fvVector;
fvVector.emplace_back("state", "ok");
m_stateVrfTable.set(vrfName, fvVector);

SWSS_LOG_NOTICE("Created vrf netdev %s", vrfName.c_str());
if (consumer.getTableName() == CFG_VRF_TABLE_NAME)
{
m_appVrfTableProducer.set(vrfName, kfvFieldsValues(t));
doVrfEvpnNvoAddTask(t);
}
else
{
m_appVnetTableProducer.set(vrfName, kfvFieldsValues(t));
if (!setLink(vrfName))
{
SWSS_LOG_ERROR("Failed to create vrf netdev %s", vrfName.c_str());
Copy link

Choose a reason for hiding this comment

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

How would we recover from Vrf creation failures?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is existing vrfmgr code in base/master branch. Not changed due to EVPN changes. Need to address this separately.

}

vector<FieldValueTuple> fvVector;
fvVector.emplace_back("state", "ok");
m_stateVrfTable.set(vrfName, fvVector);

SWSS_LOG_NOTICE("Created vrf netdev %s", vrfName.c_str());
if (consumer.getTableName() == CFG_VRF_TABLE_NAME)
{
m_appVrfTableProducer.set(vrfName, kfvFieldsValues(t));

doVrfVxlanTableCreateTask (t);
}
else
{
m_appVnetTableProducer.set(vrfName, kfvFieldsValues(t));
}
}
}
else if (op == DEL_COMMAND)
Expand All @@ -229,7 +239,11 @@ void VrfMgr::doTask(Consumer &consumer)
* Now state VRF_TABLE|Vrf represent vrf exist in appDB, if it exist vrf device is always effective.
* VRFOrch add/del state VRF_OBJECT_TABLE|Vrf to represent object existence. VNETOrch is not do so now.
*/
if (consumer.getTableName() == CFG_VRF_TABLE_NAME)
if (consumer.getTableName() == CFG_VXLAN_EVPN_NVO_TABLE_NAME)
{
doVrfEvpnNvoDelTask (t);
}
else if (consumer.getTableName() == CFG_VRF_TABLE_NAME)
{
vector<FieldValueTuple> temp;

Expand All @@ -242,6 +256,7 @@ void VrfMgr::doTask(Consumer &consumer)
continue;
}

doVrfVxlanTableRemoveTask (t);
m_appVrfTableProducer.del(vrfName);
m_stateVrfTable.del(vrfName);
}
Expand All @@ -258,9 +273,12 @@ void VrfMgr::doTask(Consumer &consumer)
m_stateVrfTable.del(vrfName);
}

if (!delLink(vrfName))
if (consumer.getTableName() != CFG_VXLAN_EVPN_NVO_TABLE_NAME)
{
SWSS_LOG_ERROR("Failed to remove vrf netdev %s", vrfName.c_str());
if (!delLink(vrfName))
{
SWSS_LOG_ERROR("Failed to remove vrf netdev %s", vrfName.c_str());
}
}

SWSS_LOG_NOTICE("Removed vrf netdev %s", vrfName.c_str());
Expand All @@ -273,3 +291,170 @@ void VrfMgr::doTask(Consumer &consumer)
it = consumer.m_toSync.erase(it);
}
}

bool VrfMgr::doVrfEvpnNvoAddTask(const KeyOpFieldsValuesTuple & t)
{
SWSS_LOG_ENTER();
string tunnel_name = "";
const vector<FieldValueTuple>& data = kfvFieldsValues(t);
for (auto idx : data)
{
const auto &field = fvField(idx);
const auto &value = fvValue(idx);

if (field == "source_vtep")
{
tunnel_name = value;
}
}

if (m_evpnVxlanTunnel.empty())
{
m_evpnVxlanTunnel = tunnel_name;
VrfVxlanTableSync(true);
}

SWSS_LOG_NOTICE("Added evpn nvo tunnel %s", m_evpnVxlanTunnel.c_str());
return true;
}

bool VrfMgr::doVrfEvpnNvoDelTask(const KeyOpFieldsValuesTuple & t)
{
SWSS_LOG_ENTER();

if (!m_evpnVxlanTunnel.empty())
{
VrfVxlanTableSync(false);
m_evpnVxlanTunnel = "";
}

SWSS_LOG_NOTICE("Removed evpn nvo tunnel %s", m_evpnVxlanTunnel.c_str());
Copy link

Choose a reason for hiding this comment

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

If m_evpnVxlanTunnel is empty, this log wouldnt print anything.

If m_evpnVxlanTunnel is not empty
m_evpnVxlanTunnel will be empty after m_evpnVxlanTunnel = "";

Finally this log wouldnt print anything for m_evpnVxlanTunnel ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. Will update.

return true;
}

bool VrfMgr::doVrfVxlanTableCreateTask(const KeyOpFieldsValuesTuple & t)
{
SWSS_LOG_ENTER();

auto vrf_name = kfvKey(t);
uint32_t vni = 0, old_vni = 0;
string s_vni = "";
bool add = true;

const vector<FieldValueTuple>& data = kfvFieldsValues(t);
for (auto idx : data)
{
const auto &field = fvField(idx);
const auto &value = fvValue(idx);

if (field == "vni")
{
s_vni = value;
vni = static_cast<uint32_t>(stoul(value));
Copy link
Collaborator

Choose a reason for hiding this comment

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

I dont think this vni conversion is required. I can't find it being used other than "if" check. You could always check if vni == ""? Suggest avoid the extra integer conversion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Storing integer value in internal data structure vrf_vni_map_table_. So converted to integer.

}
}

old_vni = getVRFmappedVNI(vrf_name);
SWSS_LOG_NOTICE("VRF VNI map update vrf %s, vni %d, old_vni %d", vrf_name.c_str(), vni, old_vni);
if (vni != old_vni)
Copy link
Collaborator

Choose a reason for hiding this comment

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

So what triggers this value change? VRF_TABLE is part of config_db right? so is this some user config that they change the vni value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. VRF VNI Map is stored in CONFIG DB VRF_TABLE, This takes care of scenario if VRF VNI mapping is changed.

{
if (vni == 0)
{
m_vrfVniMapTable.erase(vrf_name);
s_vni = to_string(old_vni);
add = false;
}
else
{
m_vrfVniMapTable[vrf_name] = vni;
}

}

if ((vni == 0) && add)
Copy link

Choose a reason for hiding this comment

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

What will be this case, where VNI is 0 and is still an 'add' condition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to take care of the scenario when VRF is configured but VRF to VNI Mapping is not configured.

{
return true;
}

SWSS_LOG_NOTICE("VRF VNI map update vrf %s, s_vni %s, add %d", vrf_name.c_str(), s_vni.c_str(), add);
doVrfVxlanTableUpdate(vrf_name, s_vni, add);
Copy link

Choose a reason for hiding this comment

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

There could be VNIs which are only mapped to VLANs but not VRFs. How do we make sure that, we are using unique VNIs here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

VNI can be both L2 + L3. For the case where Vlan associated with VNI is tenant Vlan as well.
So L3 VNI need not be unique.

Copy link

Choose a reason for hiding this comment

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

Didnt quite understand this. An L2 VNI between 2 leafs for a given Tenant, could also be a L3 VNI for different Tenant?
My understanding is that, either L2 or L3 VNI, they have to be unique in a given system.

Copy link

Choose a reason for hiding this comment

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

VLAN-X +VNI-X -- L2 VXLAN
VLAN-Y + VNI-Y + VRF-Y -- L3 VXLAN (for TYPE-5)
Where do we verify if VNI-X and VNI-Y are not overlapping?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently it is taken care in CLI Validation.

Copy link
Contributor

Choose a reason for hiding this comment

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

@tapashdas If the configuration is done directly in config_db, we would need to check the validity of the global VLAN/VRF-->VNI mapping. It will be better to maintain or validate global vlan/vrf<-->VNI mapping in the cfgmgr.

Copy link
Contributor

Choose a reason for hiding this comment

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

@tapashdas In vxlanmgr.cpp, if the vni is already mapped to a vlan, mapper entry is rejected. But here the mapping entry is updated. Can we reject the config similar to vxlanmgr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok. Will do.

return true;
}

bool VrfMgr::doVrfVxlanTableRemoveTask(const KeyOpFieldsValuesTuple & t)
{
SWSS_LOG_ENTER();

auto vrf_name = kfvKey(t);
uint32_t vni = 0;
string s_vni = "";

vni = getVRFmappedVNI(vrf_name);
SWSS_LOG_NOTICE("VRF VNI map remove vrf %s, vni %d", vrf_name.c_str(), vni);
if (vni != 0)
{
s_vni = to_string(vni);
doVrfVxlanTableUpdate(vrf_name, s_vni, false);
m_vrfVniMapTable.erase(vrf_name);
}

return true;
}

bool VrfMgr::doVrfVxlanTableUpdate(const string& vrf_name, const string& vni, bool add)
{
SWSS_LOG_ENTER();
string key;

if (m_evpnVxlanTunnel.empty())
{
SWSS_LOG_NOTICE("NVO Tunnel not present. vrf %s, vni %s, add %d", vrf_name.c_str(), vni.c_str(), add);
return false;
}

key = m_evpnVxlanTunnel + ":" + "evpn_map_" + vni + "_" + vrf_name;

std::vector<FieldValueTuple> fvVector;
FieldValueTuple v1("vni", vni);
FieldValueTuple v2("vrf", vrf_name);
fvVector.push_back(v1);
fvVector.push_back(v2);

SWSS_LOG_NOTICE("VRF VNI map table update vrf %s, vni %s, add %d", vrf_name.c_str(), vni.c_str(), add);
if (add)
{
m_appVxlanVrfTableProducer.set(key, fvVector);
}
else
{
m_appVxlanVrfTableProducer.del(key);
}

return true;
}

void VrfMgr::VrfVxlanTableSync(bool add)
{
SWSS_LOG_ENTER();
string s_vni = "";

for (auto itr : m_vrfVniMapTable)
{
s_vni = to_string(itr.second);
SWSS_LOG_NOTICE("vrf %s, vni %s, add %d", (itr.first).c_str(), s_vni.c_str(), add);
doVrfVxlanTableUpdate(itr.first, s_vni, add);
}
}

uint32_t VrfMgr::getVRFmappedVNI(const std::string& vrf_name)
{
if (m_vrfVniMapTable.find(vrf_name) != std::end(m_vrfVniMapTable))
{
return m_vrfVniMapTable.at(vrf_name);
}
else
{
return 0;
}
}

14 changes: 13 additions & 1 deletion cfgmgr/vrfmgr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,16 @@ using namespace std;

namespace swss {

typedef std::unordered_map<std::string, uint32_t> VRFNameVNIMapTable;

class VrfMgr : public Orch
{
public:
VrfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, const std::vector<std::string> &tableNames);
using Orch::doTask;
std::string m_evpnVxlanTunnel;

uint32_t getVRFmappedVNI(const std::string& vrf_name);

private:
bool delLink(const std::string& vrfName);
Expand All @@ -25,13 +30,20 @@ class VrfMgr : public Orch
void recycleTable(uint32_t table);
uint32_t getFreeTable(void);
void handleVnetConfigSet(KeyOpFieldsValuesTuple &t);
bool doVrfEvpnNvoAddTask(const KeyOpFieldsValuesTuple & t);
bool doVrfEvpnNvoDelTask(const KeyOpFieldsValuesTuple & t);
bool doVrfVxlanTableCreateTask(const KeyOpFieldsValuesTuple & t);
bool doVrfVxlanTableRemoveTask(const KeyOpFieldsValuesTuple & t);
bool doVrfVxlanTableUpdate(const string& vrf_name, const string& vni, bool add);
void VrfVxlanTableSync(bool add);
void doTask(Consumer &consumer);

std::map<std::string, uint32_t> m_vrfTableMap;
std::set<uint32_t> m_freeTables;
VRFNameVNIMapTable m_vrfVniMapTable;

Table m_stateVrfTable, m_stateVrfObjectTable;
ProducerStateTable m_appVrfTableProducer, m_appVnetTableProducer;
ProducerStateTable m_appVrfTableProducer, m_appVnetTableProducer, m_appVxlanVrfTableProducer;
};

}
Expand Down
1 change: 1 addition & 0 deletions cfgmgr/vrfmgrd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ int main(int argc, char **argv)
vector<string> cfg_vrf_tables = {
CFG_VRF_TABLE_NAME,
CFG_VNET_TABLE_NAME,
CFG_VXLAN_EVPN_NVO_TABLE_NAME,
};

DBConnector cfgDb("CONFIG_DB", 0);
Expand Down
6 changes: 4 additions & 2 deletions orchagent/mirrororch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ MirrorEntry::MirrorEntry(const string& platform) :
greType = 0x88be;
}

string alias = "";
nexthopInfo.prefix = IpPrefix("0.0.0.0/0");
nexthopInfo.nexthop = NextHopKey("0.0.0.0", "");
nexthopInfo.nexthop = NextHopKey("0.0.0.0", alias);
}

MirrorOrch::MirrorOrch(TableConnector stateDbConnector, TableConnector confDbConnector,
Expand Down Expand Up @@ -1203,7 +1204,8 @@ void MirrorOrch::updateNextHop(const NextHopUpdate& update)
}
else
{
session.nexthopInfo.nexthop = NextHopKey("0.0.0.0", "");
string alias = "";
session.nexthopInfo.nexthop = NextHopKey("0.0.0.0", alias);
}

// Update State DB Nexthop
Expand Down
Loading