Skip to content

EVPN VxLAN enhancement to support P2MP tunnel based programming for Layer2 extension #1858

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

Merged
prsunny merged 16 commits into from
Oct 20, 2021
Merged

EVPN VxLAN enhancement to support P2MP tunnel based programming for Layer2 extension #1858

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
d9e23ad
Vxlan evpn p2mp changes for Layer2 functionality
dgsudharsan Jun 9, 2021
3b6620b
Fixing compilation
dgsudharsan Jul 14, 2021
979860f
Added referencing logic
dgsudharsan Jul 21, 2021
c458dba
Addressing code review comments
dgsudharsan Aug 4, 2021
67ca9cc
Addressing code review comments
dgsudharsan Aug 6, 2021
6cc06ec
Merge branch 'master' of github.com:Azure/sonic-swss into vxlan_evpn_…
dgsudharsan Aug 7, 2021
beeb58a
Fixing lgtm
dgsudharsan Aug 7, 2021
04858de
Addressing code review comments
dgsudharsan Sep 1, 2021
e2061ec
Addressing code review comments
dgsudharsan Sep 3, 2021
d0c92ef
Merge branch 'master' of github.com:Azure/sonic-swss into HEAD
dgsudharsan Sep 3, 2021
9e07e78
Fixing test issues and flaky tests
dgsudharsan Sep 4, 2021
a74eb57
Merge remote-tracking branch 'upstream/master' into vxlan_evpn_p2mp
dgsudharsan Sep 16, 2021
ffbf06a
Addressing code review comments
dgsudharsan Oct 12, 2021
c750f17
Cosmetic cleanups
dgsudharsan Oct 12, 2021
dd9781a
Merge branch 'master' of github.com:Azure/sonic-swss into vxlan_evpn_…
dgsudharsan Oct 12, 2021
c0631dc
Updating tests
dgsudharsan Oct 12, 2021
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
47 changes: 32 additions & 15 deletions orchagent/fdborch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,18 +760,35 @@ void FdbOrch::doTask(Consumer& consumer)
}
}

/* FDB type is either dynamic or static */
assert(type == "dynamic" || type == "dynamic_local" || type == "static" );
bool check_vlan_member = true;

if(origin == FDB_ORIGIN_VXLAN_ADVERTIZED)
{
VxlanTunnelOrch* tunnel_orch = gDirectory.get<VxlanTunnelOrch*>();

if(!remote_ip.length())
if (tunnel_orch->dipTunnelsUsed())
{
it = consumer.m_toSync.erase(it);
continue;
if(!remote_ip.length())
{
it = consumer.m_toSync.erase(it);
continue;
}
port = tunnel_orch->getTunnelPortName(remote_ip);
}
else
{
EvpnNvoOrch* evpn_nvo_orch = gDirectory.get<EvpnNvoOrch*>();
VxlanTunnel* sip_tunnel = evpn_nvo_orch->getEVPNVtep();
check_vlan_member = false;
if (sip_tunnel == NULL)
{
it = consumer.m_toSync.erase(it);
continue;
}
port = tunnel_orch->getTunnelPortName(sip_tunnel->getSrcIP().to_string(), true);
}
port = tunnel_orch->getTunnelPortName(remote_ip);
}


Expand All @@ -782,7 +799,7 @@ void FdbOrch::doTask(Consumer& consumer)
fdbData.remote_ip = remote_ip;
fdbData.esi = esi;
fdbData.vni = vni;
if (addFdbEntry(entry, port, fdbData))
if (addFdbEntry(entry, port, fdbData, check_vlan_member))
{
if (origin == FDB_ORIGIN_MCLAG_ADVERTIZED)
{
Expand All @@ -804,9 +821,6 @@ void FdbOrch::doTask(Consumer& consumer)
}
port = tunnel_orch->getTunnelPortName(remote_ip);
}


Copy link
Contributor

