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.

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

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
113 changes: 91 additions & 22 deletions orchagent/fdborch.cpp
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 @@ -440,38 +446,68 @@ void FdbOrch::updateVlanMember(const VlanMemberUpdate& update)
bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const string& type)
shine4chen marked this conversation as resolved.
Show resolved Hide resolved
{
SWSS_LOG_ENTER();

#if 0
shine4chen marked this conversation as resolved.
Show resolved Hide resolved
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;
}

#endif
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.

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.

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(),"VTT",3) == 0)
shine4chen marked this conversation as resolved.
Show resolved Hide resolved
{
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 +518,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(),"VTT",3) != 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 +562,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(),"VTT",3) != 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 +604,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
91 changes: 91 additions & 0 deletions orchagent/vxlanorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@
#include "vxlanorch.h"
#include "directory.h"
#include "swssnet.h"
#include "exec.h"
#include "../cfgmgr/shellcmd.h"
#include "sai_serialize.h"

using namespace swss;

/* Global variables */
extern sai_object_id_t gSwitchId;
Expand All @@ -23,6 +28,8 @@ extern sai_next_hop_api_t *sai_next_hop_api;
extern Directory<Orch*> gDirectory;
extern PortsOrch* gPortsOrch;
extern sai_object_id_t gUnderlayIfId;
extern sai_bridge_api_t *sai_bridge_api;
extern sai_switch_api_t *sai_switch_api;

const map<MAP_T, uint32_t> vxlanTunnelMap =
{
Expand Down Expand Up @@ -328,6 +335,58 @@ create_tunnel_termination(
return term_table_id;
}

static sai_object_id_t create_tunnel_bridge_port(sai_object_id_t tunnel_oid)
{
SWSS_LOG_ENTER();

sai_attribute_t attr;
vector<sai_attribute_t> attrs;

vector<sai_attribute_t> attrs_bridge;
sai_object_id_t default1QBridge;
sai_status_t status;

attr.id = SAI_SWITCH_ATTR_DEFAULT_1Q_BRIDGE_ID;
attrs_bridge.push_back(attr);

status = sai_switch_api->get_switch_attribute(gSwitchId, (uint32_t)attrs_bridge.size(), attrs_bridge.data());
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get default 1Q bridge, rv:%d", status);
throw "PortsOrch initialization failure";
shine4chen marked this conversation as resolved.
Show resolved Hide resolved
}

default1QBridge = attrs_bridge[0].value.oid;

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

attr.id = SAI_BRIDGE_PORT_ATTR_TUNNEL_ID;
attr.value.oid = tunnel_oid;
attrs.push_back(attr);

attr.id = SAI_BRIDGE_PORT_ATTR_BRIDGE_ID;
attr.value.oid = default1QBridge;
attrs.push_back(attr);

/* Create a bridge port with admin status set to UP */
attr.id = SAI_BRIDGE_PORT_ATTR_ADMIN_STATE;
attr.value.booldata = true;
attrs.push_back(attr);

sai_object_id_t m_bridge_port_id;
status = sai_bridge_api->create_bridge_port(&m_bridge_port_id, gSwitchId, (uint32_t)attrs.size(), attrs.data());
if (status != SAI_STATUS_SUCCESS)
{
throw std::runtime_error("Failed to add tunnel bridge port");
}

SWSS_LOG_NOTICE("Add tunnel bridge port to default 1Q bridge, bridge port id is %lu", m_bridge_port_id);

return m_bridge_port_id;
}

bool VxlanTunnel::createTunnel(MAP_T encap, MAP_T decap)
{
try
Expand All @@ -350,6 +409,15 @@ bool VxlanTunnel::createTunnel(MAP_T encap, MAP_T decap)

ids_.tunnel_id = create_tunnel(ids_.tunnel_encap_id, ids_.tunnel_decap_id, ip, gUnderlayIfId);

shared_ptr<DBConnector> m_counter_db = shared_ptr<DBConnector>(new DBConnector(COUNTERS_DB, DBConnector::DEFAULT_UNIXSOCKET, 0));
shine4chen marked this conversation as resolved.
Show resolved Hide resolved
unique_ptr<Table> m_counterTable = unique_ptr<Table>(new Table(m_counter_db.get(), COUNTERS_PORT_NAME_MAP));

/* Add tunnel name map to counter table */
FieldValueTuple tuple(tunnel_name_, sai_serialize_object_id(ids_.tunnel_id));
vector<FieldValueTuple> fields;
fields.push_back(tuple);
m_counterTable->set("", fields);

ip = nullptr;
if (!dst_ip_.isZero())
{
Expand All @@ -360,6 +428,9 @@ bool VxlanTunnel::createTunnel(MAP_T encap, MAP_T decap)
ids_.tunnel_term_id = create_tunnel_termination(ids_.tunnel_id, ips, ip, gVirtualRouterId);
active_ = true;
tunnel_map_ = { encap, decap };

ids_.tunnel_bridge_port_id = create_tunnel_bridge_port(ids_.tunnel_id);
shine4chen marked this conversation as resolved.
Show resolved Hide resolved
SWSS_LOG_NOTICE("Create tunnel bridge port id is %lu", ids_.tunnel_bridge_port_id);
}
catch (const std::runtime_error& error)
{
Expand Down Expand Up @@ -731,6 +802,26 @@ bool VxlanTunnelMapOrch::addOperation(const Request& request)
SWSS_LOG_NOTICE("Vxlan tunnel map entry '%s' for tunnel '%s' was created",
tunnel_map_entry_name.c_str(), tunnel_name.c_str());

//ip link add <vxlan_dev_name> type vxlan id <vni> local <src_ip> remote <dst_ip> dstport 4789
shine4chen marked this conversation as resolved.
Show resolved Hide resolved
//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
IpAddress ips, ipd;
std::string vxlan_dev_name;
ips = tunnel_obj->getSrcIp();
ipd = tunnel_obj->getDstIp();
vxlan_dev_name = std::string("") + std::string(tunnel_name) + "-" + std::to_string(vni_id);
const std::string cmds = std::string("")
+ BASH_CMD + " -c \""
+ IP_CMD + " link add " + vxlan_dev_name + " type vxlan id " + std::to_string(vni_id)
+ " local " + ips.to_string().c_str() + (ipd.isZero() ? "" : (" remote " + ipd.to_string())) + " dstport 4789 " + " && "
+ IP_CMD + " link set " + vxlan_dev_name + " master Bridge " + " && "
+ BRIDGE_CMD + " vlan add vid " + std::to_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 Down
16 changes: 16 additions & 0 deletions orchagent/vxlanorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ struct tunnel_ids_t
sai_object_id_t tunnel_decap_id;
sai_object_id_t tunnel_id;
sai_object_id_t tunnel_term_id;
sai_object_id_t tunnel_bridge_port_id;
};

struct nh_key_t
Expand Down Expand Up @@ -104,6 +105,21 @@ class VxlanTunnel
return ids_.tunnel_encap_id;
}

sai_object_id_t getBridgePortId() const
{
return ids_.tunnel_bridge_port_id;
}

IpAddress getSrcIp() const
{
return src_ip_;
}

IpAddress getDstIp() const
{
return dst_ip_;
}

void updateNextHop(IpAddress& ipAddr, MacAddress macAddress, uint32_t vni, sai_object_id_t nhId);
bool removeNextHop(IpAddress& ipAddr, MacAddress macAddress, uint32_t vni);
sai_object_id_t getNextHop(IpAddress& ipAddr, MacAddress macAddress, uint32_t vni) const;
Expand Down