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

Jump to bottom

[vxlan]add L2-vxlan [#376] #867

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[vxlan]add L2-vxlan [#376] #867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1146d04
L2-vxlan feature[#376]
Apr 30, 2019
1c1b182
add the corresponding vs-test
leoli-nps May 31, 2019
06ef3cf
refine code according to community review
Jun 18, 2019
66a751a
merge to swss master branch and fix some conflict
Jul 10, 2019
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
84 changes: 84 additions & 0 deletions cfgmgr/vxlanmgr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ using namespace swss;
// Fields name
#define VXLAN_TUNNEL "vxlan_tunnel"
#define SOURCE_IP "src_ip"
#define DST_IP "dst_ip"
#define VNI "vni"
#define VNET "vnet"
#define VXLAN "vxlan"
#define VXLAN_IF "vxlan_if"
#define VLAN "vlan"

#define VXLAN_NAME_PREFIX "Vxlan"
#define VXLAN_IF_NAME_PREFIX "Brvxlan"
Expand Down Expand Up @@ -386,6 +388,78 @@ bool VxlanMgr::doVxlanTunnelMapCreateTask(const KeyOpFieldsValuesTuple & t)
m_appVxlanTunnelMapTable.set(vxlanTunnelMapName, kfvFieldsValues(t));

SWSS_LOG_NOTICE("Create vxlan tunnel map %s", vxlanTunnelMapName.c_str());

/*Create vxlan tunnel in Linux kernel, the foramt is vxlanTunnelName-vni, such as VTTNL0001-1000*/
std::string vlan, vlan_id, vni_id, src_ip, dst_ip;
for (auto i : kfvFieldsValues(t))
{
const std::string & field = fvField(i);
const std::string & value = fvValue(i);
if (field == VLAN)
{
vlan = value;
}
else if (field == VNI)
{
vni_id = value;
}
}

const auto vlan_prefix = std::string("Vlan");
const auto prefix_len = vlan_prefix.length();
vlan_id = vlan.substr(prefix_len);

size_t found = vxlanTunnelMapName.find(delimiter);
const auto vxlanTunnelName = vxlanTunnelMapName.substr(0, found);

// If the vxlan tunnel has been created
auto it = m_vxlanTunnelCache.find(vxlanTunnelName);
if (it == m_vxlanTunnelCache.end())
{
SWSS_LOG_DEBUG("Vxlan tunnel %s has not been created", vxlanTunnelName.c_str());
// Suspend this message util the vxlan tunnel is created
return false;
}

auto sourceIp = std::find_if(
it->second.begin(),
it->second.end(),
[](const FieldValueTuple & fvt){ return fvt.first == SOURCE_IP; });
if (sourceIp == it->second.end())
{
SWSS_LOG_DEBUG("Vxlan tunnel %s has no field src_ip", vxlanTunnelName.c_str());
return true;
}
else
{
src_ip = sourceIp->second;
}
auto dstIp = std::find_if(
it->second.begin(),
it->second.end(),
[](const FieldValueTuple & fvt){ return fvt.first == DST_IP; });
if (dstIp != it->second.end())
{
dst_ip = dstIp->second;
}

//ip link add <vxlan_dev_name> type vxlan id <vni> local <src_ip> remote <dst_ip> dstport 4789
//ip link set <vxlan_dev_name> master DOT1Q_BRIDGE_NAME
//bridge vlan add vid <vlan_id> dev <vxlan_dev_name>
//ip link set <vxlan_dev_name> up
std::string vxlan_dev_name;
vxlan_dev_name = std::string("") + std::string(vxlanTunnelName) + "-" + std::string(vni_id);
const std::string cmds = std::string("")
+ BASH_CMD + " -c \""
+ IP_CMD + " link add " + vxlan_dev_name + " type vxlan id " + std::string(vni_id)
+ " local " + src_ip + (dstIp == it->second.end() ? "" : (" remote " + dst_ip)) + " dstport 4789 " + " && "
+ IP_CMD + " link set " + vxlan_dev_name + " master Bridge " + " && "
+ BRIDGE_CMD + " vlan add vid " + std::string(vlan_id) + " dev " + vxlan_dev_name + " && "
+ IP_CMD + " link set " + vxlan_dev_name + " up " + "\"";

std::string res;
EXEC_WITH_ERROR_THROW(cmds, res);

return true;
}

Expand All @@ -398,6 +472,16 @@ bool VxlanMgr::doVxlanTunnelMapDeleteTask(const KeyOpFieldsValuesTuple & t)
m_appVxlanTunnelMapTable.del(vxlanTunnelMapName);

SWSS_LOG_NOTICE("Delete vxlan tunnel map %s", vxlanTunnelMapName.c_str());

// ip link del dev {{VXLAN}}
size_t found = vxlanTunnelMapName.find(delimiter);
const auto vxlanTunnelName = vxlanTunnelMapName.substr(0, found);
std::string res;
const std::string cmd = std::string("")
+ IP_CMD " link del dev "
+ vxlanTunnelName;
EXECUTE(cmd, res);

return true;
}

Expand Down
117 changes: 89 additions & 28 deletions orchagent/fdborch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,18 @@
#include "crmorch.h"
#include "notifier.h"
#include "sai_serialize.h"
#include "directory.h"
#include "vxlanorch.h"
#include "swssnet.h"

using namespace swss;

extern sai_fdb_api_t *sai_fdb_api;

extern sai_object_id_t gSwitchId;
extern PortsOrch* gPortsOrch;
extern CrmOrch * gCrmOrch;
extern Directory<Orch*> gDirectory;

const int fdborch_pri = 20;

Expand Down Expand Up @@ -441,37 +447,59 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const
{
SWSS_LOG_ENTER();

if (m_entries.count(entry) != 0) // we already have such entries
{
// FIXME: should we check that the entry are moving to another port?
// FIXME: should we check that the entry are changing its type?
SWSS_LOG_ERROR("FDB entry already exists. mac=%s bv_id=0x%lx", entry.mac.to_string().c_str(), entry.bv_id);
return true;
}

sai_fdb_entry_t fdb_entry;

fdb_entry.switch_id = gSwitchId;
memcpy(fdb_entry.mac_address, entry.mac.getMac(), sizeof(sai_mac_t));
fdb_entry.bv_id = entry.bv_id;

Port port;
/* Retry until port is created */
if (!m_portsOrch->getPort(port_name, port))
sai_object_id_t bridge_port_id = SAI_NULL_OBJECT_ID;
Copy link
Contributor

Choose a reason for hiding this comment

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

The Sonic VxLAN HLD refers to a separate VXLAN_FDB_TABLE for MACs pointing to remote VTEP. addFdbEntry is to handle entry additions in APP_FDB_TABLE. The current code changes seem to indicate that the VxLAN MACs are populated in the APP_FDB_TABLE. Is there a change in the design ?

Either way It is better to handle VxLAN FDB entry add/remove in a different function to keep it clean instead of checking for port_name against VTT in multiple places.

Copy link

Choose a reason for hiding this comment

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

Yes, vxlan tunnel MAC is set in APP_FDB_TABLE currently. Packet forwarding is correctly in lab test.
In Sonic VxLAN HLD, I don't see any description about VXLAN_FDB_TABLE, only 'Add VxlanOrch as a member of FDBOrch. For FDB entries learnt on remote VTEP, app-fdb-table shall be updated and programmed to SAI by getting the BridgeIf/RemoteVTEP mapping from VxlanOrch. (TBD)'.

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/Azure/SONiC/blob/master/doc/vxlan/Vxlan_hld.md#22-app-db

VXLAN_FDB_TABLE is defined here.

Copy link

Choose a reason for hiding this comment

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

Thanks. I think that VXLAN_FDB_TABLE is used for L3 vxlan scenario, such as EVPN, entries in the table are exchanged by MBGP. The modification here is only for L2 vxlan scenario, entries are learned by ASIC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The existing fdborch can't listen to VXLAN_FDB_TABLE. We can refine the code after fdbOrch support listening to vxlan_fdb_table event.

VxlanTunnelOrch* tunnel_orch = gDirectory.get<VxlanTunnelOrch*>();
VxlanTunnel *tunnel_obj = NULL;
if(strncmp(port_name.c_str(),VXLAN_TUNNEL_PREFIX,strlen(VXLAN_TUNNEL_PREFIX)) == 0)
{
SWSS_LOG_DEBUG("Saving a fdb entry until port %s becomes active", port_name.c_str());
saved_fdb_entries[port_name].push_back({entry, type});
/* Retry until tunnel is created */
if (!tunnel_orch->isTunnelExists(port_name))
{
SWSS_LOG_WARN("Vxlan tunnel '%s' doesn't exist", port_name.c_str());
return true;
}

return true;
}
tunnel_obj = tunnel_orch->getVxlanTunnel(port_name);

bridge_port_id = tunnel_obj->getBridgePortId();

/* Retry until port is added to the VLAN */
if (!port.m_bridge_port_id)
/* Retry until tunnel bridge port is created */
if (!bridge_port_id)
{
SWSS_LOG_WARN("Vxlan tunnel '%s' bridge port doesn't exist", port_name.c_str());
return true;
}
else
{
SWSS_LOG_NOTICE("Get tunnel bridge port id is %lu", bridge_port_id);
}
}
else
{
SWSS_LOG_DEBUG("Saving a fdb entry until port %s has got a bridge port ID", port_name.c_str());
saved_fdb_entries[port_name].push_back({entry, type});
/* Retry until port is created */
if (!m_portsOrch->getPort(port_name, port))
{
SWSS_LOG_DEBUG("Saving a fdb entry until port %s becomes active", port_name.c_str());
saved_fdb_entries[port_name].push_back({entry, type});

return true;
return true;
}

/* Retry until port is added to the VLAN */
if (!port.m_bridge_port_id)
{
SWSS_LOG_DEBUG("Saving a fdb entry until port %s has got a bridge port ID", port_name.c_str());
saved_fdb_entries[port_name].push_back({entry, type});

return true;
}
}

sai_attribute_t attr;
Expand All @@ -482,13 +510,36 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const
attrs.push_back(attr);

attr.id = SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID;
attr.value.oid = port.m_bridge_port_id;
attrs.push_back(attr);
if(strncmp(port_name.c_str(),VXLAN_TUNNEL_PREFIX,strlen(VXLAN_TUNNEL_PREFIX)) != 0)
{
attr.value.oid = port.m_bridge_port_id;
attrs.push_back(attr);
}
else
{
attr.value.oid = bridge_port_id;
attrs.push_back(attr);

if(tunnel_obj)
{
attr.id = SAI_FDB_ENTRY_ATTR_ENDPOINT_IP;
swss::copy(attr.value.ipaddr, tunnel_obj->getDstIp());
attrs.push_back(attr);
char buf[INET6_ADDRSTRLEN];
inet_ntop(AF_INET, &attr.value.ipaddr.addr.ip4, buf, INET_ADDRSTRLEN);
SWSS_LOG_NOTICE("Create tunnel SAI_FDB_ENTRY_ATTR_ENDPOINT_IP %s, family %d", buf, attr.value.ipaddr.addr_family);
}
}

attr.id = SAI_FDB_ENTRY_ATTR_PACKET_ACTION;
attr.value.s32 = SAI_PACKET_ACTION_FORWARD;
attrs.push_back(attr);

if (m_entries.count(entry) != 0) // we already have such entries
{
removeFdbEntry(entry);
}

sai_status_t status = sai_fdb_api->create_fdb_entry(&fdb_entry, (uint32_t)attrs.size(), attrs.data());
if (status != SAI_STATUS_SUCCESS)
{
Expand All @@ -503,10 +554,14 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const

gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_FDB_ENTRY);

FdbUpdate update = {entry, port, true};
for (auto observer: m_observers)
/*TBD: Vxlan tunnel is not in portlist, here skip observer, refine later*/
if(strncmp(port_name.c_str(),VXLAN_TUNNEL_PREFIX,strlen(VXLAN_TUNNEL_PREFIX)) != 0)
{
observer->update(SUBJECT_TYPE_FDB_CHANGE, &update);
FdbUpdate update = {entry, port, true};
for (auto observer: m_observers)
{
observer->update(SUBJECT_TYPE_FDB_CHANGE, &update);
}
}

return true;
Expand Down Expand Up @@ -541,12 +596,18 @@ bool FdbOrch::removeFdbEntry(const FdbEntry& entry)
gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_FDB_ENTRY);