@srj102 srj102 Sep 3, 2021

Choose a reason for hiding this comment

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

Is this an intentional change ? By not removing this line the message will continue to remain in the m_tosync queue and be processed infinitely.

Copy link
Collaborator Author

@dgsudharsan dgsudharsan Sep 4, 2021

Choose a reason for hiding this comment

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

That's a good catch. It got removed during a merge conflict. I have added it back.

it = consumer.m_toSync.erase(it);
}
else
it++;
Expand Down Expand Up @@ -1121,15 +1135,15 @@ void FdbOrch::updateVlanMember(const VlanMemberUpdate& update)
}

bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name,
FdbData fdbData)
FdbData fdbData, bool check_vlan_member)
Copy link
Contributor

@srj102 srj102 Sep 3, 2021

Choose a reason for hiding this comment

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

Do we need to change the API signature ? If there is a need to check for vlan member it can be done using the isDipTunnelSupported call ?

Copy link
Collaborator Author

@dgsudharsan dgsudharsan Sep 4, 2021

Choose a reason for hiding this comment

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

Removed changes to API signature.

{
Port vlan;
Port port;

SWSS_LOG_ENTER();
SWSS_LOG_INFO("mac=%s bv_id=0x%" PRIx64 " port_name=%s type=%s origin=%d",
SWSS_LOG_INFO("mac=%s bv_id=0x%" PRIx64 " port_name=%s type=%s origin=%d remote_ip=%s",
entry.mac.to_string().c_str(), entry.bv_id, port_name.c_str(),
fdbData.type.c_str(), fdbData.origin);
fdbData.type.c_str(), fdbData.origin, fdbData.remote_ip.c_str());

if (!m_portsOrch->getPort(entry.bv_id, vlan))
{
Expand All @@ -1147,7 +1161,7 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name,
}

/* Retry until port is member of vlan*/
if (vlan.m_members.find(port_name) == vlan.m_members.end())
if (check_vlan_member && vlan.m_members.find(port_name) == vlan.m_members.end())
Copy link
Contributor

@srj102 srj102 Sep 3, 2021

Choose a reason for hiding this comment

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

I see that the EvpnRemoteVnip2mpOrch::addOperation already has a call to
gPortsOrch->addVlanMember in which case I don't see why this check is required.
if we call portsorch->isVlanMember.. It should work for both P2P and P2MP ?

Copy link
Collaborator Author

@dgsudharsan dgsudharsan Sep 4, 2021

Choose a reason for hiding this comment

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

Actually I thought of a better logic to include checks for P2P and P2MP. In P2MP scenario, the vlan member differentiator is remote IP (since only one port exists based on SIP tunnel) and thus including that in check will ensure it works in both scenarios.

{
SWSS_LOG_INFO("Saving a fdb entry until port %s becomes vlan %s member", port_name.c_str(), vlan.m_alias.c_str());
saved_fdb_entries[port_name].push_back({entry.mac,
Expand All @@ -1163,6 +1177,7 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name,

Port oldPort;
string oldType;
string oldRemoteIp;
FdbOrigin oldOrigin = FDB_ORIGIN_INVALID ;
bool macUpdate = false;

Expand All @@ -1172,19 +1187,21 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name,
/* get existing port and type */
oldType = it->second.type;
oldOrigin = it->second.origin;
oldRemoteIp = it->second.remote_ip;

if (!m_portsOrch->getPortByBridgePortId(it->second.bridge_port_id, oldPort))
{
SWSS_LOG_ERROR("Existing port 0x%" PRIx64 " details not found", it->second.bridge_port_id);
return false;
}

if ((oldOrigin == fdbData.origin) && (oldType == fdbData.type) && (port.m_bridge_port_id == it->second.bridge_port_id))
if ((oldOrigin == fdbData.origin) && (oldType == fdbData.type) && (port.m_bridge_port_id == it->second.bridge_port_id)
&& (oldRemoteIp == fdbData.remote_ip))
{
/* Duplicate Mac */
SWSS_LOG_INFO("FdbOrch: mac=%s %s port=%s type=%s origin=%d is duplicate", entry.mac.to_string().c_str(),
SWSS_LOG_INFO("FdbOrch: mac=%s %s port=%s type=%s origin=%d remote_ip=%s is duplicate", entry.mac.to_string().c_str(),
vlan.m_alias.c_str(), port_name.c_str(),
fdbData.type.c_str(), fdbData.origin);
fdbData.type.c_str(), fdbData.origin, fdbData.remote_ip.c_str());
return true;
}
else if (fdbData.origin != oldOrigin)
Expand Down
2 changes: 1 addition & 1 deletion orchagent/fdborch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ class FdbOrch: public Orch, public Subject, public Observer
void updateVlanMember(const VlanMemberUpdate&);
void updatePortOperState(const PortOperStateUpdate&);

bool addFdbEntry(const FdbEntry&, const string&, FdbData fdbData);
bool addFdbEntry(const FdbEntry&, const string&, FdbData fdbData, bool check_vlan_member=true);
void deleteFdbEntryFromSavedFDB(const MacAddress &mac, const unsigned short &vlanId, FdbOrigin origin, const string portName="");

bool storeFdbEntryState(const FdbUpdate& update);
Expand Down
27 changes: 27 additions & 0 deletions orchagent/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ sai_object_id_t gUnderlayIfId;
sai_object_id_t gSwitchId = SAI_NULL_OBJECT_ID;
MacAddress gMacAddress;
MacAddress gVxlanMacAddress;
bool gP2PTunnelSupported;

extern size_t gMaxBulkSize;

Expand Down Expand Up @@ -643,6 +644,32 @@ int main(int argc, char **argv)
orchDaemon = make_shared<FabricOrchDaemon>(&appl_db, &config_db, &state_db, chassis_app_db.get());
}

uint32_t max_tunnel_modes = 2;
Copy link
Collaborator

@prsunny prsunny Oct 8, 2021

Choose a reason for hiding this comment

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

Could you check if this can be moved to vxlan constructor?

Copy link
Collaborator Author

@dgsudharsan dgsudharsan Oct 12, 2021

Choose a reason for hiding this comment

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

Done

vector<int32_t> tunnel_peer_modes(max_tunnel_modes, 0);
sai_s32_list_t values;
values.count = max_tunnel_modes;
values.list = tunnel_peer_modes.data();

status = sai_query_attribute_enum_values_capability(gSwitchId, SAI_OBJECT_TYPE_TUNNEL,
SAI_TUNNEL_ATTR_PEER_MODE, &values);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_WARN("Unable to get supported tunnel peer modes. Defaulting to P2P");
gP2PTunnelSupported = true;
}
else
{
gP2PTunnelSupported = false;
for (uint32_t idx = 0; idx < values.count; idx++)
{
if (values.list[idx] == SAI_TUNNEL_PEER_MODE_P2P)
{
gP2PTunnelSupported = true;
break;
}
}
}

if (!orchDaemon->init())
{
SWSS_LOG_ERROR("Failed to initialize orchestration daemon");
Expand Down
23 changes: 18 additions & 5 deletions orchagent/orchdaemon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ using namespace swss;
extern sai_switch_api_t* sai_switch_api;
extern sai_object_id_t gSwitchId;
extern bool gSaiRedisLogRotate;
extern bool gP2PTunnelSupported;

extern void syncd_apply_view();
/*
Expand Down Expand Up @@ -167,19 +168,18 @@ bool OrchDaemon::init()
CoppOrch *copp_orch = new CoppOrch(m_applDb, APP_COPP_TABLE_NAME);
TunnelDecapOrch *tunnel_decap_orch = new TunnelDecapOrch(m_applDb, APP_TUNNEL_DECAP_TABLE_NAME);

VxlanTunnelOrch *vxlan_tunnel_orch = new VxlanTunnelOrch(m_stateDb, m_applDb, APP_VXLAN_TUNNEL_TABLE_NAME);
gDirectory.set(vxlan_tunnel_orch);
VxlanTunnelMapOrch *vxlan_tunnel_map_orch = new VxlanTunnelMapOrch(m_applDb, APP_VXLAN_TUNNEL_MAP_TABLE_NAME);
gDirectory.set(vxlan_tunnel_map_orch);
VxlanVrfMapOrch *vxlan_vrf_orch = new VxlanVrfMapOrch(m_applDb, APP_VXLAN_VRF_TABLE_NAME);
gDirectory.set(vxlan_vrf_orch);

EvpnRemoteVniOrch* evpn_remote_vni_orch = new EvpnRemoteVniOrch(m_applDb, APP_VXLAN_REMOTE_VNI_TABLE_NAME);
gDirectory.set(evpn_remote_vni_orch);

EvpnNvoOrch* evpn_nvo_orch = new EvpnNvoOrch(m_applDb, APP_VXLAN_EVPN_NVO_TABLE_NAME);
gDirectory.set(evpn_nvo_orch);

VxlanTunnelOrch *vxlan_tunnel_orch = new VxlanTunnelOrch(m_stateDb, m_applDb,
APP_VXLAN_TUNNEL_TABLE_NAME, gP2PTunnelSupported);
gDirectory.set(vxlan_tunnel_orch);

vector<string> qos_tables = {
CFG_TC_TO_QUEUE_MAP_TABLE_NAME,
Expand Down Expand Up @@ -351,7 +351,20 @@ bool OrchDaemon::init()
m_orchList.push_back(vxlan_tunnel_orch);
m_orchList.push_back(evpn_nvo_orch);
m_orchList.push_back(vxlan_tunnel_map_orch);
m_orchList.push_back(evpn_remote_vni_orch);

if (gP2PTunnelSupported)
{
EvpnRemoteVnip2pOrch* evpn_remote_vni_orch = new EvpnRemoteVnip2pOrch(m_applDb, APP_VXLAN_REMOTE_VNI_TABLE_NAME);
gDirectory.set(evpn_remote_vni_orch);
m_orchList.push_back(evpn_remote_vni_orch);
}
else
{
EvpnRemoteVnip2mpOrch* evpn_remote_vni_orch = new EvpnRemoteVnip2mpOrch(m_applDb, APP_VXLAN_REMOTE_VNI_TABLE_NAME);
gDirectory.set(evpn_remote_vni_orch);
m_orchList.push_back(evpn_remote_vni_orch);
}

m_orchList.push_back(vxlan_vrf_orch);
m_orchList.push_back(cfg_vnet_rt_orch);
m_orchList.push_back(vnet_orch);
Expand Down
6 changes: 6 additions & 0 deletions orchagent/port.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,17 @@ struct VlanMemberEntry

typedef std::map<sai_vlan_id_t, VlanMemberEntry> vlan_members_t;

typedef std::map<std::string, sai_object_id_t> endpoint_ip_l2mc_group_member_map_t;

struct VlanInfo
{
sai_object_id_t vlan_oid = 0;
sai_vlan_id_t vlan_id = 0;
sai_object_id_t host_intf_id = SAI_NULL_OBJECT_ID;
sai_vlan_flood_control_type_t uuc_flood_type = SAI_VLAN_FLOOD_CONTROL_TYPE_ALL;
sai_vlan_flood_control_type_t bc_flood_type = SAI_VLAN_FLOOD_CONTROL_TYPE_ALL;
sai_object_id_t l2mc_group_id = SAI_NULL_OBJECT_ID;
endpoint_ip_l2mc_group_member_map_t l2mc_members;
};

struct SystemPortInfo
Expand Down
Loading