Port port;
m_portsOrch->getPortByBridgePortId(entry.bv_id, port);
bool res = false;

FdbUpdate update = {entry, port, false};
for (auto observer: m_observers)
res = m_portsOrch->getPortByBridgePortId(entry.bv_id, port);

/*TBD: Vxlan tunnel is not in portlist, here skip observer, refine later*/
if(res == true)
{
observer->update(SUBJECT_TYPE_FDB_CHANGE, &update);
FdbUpdate update = {entry, port, false};
for (auto observer: m_observers)
{
observer->update(SUBJECT_TYPE_FDB_CHANGE, &update);
}
}

return true;
Expand Down
38 changes: 1 addition & 37 deletions orchagent/vnetorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,36 +470,7 @@ VnetBridgeInfo VNetBitmapObject::getBridgeInfoByVni(uint32_t vni, string tunnelN
tunnel->createTunnel(MAP_T::BRIDGE_TO_VNI, MAP_T::VNI_TO_BRIDGE);
}

attr.id = SAI_BRIDGE_PORT_ATTR_TYPE;
attr.value.s32 = SAI_BRIDGE_PORT_TYPE_TUNNEL;
bpt_attrs.push_back(attr);

attr.id = SAI_BRIDGE_PORT_ATTR_BRIDGE_ID;
attr.value.oid = info.bridge_id;
bpt_attrs.push_back(attr);

attr.id = SAI_BRIDGE_PORT_ATTR_ADMIN_STATE;
attr.value.booldata = true;
bpt_attrs.push_back(attr);

attr.id = SAI_BRIDGE_PORT_ATTR_TUNNEL_ID;
attr.value.oid = tunnel->getTunnelId();
bpt_attrs.push_back(attr);

attr.id = SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE;
attr.value.s32 = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE;
bpt_attrs.push_back(attr);

status = sai_bridge_api->create_bridge_port(
&info.bridge_port_tunnel_id,
gSwitchId,
(uint32_t)bpt_attrs.size(),
bpt_attrs.data());
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to create tunnel bridge port for vni %u", vni);
throw std::runtime_error("vni creation failed");
}
info.bridge_port_tunnel_id = tunnel->getBridgePortId();

// FIXME: Use "createVxlanTunnelMap()" for tunnel mapper creation
auto tunnelEncapMapperEntry = tunnel->addEncapMapperEntry(info.bridge_id, vni);
Expand Down Expand Up @@ -534,13 +505,6 @@ bool VNetBitmapObject::clearBridgeInfoByVni(uint32_t vni, string tunnelName)

sai_status_t status;

status = sai_bridge_api->remove_bridge_port(bridgeInfo.bridge_port_tunnel_id);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to remove tunnel bridge port for VNI %u, SAI rc: %d", vni, status);
return false;
}

status = sai_bridge_api->remove_bridge_port(bridgeInfo.bridge_port_rif_id);
if (status != SAI_STATUS_SUCCESS)
{
Expand Down
Loading