From 6d3d2b5a31403b983ca8acb5ad45e02fd37364f0 Mon Sep 17 00:00:00 2001 From: Pankaj Jain Date: Tue, 28 Apr 2020 14:06:35 -0700 Subject: [PATCH 01/21] [Orchagent]: FdbOrch changes for EVPN VXLAN Signed-off-by: Pankaj Jain --- orchagent/fdborch.cpp | 1104 +++++++++++++++++++++++++++++--------- orchagent/fdborch.h | 65 ++- orchagent/orchdaemon.cpp | 8 +- tests/test_evpn_fdb.py | 680 +++++++++++++++++++++++ 4 files changed, 1605 insertions(+), 252 deletions(-) create mode 100644 tests/test_evpn_fdb.py diff --git a/orchagent/fdborch.cpp b/orchagent/fdborch.cpp index 9d587cb55a..00301d511e 100644 --- a/orchagent/fdborch.cpp +++ b/orchagent/fdborch.cpp @@ -11,23 +11,30 @@ #include "crmorch.h" #include "notifier.h" #include "sai_serialize.h" +#include "vxlanorch.h" +#include "directory.h" extern sai_fdb_api_t *sai_fdb_api; extern sai_object_id_t gSwitchId; extern PortsOrch* gPortsOrch; extern CrmOrch * gCrmOrch; +extern Directory gDirectory; -const int fdborch_pri = 20; +const int FdbOrch::fdborch_pri = 20; -FdbOrch::FdbOrch(TableConnector applDbConnector, TableConnector stateDbConnector, PortsOrch *port) : - Orch(applDbConnector.first, applDbConnector.second, fdborch_pri), +FdbOrch::FdbOrch(DBConnector* applDbConnector, vector appFdbTables, TableConnector stateDbFdbConnector, PortsOrch *port) : + Orch(applDbConnector, appFdbTables), m_portsOrch(port), - m_table(applDbConnector.first, applDbConnector.second), - m_fdbStateTable(stateDbConnector.first, stateDbConnector.second) + m_fdbStateTable(stateDbFdbConnector.first, stateDbFdbConnector.second) { + for(auto it: appFdbTables) + { + m_appTables.push_back(new Table(applDbConnector, it.first)); + } + m_portsOrch->attach(this); - m_flushNotificationsConsumer = new NotificationConsumer(applDbConnector.first, "FLUSHFDBREQUEST"); + m_flushNotificationsConsumer = new NotificationConsumer(applDbConnector, "FLUSHFDBREQUEST"); auto flushNotifier = new Notifier(m_flushNotificationsConsumer, this, "FLUSHFDBREQUEST"); Orch::addExecutor(flushNotifier); @@ -57,7 +64,11 @@ bool FdbOrch::bake() bool FdbOrch::storeFdbEntryState(const FdbUpdate& update) { const FdbEntry& entry = update.entry; + FdbData fdbdata; + FdbData oldFdbData; + const Port& port = update.port; const MacAddress& mac = entry.mac; + string portName = port.m_alias; Port vlan; if (!m_portsOrch->getPort(entry.bv_id, vlan)) @@ -72,31 +83,55 @@ bool FdbOrch::storeFdbEntryState(const FdbUpdate& update) if (update.add) { - SWSS_LOG_INFO("Storing FDB entry: [%s, 0x%" PRIx64 "] [ port: %s ]", - entry.mac.to_string().c_str(), - entry.bv_id, entry.port_name.c_str()); - auto inserted = m_entries.insert(entry); - - SWSS_LOG_DEBUG("FdbOrch notification: mac %s was inserted into bv_id 0x%" PRIx64, - entry.mac.to_string().c_str(), entry.bv_id); - - if (!inserted.second) + bool mac_move = false; + auto it = m_entries.find(entry); + if (it != m_entries.end()) { - SWSS_LOG_INFO("FdbOrch notification: mac %s is duplicate", entry.mac.to_string().c_str()); - return false; + /* This block is specifically added for MAC_MOVE event + and not expected to be executed for LEARN event + */ + if (port.m_bridge_port_id == it->second.bridge_port_id) + { + SWSS_LOG_INFO("FdbOrch notification: mac %s is duplicate", entry.mac.to_string().c_str()); + return false; + } + mac_move = true; + oldFdbData = it->second; } + fdbdata.bridge_port_id = update.port.m_bridge_port_id; + fdbdata.type = update.type; + fdbdata.origin = FDB_ORIGIN_LEARN; + fdbdata.remote_ip = ""; + fdbdata.esi = ""; + fdbdata.vni = 0; + + m_entries[entry] = fdbdata; + SWSS_LOG_INFO("FdbOrch notification: mac %s was inserted in port %s into bv_id 0x%" PRIx64, + entry.mac.to_string().c_str(), portName.c_str(), entry.bv_id); + SWSS_LOG_INFO("m_entries size=%lu mac=%s port=0x%" PRIx64, + m_entries.size(), entry.mac.to_string().c_str(), m_entries[entry].bridge_port_id); + // Write to StateDb std::vector fvs; - fvs.push_back(FieldValueTuple("port", entry.port_name)); - fvs.push_back(FieldValueTuple("type", "dynamic")); + fvs.push_back(FieldValueTuple("port", portName)); + fvs.push_back(FieldValueTuple("type", update.type)); m_fdbStateTable.set(key, fvs); - gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_FDB_ENTRY); + if (!mac_move) + { + gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_FDB_ENTRY); + } return true; } else { + auto it= m_entries.find(entry); + if(it != m_entries.end()) + { + oldFdbData = it->second; + } + size_t erased = m_entries.erase(entry); SWSS_LOG_DEBUG("FdbOrch notification: mac %s was removed from bv_id 0x%" PRIx64, entry.mac.to_string().c_str(), entry.bv_id); @@ -105,8 +140,11 @@ bool FdbOrch::storeFdbEntryState(const FdbUpdate& update) return false; } - // Remove in StateDb - m_fdbStateTable.del(key); + if (oldFdbData.origin != FDB_ORIGIN_VXLAN_ADVERTIZED) + { + // Remove in StateDb for non advertised mac addresses + m_fdbStateTable.del(key); + } gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_FDB_ENTRY); return true; @@ -122,6 +160,8 @@ void FdbOrch::update(sai_fdb_event_t type, FdbUpdate update; update.entry.mac = entry->mac_address; update.entry.bv_id = entry->bv_id; + update.type = "dynamic"; + Port vlan; SWSS_LOG_INFO("FDB event:%d, MAC: %s , BVID: 0x%" PRIx64 " , \ bridge port ID: 0x%" PRIx64 ".", @@ -139,132 +179,237 @@ void FdbOrch::update(sai_fdb_event_t type, switch (type) { case SAI_FDB_EVENT_LEARNED: + { + SWSS_LOG_INFO("Received LEARN event for bvid=0x%" PRIx64 "mac=%s port=0x%" PRIx64, entry->bv_id, update.entry.mac.to_string().c_str(), bridge_port_id); + + if (!m_portsOrch->getPort(entry->bv_id, vlan)) + { + SWSS_LOG_ERROR("FdbOrch LEARN notification: Failed to locate vlan port from bv_id 0x%" PRIx64, entry->bv_id); + return; + } + if (!m_portsOrch->getPortByBridgePortId(bridge_port_id, update.port)) { - SWSS_LOG_ERROR("Failed to get port by bridge port ID 0x%" PRIx64, bridge_port_id); + SWSS_LOG_ERROR("FdbOrch LEARN notification: Failed to get port by bridge port ID 0x%" PRIx64, bridge_port_id); return; } // we already have such entries - if (m_entries.find(update.entry) != m_entries.end()) + auto existing_entry = m_entries.find(update.entry); + if (existing_entry != m_entries.end()) { - SWSS_LOG_INFO("FdbOrch notification: mac %s is already in bv_id 0x%" PRIx64, - update.entry.mac.to_string().c_str(), entry->bv_id); + SWSS_LOG_INFO("FdbOrch LEARN notification: mac %s is already in bv_id 0x%" + PRIx64 "existing-bp 0x%" PRIx64 "new-bp:0x%" PRIx64, + update.entry.mac.to_string().c_str(), entry->bv_id, existing_entry->second.bridge_port_id, bridge_port_id); break; } update.add = true; - update.entry.port_name = update.port.m_alias; - storeFdbEntryState(update); + update.type = "dynamic"; + update.port.m_fdb_count++; + m_portsOrch->setPort(update.port.m_alias, update.port); + vlan.m_fdb_count++; + m_portsOrch->setPort(vlan.m_alias, vlan); - SWSS_LOG_INFO("Notifying observers of FDB entry LEARN"); - for (auto observer: m_observers) - { - observer->update(SUBJECT_TYPE_FDB_CHANGE, &update); - } + storeFdbEntryState(update); + notify(SUBJECT_TYPE_FDB_CHANGE, &update); break; - + } case SAI_FDB_EVENT_AGED: - case SAI_FDB_EVENT_MOVE: - update.add = false; - storeFdbEntryState(update); + { + SWSS_LOG_INFO("Received AGE event for bvid=%lx mac=%s port=%lx", entry->bv_id, update.entry.mac.to_string().c_str(), bridge_port_id); - SWSS_LOG_INFO("Notifying observers of FDB entry removal on AGED/MOVED"); - for (auto observer: m_observers) + if (!m_portsOrch->getPort(entry->bv_id, vlan)) { - observer->update(SUBJECT_TYPE_FDB_CHANGE, &update); + SWSS_LOG_NOTICE("FdbOrch AGE notification: Failed to locate vlan port from bv_id 0x%lx", entry->bv_id); } - break; + auto existing_entry = m_entries.find(update.entry); + // we don't have such entries + if (existing_entry == m_entries.end()) + { + SWSS_LOG_INFO("FdbOrch AGE notification: mac %s is not present in bv_id 0x%lx bp 0x%lx", + update.entry.mac.to_string().c_str(), entry->bv_id, bridge_port_id); + break; + } - case SAI_FDB_EVENT_FLUSHED: + if (!m_portsOrch->getPortByBridgePortId(bridge_port_id, update.port)) + { + SWSS_LOG_NOTICE("FdbOrch AGE notification: Failed to get port by bridge port ID 0x%lx", bridge_port_id); + } - SWSS_LOG_INFO("FDB Flush event received: [ %s , 0x%" PRIx64 " ], \ - bridge port ID: 0x%" PRIx64 ".", - update.entry.mac.to_string().c_str(), entry->bv_id, - bridge_port_id); + if (existing_entry->second.bridge_port_id != bridge_port_id) + { + SWSS_LOG_NOTICE("FdbOrch AGE notification: Stale aging event received for mac-bv_id %s-0x%lx with bp=0x%lx existing bp=0x%lx", update.entry.mac.to_string().c_str(), entry->bv_id, bridge_port_id, existing_entry->second.bridge_port_id); + // We need to get the port for bridge-port in existing fdb + if (!m_portsOrch->getPortByBridgePortId(existing_entry->second.bridge_port_id, update.port)) + { + SWSS_LOG_NOTICE("FdbOrch AGE notification: Failed to get port by bridge port ID 0x%lx", existing_entry->second.bridge_port_id); + } + // dont return, let it delete just to bring SONiC and SAI in sync + // return; + } - string vlanName = "-"; - if (entry->bv_id) { - Port vlan; + if (existing_entry->second.type == "static") + { + update.type = "static"; - if (!m_portsOrch->getPort(entry->bv_id, vlan)) + if (vlan.m_members.find(update.port.m_alias) == vlan.m_members.end()) + { + saved_fdb_entries[update.port.m_alias].push_back({existing_entry->first.mac, + vlan.m_vlan_info.vlan_id, update.type, + existing_entry->second.origin, + existing_entry->second.remote_ip, + existing_entry->second.esi, + existing_entry->second.vni}); + } + else { - SWSS_LOG_ERROR("FdbOrch notification: Failed to locate vlan\ - port from bv_id 0x%" PRIx64, entry->bv_id); + /*port added back to vlan before we receive delete + notification for flush from SAI. Re-add entry to SAI + */ + sai_attribute_t attr; + vector attrs; + + attr.id = SAI_FDB_ENTRY_ATTR_TYPE; + attr.value.s32 = SAI_FDB_ENTRY_TYPE_STATIC; + attrs.push_back(attr); + attr.id = SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID; + attr.value.oid = bridge_port_id; + attrs.push_back(attr); + auto status = sai_fdb_api->create_fdb_entry(entry, (uint32_t)attrs.size(), attrs.data()); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to create FDB %s on %s, rv:%d", + existing_entry->first.mac.to_string().c_str(), update.port.m_alias.c_str(), status); + return; + } return; } - vlanName = "Vlan" + to_string(vlan.m_vlan_info.vlan_id); } + update.add = false; + if (!update.port.m_alias.empty()) + { + update.port.m_fdb_count--; + m_portsOrch->setPort(update.port.m_alias, update.port); + } + if (!vlan.m_alias.empty()) + { + vlan.m_fdb_count--; + m_portsOrch->setPort(vlan.m_alias, vlan); + } + + storeFdbEntryState(update); + + notify(SUBJECT_TYPE_FDB_CHANGE, &update); + + notifyTunnelOrch(update.port); + break; + } + case SAI_FDB_EVENT_MOVE: + { + Port port_old; + auto existing_entry = m_entries.find(update.entry); + + SWSS_LOG_INFO("Received MOVE event for bvid=%lx mac=%s port=%lx", entry->bv_id, update.entry.mac.to_string().c_str(), bridge_port_id); + + if (!m_portsOrch->getPort(entry->bv_id, vlan)) + { + SWSS_LOG_ERROR("FdbOrch MOVE notification: Failed to locate vlan port from bv_id 0x%lx", entry->bv_id); + return; + } + + if (!m_portsOrch->getPortByBridgePortId(bridge_port_id, update.port)) + { + SWSS_LOG_ERROR("FdbOrch MOVE notification: Failed to get port by bridge port ID 0x%lx", bridge_port_id); + return; + } - if (bridge_port_id == SAI_NULL_OBJECT_ID && - entry->bv_id == SAI_NULL_OBJECT_ID) + // We should already have such entry + if (existing_entry == m_entries.end()) { - SWSS_LOG_INFO("FDB Flush: [ %s , %s ] = { port: - }", - update.entry.mac.to_string().c_str(), vlanName.c_str()); - for (auto itr = m_entries.begin(); itr != m_entries.end();) + SWSS_LOG_WARN("FdbOrch MOVE notification: mac %s is not found in bv_id 0x%lx", + update.entry.mac.to_string().c_str(), entry->bv_id); + } + else if (!m_portsOrch->getPortByBridgePortId(existing_entry->second.bridge_port_id, port_old)) + { + SWSS_LOG_ERROR("FdbOrch MOVE notification: Failed to get port by bridge port ID 0x%lx", existing_entry->second.bridge_port_id); + return; + } + + update.add = true; + + if (!port_old.m_alias.empty()) + { + port_old.m_fdb_count--; + m_portsOrch->setPort(port_old.m_alias, port_old); + } + update.port.m_fdb_count++; + m_portsOrch->setPort(update.port.m_alias, update.port); + + storeFdbEntryState(update); + + notify(SUBJECT_TYPE_FDB_CHANGE, &update); + + notifyTunnelOrch(port_old); + + break; + } + case SAI_FDB_EVENT_FLUSHED: + SWSS_LOG_INFO("Received FLUSH event for bvid=%lx mac=%s port=%lx", entry->bv_id, update.entry.mac.to_string().c_str(), bridge_port_id); + for (auto itr = m_entries.begin(); itr != m_entries.end();) + { + if ((itr->second.type == "static") || (itr->second.origin == FDB_ORIGIN_VXLAN_ADVERTIZED)) { - /* - TODO: here should only delete the dynamic fdb entries, - but unfortunately in structure FdbEntry currently have - no member to indicate the fdb entry type, - if there is static mac added, here will have issue. - */ - update.entry.mac = itr->mac; - update.entry.bv_id = itr->bv_id; - update.add = false; itr++; + continue; + } - storeFdbEntryState(update); + if (((bridge_port_id == SAI_NULL_OBJECT_ID) && (entry->bv_id == SAI_NULL_OBJECT_ID)) // Flush all DYNAMIC + || ((bridge_port_id == itr->second.bridge_port_id) && (entry->bv_id == SAI_NULL_OBJECT_ID)) // flush all DYN on a port + || ((bridge_port_id == SAI_NULL_OBJECT_ID) && (entry->bv_id == itr->first.bv_id))) // flush all DYN on a vlan + { - for (auto observer: m_observers) + if (!m_portsOrch->getPortByBridgePortId(itr->second.bridge_port_id, update.port)) { - observer->update(SUBJECT_TYPE_FDB_CHANGE, &update); + SWSS_LOG_ERROR("FdbOrch FLUSH notification: Failed to get port by bridge port ID 0x%lx", itr->second.bridge_port_id); } - } - } - else if (entry->bv_id == SAI_NULL_OBJECT_ID) - { - /* FLUSH based on port */ - SWSS_LOG_INFO("FDB Flush: [ %s , %s ] = { port: %s }", - update.entry.mac.to_string().c_str(), - vlanName.c_str(), update.port.m_alias.c_str()); - for (auto itr = m_entries.begin(); itr != m_entries.end();) - { - auto next_item = std::next(itr); - if (itr->port_name == update.port.m_alias) + if (!m_portsOrch->getPort(itr->first.bv_id, vlan)) { - update.entry.mac = itr->mac; - update.entry.bv_id = itr->bv_id; - update.add = false; + SWSS_LOG_NOTICE("FdbOrch FLUSH notification: Failed to locate vlan port from bv_id 0x%lx", itr->first.bv_id); + } - storeFdbEntryState(update); + update.entry.mac = itr->first.mac; + update.entry.bv_id = itr->first.bv_id; + update.add = false; + itr++; - for (auto observer: m_observers) - { - observer->update(SUBJECT_TYPE_FDB_CHANGE, &update); - } + if (!update.port.m_alias.empty()) + { + update.port.m_fdb_count--; + m_portsOrch->setPort(update.port.m_alias, update.port); + } + if (!vlan.m_alias.empty()) + { + vlan.m_fdb_count--; + m_portsOrch->setPort(vlan.m_alias, vlan); } - itr = next_item; - } - } - else if (bridge_port_id == SAI_NULL_OBJECT_ID) - { - /* FLUSH based on VLAN - unsupported */ - SWSS_LOG_ERROR("Unsupported FDB Flush: [ %s , %s ] = { port: - }", - update.entry.mac.to_string().c_str(), - vlanName.c_str()); - } - else - { - /* FLUSH based on port and VLAN - unsupported */ - SWSS_LOG_ERROR("Unsupported FDB Flush: [ %s , %s ] = { port: %s }", - update.entry.mac.to_string().c_str(), - vlanName.c_str(), update.port.m_alias.c_str()); + /* This will invalidate the current iterator hence itr++ is done before */ + storeFdbEntryState(update); + + SWSS_LOG_DEBUG("FdbOrch FLUSH notification: mac %s was removed", update.entry.mac.to_string().c_str()); + + notify(SUBJECT_TYPE_FDB_CHANGE, &update); + notifyTunnelOrch(update.port); + } + else + { + itr++; + } } break; } @@ -300,6 +445,7 @@ void FdbOrch::update(SubjectType type, void *cntx) bool FdbOrch::getPort(const MacAddress& mac, uint16_t vlan, Port& port) { + sai_object_id_t bridge_port_id; SWSS_LOG_ENTER(); if (!m_portsOrch->getVlanByVlanId(vlan, port)) @@ -308,23 +454,20 @@ bool FdbOrch::getPort(const MacAddress& mac, uint16_t vlan, Port& port) return false; } - sai_fdb_entry_t entry; - entry.switch_id = gSwitchId; - memcpy(entry.mac_address, mac.getMac(), sizeof(sai_mac_t)); + FdbEntry entry; entry.bv_id = port.m_vlan_info.vlan_oid; - - sai_attribute_t attr; - attr.id = SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID; - - sai_status_t status = sai_fdb_api->get_fdb_entry_attribute(&entry, 1, &attr); - if (status != SAI_STATUS_SUCCESS) + entry.mac = mac; + auto it= m_entries.find(entry); + if(it != m_entries.end()) + { + bridge_port_id = it->second.bridge_port_id; + } + else { - SWSS_LOG_ERROR("Failed to get bridge port ID for FDB entry %s, rv:%d", - mac.to_string().c_str(), status); return false; } - if (!m_portsOrch->getPortByBridgePortId(attr.value.oid, port)) + if (!m_portsOrch->getPortByBridgePortId(bridge_port_id, port)) { SWSS_LOG_ERROR("Failed to get port by bridge port ID 0x%" PRIx64, attr.value.oid); return false; @@ -342,6 +485,15 @@ void FdbOrch::doTask(Consumer& consumer) return; } + FdbOrigin origin = FDB_ORIGIN_PROVISIONED; + + string table_name = consumer.getTableName(); + if(table_name == APP_VXLAN_FDB_TABLE_NAME) + { + origin = FDB_ORIGIN_VXLAN_ADVERTIZED; + } + + auto it = consumer.m_toSync.begin(); while (it != consumer.m_toSync.end()) { @@ -355,7 +507,24 @@ void FdbOrch::doTask(Consumer& consumer) if (!m_portsOrch->getPort(keys[0], vlan)) { SWSS_LOG_INFO("Failed to locate %s", keys[0].c_str()); - it++; + if(op == DEL_COMMAND) + { + /* Delete if it is in saved_fdb_entry */ + unsigned short vlan_id; + try { + vlan_id = (unsigned short) stoi(keys[0].substr(4)); + } catch(exception &e) { + it = consumer.m_toSync.erase(it); + continue; + } + deleteFdbEntryFromSavedFDB(MacAddress(keys[1]), vlan_id, origin); + + it = consumer.m_toSync.erase(it); + } + else + { + it++; + } continue; } @@ -365,8 +534,12 @@ void FdbOrch::doTask(Consumer& consumer) if (op == SET_COMMAND) { - string port; - string type; + string port = ""; + string type = "dynamic"; + string remote_ip = ""; + string esi = ""; + unsigned int vni = 0; + string sticky = ""; for (auto i : kfvFieldsValues(t)) { @@ -379,28 +552,67 @@ void FdbOrch::doTask(Consumer& consumer) { type = fvValue(i); } + + if(origin == FDB_ORIGIN_VXLAN_ADVERTIZED) + { + if (fvField(i) == "remote_vtep") + { + remote_ip = fvValue(i); + // Creating an IpAddress object to validate if remote_ip is valid + // if invalid it will throw the exception and we will ignore the + // event + try { + IpAddress valid_ip = IpAddress(remote_ip); + (void)valid_ip; // To avoid g++ warning + } catch(exception &e) { + SWSS_LOG_NOTICE("Invalid IP address in remote MAC %s", remote_ip.c_str()); + remote_ip = ""; + break; + } + } + + if (fvField(i) == "esi") + { + esi = fvValue(i); + } + + if (fvField(i) == "vni") + { + try { + vni = (unsigned int) stoi(fvValue(i)); + } catch(exception &e) { + SWSS_LOG_INFO("Invalid VNI in remote MAC %s", fvValue(i).c_str()); + vni = 0; + break; + } + } + } } - entry.port_name = port; /* FDB type is either dynamic or static */ assert(type == "dynamic" || type == "static"); - if (addFdbEntry(entry, type)) + if(origin == FDB_ORIGIN_VXLAN_ADVERTIZED) + { + VxlanTunnelOrch* tunnel_orch = gDirectory.get(); + + if(!remote_ip.length()) + { + it = consumer.m_toSync.erase(it); + continue; + } + port = tunnel_orch->getTunnelPortName(remote_ip); + } + + + if (addFdbEntry(entry, port, type, origin, remote_ip, vni, esi)) it = consumer.m_toSync.erase(it); else it++; - - /* Remove corresponding APP_DB entry if type is 'dynamic' */ - // FIXME: The modification of table is not thread safe. - // Uncomment this after this issue is fixed. - // if (type == "dynamic") - // { - // m_table.del(kfvKey(t)); - // } } else if (op == DEL_COMMAND) { - if (removeFdbEntry(entry)) + if (removeFdbEntry(entry, origin)) it = consumer.m_toSync.erase(it); else it++; @@ -423,7 +635,6 @@ void FdbOrch::doTask(NotificationConsumer& consumer) return; } - sai_status_t status; std::string op; std::string data; std::vector values; @@ -434,28 +645,17 @@ void FdbOrch::doTask(NotificationConsumer& consumer) { if (op == "ALL") { - /* - * so far only support flush all the FDB entris - * flush per port and flush per vlan will be added later. - */ - status = sai_fdb_api->flush_fdb_entries(gSwitchId, 0, NULL); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Flush fdb failed, return code %x", status); - } - + flushFdbAll(0); return; } else if (op == "PORT") { - /*place holder for flush port fdb*/ - SWSS_LOG_ERROR("Received unsupported flush port fdb request"); + flushFdbByPort(data, 0); return; } else if (op == "VLAN") { - /*place holder for flush vlan fdb*/ - SWSS_LOG_ERROR("Received unsupported flush vlan fdb request"); + flushFdbByVlan(data, 0); return; } else @@ -491,70 +691,41 @@ void FdbOrch::doTask(NotificationConsumer& consumer) } } -void FdbOrch::flushFDBEntries(sai_object_id_t bridge_port_oid, - sai_object_id_t vlan_oid) -{ - vector attrs; - sai_attribute_t attr; - sai_status_t rv = SAI_STATUS_SUCCESS; - - SWSS_LOG_ENTER(); - - if (SAI_NULL_OBJECT_ID == bridge_port_oid && - SAI_NULL_OBJECT_ID == vlan_oid) - { - SWSS_LOG_WARN("Couldn't flush FDB. Bridge port OID: 0x%" PRIx64 " bvid:%" PRIx64 ",", - bridge_port_oid, vlan_oid); - return; - } - - if (SAI_NULL_OBJECT_ID != bridge_port_oid) - { - attr.id = SAI_FDB_FLUSH_ATTR_BRIDGE_PORT_ID; - attr.value.oid = bridge_port_oid; - attrs.push_back(attr); - } - - if (SAI_NULL_OBJECT_ID != vlan_oid) - { - attr.id = SAI_FDB_FLUSH_ATTR_BV_ID; - attr.value.oid = vlan_oid; - attrs.push_back(attr); - } - - SWSS_LOG_INFO("Flushing FDB bridge_port_oid: 0x%" PRIx64 ", and bvid_oid:0x%" PRIx64 ".", bridge_port_oid, vlan_oid); - - rv = sai_fdb_api->flush_fdb_entries(gSwitchId, (uint32_t)attrs.size(), attrs.data()); - if (SAI_STATUS_SUCCESS != rv) - { - SWSS_LOG_ERROR("Flushing FDB failed. rv:%d", rv); - } -} - void FdbOrch::updatePortOperState(const PortOperStateUpdate& update) { SWSS_LOG_ENTER(); if (update.operStatus == SAI_PORT_OPER_STATUS_DOWN) { swss::Port p = update.port; - flushFDBEntries(p.m_bridge_port_id, SAI_NULL_OBJECT_ID); + flushFdbByPort(p.m_alias, 0); } return; } void FdbOrch::updateVlanMember(const VlanMemberUpdate& update) { + Port port; + + string port_name = update.member.m_alias; + string vlan_name = update.vlan.m_alias; + SWSS_LOG_ENTER(); + if (!m_portsOrch->getPort(port_name, port)) + { + SWSS_LOG_ERROR("could not locate port from alias %s", port_name.c_str()); + return; + } + if (!update.add) { - swss::Port vlan = update.vlan; - swss::Port port = update.member; - flushFDBEntries(port.m_bridge_port_id, vlan.m_vlan_info.vlan_oid); + if(port.m_type != Port::TUNNEL) + { + flushFdbByPortVlan(port_name, vlan_name, 1); + } return; } - string port_name = update.member.m_alias; auto fdb_list = std::move(saved_fdb_entries[port_name]); if(!fdb_list.empty()) { @@ -562,99 +733,327 @@ void FdbOrch::updateVlanMember(const VlanMemberUpdate& update) { // try to insert an FDB entry. If the FDB entry is not ready to be inserted yet, // it would be added back to the saved_fdb_entries structure by addFDBEntry() - (void)addFdbEntry(fdb.entry, fdb.type); + if(fdb.vlanId == update.vlan.m_vlan_info.vlan_id) + { + FdbEntry entry; + entry.mac = fdb.mac; + entry.bv_id = update.vlan.m_vlan_info.vlan_oid; + (void)addFdbEntry(entry, port_name, fdb.type, fdb.origin, + fdb.remote_ip, fdb.vni, fdb.esi); + } + else + { + saved_fdb_entries[port_name].push_back(fdb); + } } } } -bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& type) +bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const string& type, FdbOrigin origin, const string& remote_ip, unsigned int vni, const string& esi) { - SWSS_LOG_ENTER(); + Port vlan; + Port port; - sai_fdb_entry_t fdb_entry; + SWSS_LOG_ENTER(); + SWSS_LOG_NOTICE("mac=%s bv_id=0x%lx port_name=%s type=%s origin=%d", entry.mac.to_string().c_str(), entry.bv_id, port_name.c_str(), type.c_str(), origin); - fdb_entry.switch_id = gSwitchId; - memcpy(fdb_entry.mac_address, entry.mac.getMac(), sizeof(sai_mac_t)); - fdb_entry.bv_id = entry.bv_id; + if (!m_portsOrch->getPort(entry.bv_id, vlan)) + { + SWSS_LOG_NOTICE("addFdbEntry: Failed to locate vlan port from bv_id 0x%lx", entry.bv_id); + return false; + } - Port port; /* Retry until port is created */ - if (!m_portsOrch->getPort(entry.port_name, port)) + if (!m_portsOrch->getPort(port_name, port) || (port.m_bridge_port_id == SAI_NULL_OBJECT_ID)) { - SWSS_LOG_DEBUG("Saving a fdb entry until port %s becomes active", - entry. port_name.c_str()); - saved_fdb_entries[entry.port_name].push_back({entry, type}); + SWSS_LOG_INFO("Saving a fdb entry until port %s becomes active", port_name.c_str()); + saved_fdb_entries[port_name].push_back({entry.mac, + vlan.m_vlan_info.vlan_id, type, origin, remote_ip, esi, vni}); + return true; + } + /* Retry until port is member of vlan*/ + if (vlan.m_members.find(port_name) == vlan.m_members.end()) + { + 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, + vlan.m_vlan_info.vlan_id, type, origin, remote_ip, esi, vni}); return true; } - /* Retry until port is added to the VLAN */ - if (!port.m_bridge_port_id) + sai_status_t status; + 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 oldPort; + string oldType; + FdbOrigin oldOrigin = FDB_ORIGIN_INVALID ; + bool macUpdate = false; + auto it = m_entries.find(entry); + if(it != m_entries.end()) { - SWSS_LOG_DEBUG("Saving a fdb entry until port %s has got a bridge port ID", - entry.port_name.c_str()); - saved_fdb_entries[entry.port_name].push_back({entry, type}); + /* get existing port and type */ + oldType = it->second.type; + oldOrigin = it->second.origin; - return true; + if (!m_portsOrch->getPortByBridgePortId(it->second.bridge_port_id, oldPort)) + { + SWSS_LOG_ERROR("Existing port 0x%lx details not found", it->second.bridge_port_id); + return false; + } + + if((oldOrigin == origin) && (oldType == type) && (port.m_bridge_port_id == it->second.bridge_port_id)) + { + /* Duplicate Mac */ + SWSS_LOG_NOTICE("FdbOrch: mac=%s %s port=%s type=%s origin=%d is duplicate", entry.mac.to_string().c_str(), vlan.m_alias.c_str(), port_name.c_str(), type.c_str(), origin); + return true; + } + else if(origin != oldOrigin) + { + /* Mac origin has changed */ + if((oldType == "static") && (oldOrigin == FDB_ORIGIN_PROVISIONED)) + { + /* old mac was static and provisioned, it can not be changed by Remote Mac */ + SWSS_LOG_NOTICE("Already existing static MAC:%s in Vlan:%d. " + "Received same MAC from peer:%s; " + "Peer mac ignored", + entry.mac.to_string().c_str(), vlan.m_vlan_info.vlan_id, + remote_ip.c_str()); + + return true; + } + else if((oldType == "static") && (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED) && (type == "dynamic")) + { + /* old mac was static and received from remote, it can not be changed by dynamic locally provisioned Mac */ + SWSS_LOG_NOTICE("Already existing static MAC:%s in Vlan:%d " + "from Peer:%s. Now same is provisioned as dynamic; " + "Provisioned dynamic mac is ignored", + entry.mac.to_string().c_str(), vlan.m_vlan_info.vlan_id, + it->second.remote_ip.c_str()); + return true; + } + else if(oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED) + { + if((oldType == "static") && (type == "static")) + { + SWSS_LOG_WARN("You have just overwritten existing static MAC:%s " + "in Vlan:%d from Peer:%s, " + "If it is a mistake, it will result in inconsistent Traffic Forwarding", + entry.mac.to_string().c_str(), + vlan.m_vlan_info.vlan_id, + it->second.remote_ip.c_str()); + } + } + } + else /* (origin == oldOrigin) */ + { + /* Mac origin is same, all changes are allowed */ + /* Allowed + * Bridge-port is changed or/and + * Sticky bit from remote is modified or + * provisioned mac is converted from static<-->dynamic + */ + } + + macUpdate = true; } sai_attribute_t attr; vector attrs; attr.id = SAI_FDB_ENTRY_ATTR_TYPE; - attr.value.s32 = (type == "dynamic") ? SAI_FDB_ENTRY_TYPE_DYNAMIC : SAI_FDB_ENTRY_TYPE_STATIC; + if (origin == FDB_ORIGIN_VXLAN_ADVERTIZED) + { + attr.value.s32 = (type == "dynamic") ? SAI_FDB_ENTRY_TYPE_STATIC_MACMOVE : SAI_FDB_ENTRY_TYPE_STATIC; + } + else + { + attr.value.s32 = (type == "dynamic") ? SAI_FDB_ENTRY_TYPE_DYNAMIC : SAI_FDB_ENTRY_TYPE_STATIC; + } 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); - 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 + if(origin == FDB_ORIGIN_VXLAN_ADVERTIZED) { - removeFdbEntry(entry); + IpAddress remote = IpAddress(remote_ip); + sai_ip_address_t ipaddr; + if(remote.isV4()) + { + ipaddr.addr_family = SAI_IP_ADDR_FAMILY_IPV4; + ipaddr.addr.ip4 = remote.getV4Addr(); + } + else + { + ipaddr.addr_family = SAI_IP_ADDR_FAMILY_IPV6; + memcpy(ipaddr.addr.ip6, remote.getV6Addr(), sizeof(ipaddr.addr.ip6)); + } + attr.id = SAI_FDB_ENTRY_ATTR_ENDPOINT_IP; + attr.value.ipaddr = ipaddr; + attrs.push_back(attr); + } + else if(macUpdate && (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED) && (origin != oldOrigin)) + { + /* origin is changed from Remote-advertized to Local-provisioned + * Remove the end-point ip attribute from fdb entry + */ + sai_ip_address_t ipaddr; + ipaddr.addr_family = SAI_IP_ADDR_FAMILY_IPV4; + ipaddr.addr.ip4 = 0; + attr.id = SAI_FDB_ENTRY_ATTR_ENDPOINT_IP; + attr.value.ipaddr = ipaddr; + attrs.push_back(attr); } - sai_status_t status = sai_fdb_api->create_fdb_entry(&fdb_entry, (uint32_t)attrs.size(), attrs.data()); - if (status != SAI_STATUS_SUCCESS) + string key = "Vlan" + to_string(vlan.m_vlan_info.vlan_id) + ":" + entry.mac.to_string(); + + + if(macUpdate) + { + SWSS_LOG_NOTICE("MAC-Update FDB %s in %s on from-%s:to-%s from-%s:to-%s origin-%d-to-%d", + entry.mac.to_string().c_str(), vlan.m_alias.c_str(), oldPort.m_alias.c_str(), + port_name.c_str(), oldType.c_str(), type.c_str(), oldOrigin, origin); + for(auto itr : attrs) + { + status = sai_fdb_api->set_fdb_entry_attribute(&fdb_entry, &itr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("macUpdate-Failed for attr.id=0x%x for FDB %s in %s on %s, rv:%d", + itr.id, entry.mac.to_string().c_str(), vlan.m_alias.c_str(), port_name.c_str(), status); + return false; + } + } + if (oldPort.m_bridge_port_id != port.m_bridge_port_id) + { + oldPort.m_fdb_count--; + m_portsOrch->setPort(oldPort.m_alias, oldPort); + port.m_fdb_count++; + m_portsOrch->setPort(port.m_alias, port); + } + } + else { - SWSS_LOG_ERROR("Failed to create %s FDB %s on %s, rv:%d", - type.c_str(), entry.mac.to_string().c_str(), - entry.port_name.c_str(), status); - return false; //FIXME: it should be based on status. Some could be retried, some not + SWSS_LOG_NOTICE("MAC-Create %s FDB %s in %s on %s", type.c_str(), entry.mac.to_string().c_str(), vlan.m_alias.c_str(), port_name.c_str()); + + status = sai_fdb_api->create_fdb_entry(&fdb_entry, (uint32_t)attrs.size(), attrs.data()); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to create %s FDB %s in %s on %s, rv:%d", + type.c_str(), entry.mac.to_string().c_str(), + vlan.m_alias.c_str(), port_name.c_str(), status); + return false; //FIXME: it should be based on status. Some could be retried, some not + } + port.m_fdb_count++; + m_portsOrch->setPort(port.m_alias, port); + vlan.m_fdb_count++; + m_portsOrch->setPort(vlan.m_alias, vlan); } - SWSS_LOG_NOTICE("Storing FDB entry: [%s, 0x%" PRIx64 "] [ port: %s , type: %s]", - entry.mac.to_string().c_str(), - entry.bv_id, entry.port_name.c_str(), type.c_str()); + FdbData fdbData; + fdbData.bridge_port_id = port.m_bridge_port_id; + fdbData.type = type; + fdbData.origin = origin; + fdbData.remote_ip = remote_ip; + fdbData.esi = esi; + fdbData.vni = vni; - (void) m_entries.insert(entry); + m_entries[entry] = fdbData; - gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_FDB_ENTRY); + string key = "Vlan" + to_string(vlan.m_vlan_info.vlan_id) + ":" + entry.mac.to_string(); + + if (origin != FDB_ORIGIN_VXLAN_ADVERTIZED) + { + /* State-DB is updated only for Local Mac addresses */ + // Write to StateDb + std::vector fvs; + fvs.push_back(FieldValueTuple("port", port_name)); + if (type == "dynamic_local") + fvs.push_back(FieldValueTuple("type", "dynamic")); + else + fvs.push_back(FieldValueTuple("type", type)); + m_fdbStateTable.set(key, fvs); + } + else if (macUpdate && (oldOrigin != FDB_ORIGIN_VXLAN_ADVERTIZED)) + { + /* origin is FDB_ORIGIN_ADVERTIZED and it is mac-update + * so delete from StateDb since we only keep local fdbs + * in state-db + */ + m_fdbStateTable.del(key); + } - FdbUpdate update = {entry, port, true}; - for (auto observer: m_observers) + if(!macUpdate) { - observer->update(SUBJECT_TYPE_FDB_CHANGE, &update); + gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_FDB_ENTRY); } + FdbUpdate update; + update.entry = entry; + update.port = port; + update.type = type; + update.add = true; + + notify(SUBJECT_TYPE_FDB_CHANGE, &update); + return true; } -bool FdbOrch::removeFdbEntry(const FdbEntry& entry) +bool FdbOrch::removeFdbEntry(const FdbEntry& entry, FdbOrigin origin) { + Port vlan; + Port port; + SWSS_LOG_ENTER(); - if (m_entries.count(entry) == 0) + SWSS_LOG_NOTICE("FdbOrch RemoveFDBEntry: mac=%s bv_id=0x%lx origin %d", entry.mac.to_string().c_str(), entry.bv_id, origin); + + if (!m_portsOrch->getPort(entry.bv_id, vlan)) { - SWSS_LOG_ERROR("FDB entry isn't found. mac=%s bv_id=0x%" PRIx64 ".", - entry.mac.to_string().c_str(), entry.bv_id); + SWSS_LOG_NOTICE("FdbOrch notification: Failed to locate vlan port from bv_id 0x%lx", entry.bv_id); + return false; + } + + auto it= m_entries.find(entry); + if(it == m_entries.end()) + { + SWSS_LOG_INFO("FdbOrch RemoveFDBEntry: FDB entry isn't found. mac=%s bv_id=0x%lx", entry.mac.to_string().c_str(), entry.bv_id); + + /* check whether the entry is in the saved fdb, if so delete it from there. */ + deleteFdbEntryFromSavedFDB(entry.mac, vlan.m_vlan_info.vlan_id, origin); return true; } + FdbData fdbData = it->second; + if (!m_portsOrch->getPortByBridgePortId(fdbData.bridge_port_id, port)) + { + SWSS_LOG_NOTICE("FdbOrch RemoveFDBEntry: Failed to locate port from bridge_port_id 0x%lx", fdbData.bridge_port_id); + return false; + } + + if(fdbData.origin != origin) + { + /* When mac is moved from remote to local + * BGP will delete the mac from vxlan_fdb_table + * but we should not delete this mac here since now + * mac in orchagent represents locally learnt + */ + SWSS_LOG_NOTICE("FdbOrch RemoveFDBEntry: mac=%s fdb origin is different; found_origin:%d delete_origin:%d", + entry.mac.to_string().c_str(), fdbData.origin, origin); + + /* We may still have the mac in saved-fdb probably due to unavailability + * of bridge-port. check whether the entry is in the saved fdb, + * if so delete it from there. */ + deleteFdbEntryFromSavedFDB(entry.mac, vlan.m_vlan_info.vlan_id, origin); + + return true; + } + + string key = "Vlan" + to_string(vlan.m_vlan_info.vlan_id) + ":" + entry.mac.to_string(); + sai_status_t status; sai_fdb_entry_t fdb_entry; fdb_entry.switch_id = gSwitchId; @@ -664,24 +1063,247 @@ bool FdbOrch::removeFdbEntry(const FdbEntry& entry) status = sai_fdb_api->remove_fdb_entry(&fdb_entry); if (status != SAI_STATUS_SUCCESS) { - SWSS_LOG_ERROR("Failed to remove FDB entry. mac=%s, bv_id=0x%" PRIx64, + SWSS_LOG_ERROR("FdbOrch RemoveFDBEntry: Failed to remove FDB entry. mac=%s, bv_id=0x%lx", entry.mac.to_string().c_str(), entry.bv_id); return true; //FIXME: it should be based on status. Some could be retried. some not } + SWSS_LOG_NOTICE("Removed mac=%s bv_id=0x%lx port:%s", + entry.mac.to_string().c_str(), entry.bv_id, port.m_alias.c_str()); + + port.m_fdb_count--; + m_portsOrch->setPort(port.m_alias, port); + vlan.m_fdb_count--; + m_portsOrch->setPort(vlan.m_alias, vlan); (void)m_entries.erase(entry); + // Remove in StateDb + if (fdbData.origin != FDB_ORIGIN_VXLAN_ADVERTIZED) + { + m_fdbStateTable.del(key); + } + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_FDB_ENTRY); + FdbUpdate update; + update.entry = entry; + update.port = port; + update.type = fdbData.type; + update.add = false; + + notify(SUBJECT_TYPE_FDB_CHANGE, &update); + + notifyTunnelOrch(update.port); + + return true; +} + +bool FdbOrch::flushFdbAll(bool flush_static) +{ + sai_status_t status; + sai_attribute_t port_attr; + + if (!flush_static) + { + port_attr.id = SAI_FDB_FLUSH_ATTR_ENTRY_TYPE; + port_attr.value.s32 = SAI_FDB_FLUSH_ENTRY_TYPE_DYNAMIC; + status = sai_fdb_api->flush_fdb_entries(gSwitchId, 1, &port_attr); + } + else + { + status = sai_fdb_api->flush_fdb_entries(gSwitchId, 0, NULL); + } + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Flush fdb failed, return code %x", status); + return false; + } + return true; +} + +bool FdbOrch::flushFdbByPort(const string &alias, bool flush_static) +{ + sai_status_t status; Port port; - m_portsOrch->getPortByBridgePortId(entry.bv_id, port); + sai_attribute_t port_attr[2]; + + if (!m_portsOrch->getPort(alias, port)) + { + SWSS_LOG_ERROR("could not locate port from alias %s", alias.c_str()); + return false; + } + + if ((port.m_bridge_port_id == SAI_NULL_OBJECT_ID) || !port.m_fdb_count) + { + /* port is not an L2 port or no macs to flush */ + return true; + } + + SWSS_LOG_NOTICE("m_bridge_port_id 0x%lx flush_static %d m_fdb_count %u", port.m_bridge_port_id, flush_static, port.m_fdb_count); - SWSS_LOG_INFO("Notifying observers of FDB entry removal"); - FdbUpdate update = {entry, port, false}; - for (auto observer: m_observers) + port_attr[0].id = SAI_FDB_FLUSH_ATTR_BRIDGE_PORT_ID; + port_attr[0].value.oid = port.m_bridge_port_id; + if (!flush_static) { - observer->update(SUBJECT_TYPE_FDB_CHANGE, &update); + port_attr[1].id = SAI_FDB_FLUSH_ATTR_ENTRY_TYPE; + port_attr[1].value.s32 = SAI_FDB_FLUSH_ENTRY_TYPE_DYNAMIC; + status = sai_fdb_api->flush_fdb_entries(gSwitchId, 2, port_attr); + } + else + { + status = sai_fdb_api->flush_fdb_entries(gSwitchId, 1, port_attr); + } + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Flush fdb failed, return code %x", status); + return false; + } + return true; +} + +bool FdbOrch::flushFdbByVlan(const string &alias, bool flush_static) +{ + sai_status_t status; + Port vlan; + sai_attribute_t vlan_attr[2]; + + if (!m_portsOrch->getPort(alias, vlan)) + { + SWSS_LOG_ERROR("could not locate vlan from alias %s", alias.c_str()); + return false; + } + SWSS_LOG_NOTICE("vlan_oid 0x%lx flush_static %d", vlan.m_vlan_info.vlan_oid, flush_static); + + vlan_attr[0].id = SAI_FDB_FLUSH_ATTR_BV_ID; + vlan_attr[0].value.oid = vlan.m_vlan_info.vlan_oid; + if (!flush_static) + { + vlan_attr[1].id = SAI_FDB_FLUSH_ATTR_ENTRY_TYPE; + vlan_attr[1].value.s32 = SAI_FDB_FLUSH_ENTRY_TYPE_DYNAMIC; + status = sai_fdb_api->flush_fdb_entries(gSwitchId, 2, vlan_attr); + } + else + { + status = sai_fdb_api->flush_fdb_entries(gSwitchId, 1, vlan_attr); + } + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Flush fdb failed, return code %x", status); + return false; } return true; } + +bool FdbOrch::flushFdbByPortVlan(const string &port_alias, const string &vlan_alias, bool flush_static) +{ + + sai_status_t status; + Port vlan; + Port port; + sai_attribute_t port_vlan_attr[3]; + + SWSS_LOG_NOTICE("port %s vlan %s", port_alias.c_str(), vlan_alias.c_str()); + + if (!m_portsOrch->getPort(port_alias, port)) + { + SWSS_LOG_ERROR("could not locate port from alias %s", port_alias.c_str()); + return false; + } + if (!m_portsOrch->getPort(vlan_alias, vlan)) + { + SWSS_LOG_NOTICE("FdbOrch notification: Failed to locate vlan %s", vlan_alias.c_str()); + return false; + } + + if ((port.m_bridge_port_id == SAI_NULL_OBJECT_ID) || !port.m_fdb_count) + { + /* port is not an L2 port or no macs to flush */ + return true; + } + + SWSS_LOG_NOTICE("vlan_oid 0x%lx m_bridge_port_id 0x%lx flush_static %d m_fdb_count %u", vlan.m_vlan_info.vlan_oid, port.m_bridge_port_id, flush_static, port.m_fdb_count); + + port_vlan_attr[0].id = SAI_FDB_FLUSH_ATTR_BV_ID; + port_vlan_attr[0].value.oid = vlan.m_vlan_info.vlan_oid; + port_vlan_attr[1].id = SAI_FDB_FLUSH_ATTR_BRIDGE_PORT_ID; + port_vlan_attr[1].value.oid = port.m_bridge_port_id; + if (!flush_static) + { + port_vlan_attr[2].id = SAI_FDB_FLUSH_ATTR_ENTRY_TYPE; + port_vlan_attr[2].value.s32 = SAI_FDB_FLUSH_ENTRY_TYPE_DYNAMIC; + status = sai_fdb_api->flush_fdb_entries(gSwitchId, 3, port_vlan_attr); + } + else + { + status = sai_fdb_api->flush_fdb_entries(gSwitchId, 2, port_vlan_attr); + } + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Flush fdb failed, return code %x", status); + return false; + } + + return true; +} + +void FdbOrch::deleteFdbEntryFromSavedFDB(const MacAddress &mac, + const unsigned short &vlanId, FdbOrigin origin, const string portName) +{ + bool found=false; + SavedFdbEntry entry; + entry.mac = mac; + entry.vlanId = vlanId; + entry.type = "static"; + /* Below members are unused during delete compare */ + entry.origin = origin; + + for (auto& itr: saved_fdb_entries) + { + if(portName.empty() || (portName == itr.first)) + { + auto iter = saved_fdb_entries[itr.first].begin(); + while(iter != saved_fdb_entries[itr.first].end()) + { + if (*iter == entry) + { + if(iter->origin == origin) + { + SWSS_LOG_NOTICE("FDB entry found in saved fdb. deleting..." + "mac=%s vlan_id=0x%x origin:%d port:%s", + mac.to_string().c_str(), vlanId, origin, + itr.first.c_str()); + saved_fdb_entries[itr.first].erase(iter); + + found=true; + break; + } + else + { + SWSS_LOG_NOTICE("FDB entry found in saved fdb, but Origin is " + "different mac=%s vlan_id=0x%x reqOrigin:%d " + "foundOrigin:%d port:%s, IGNORED", + mac.to_string().c_str(), vlanId, origin, + iter->origin, itr.first.c_str()); + } + } + iter++; + } + } + if(found) + break; + } +} + +// Notify Tunnel Orch when the number of MAC entries +void FdbOrch::notifyTunnelOrch(Port& port) +{ + VxlanTunnelOrch* tunnel_orch = gDirectory.get(); + + if((port.m_type != Port::TUNNEL) || + (port.m_fdb_count != 0)) + return; + + tunnel_orch->deleteTunnelPort(port); +} + diff --git a/orchagent/fdborch.h b/orchagent/fdborch.h index 83f3086a0a..e405fc4c88 100644 --- a/orchagent/fdborch.h +++ b/orchagent/fdborch.h @@ -5,29 +5,70 @@ #include "observer.h" #include "portsorch.h" +enum FdbOrigin +{ + FDB_ORIGIN_INVALID = 0, + FDB_ORIGIN_LEARN = 1, + FDB_ORIGIN_PROVISIONED = 2, + FDB_ORIGIN_VXLAN_ADVERTIZED = 4 +}; + struct FdbEntry { MacAddress mac; sai_object_id_t bv_id; - std::string port_name; bool operator<(const FdbEntry& other) const { return tie(mac, bv_id) < tie(other.mac, other.bv_id); } + bool operator==(const FdbEntry& other) const + { + return tie(mac, bv_id) == tie(other.mac, other.bv_id); + } }; struct FdbUpdate { FdbEntry entry; Port port; + string type; bool add; }; +struct FdbData +{ + sai_object_id_t bridge_port_id; + string type; + FdbOrigin origin; + /** + {"dynamic", FDB_ORIGIN_LEARN} => dynamically learnt + {"dynamic", FDB_ORIGIN_PROVISIONED} => provisioned dynamic with swssconfig in APPDB + {"dynamic", FDB_ORIGIN_ADVERTIZED} => synced from remote device e.g. BGP MAC route + {"static", FDB_ORIGIN_LEARN} => Invalid + {"static", FDB_ORIGIN_PROVISIONED} => statically provisioned + {"static", FDB_ORIGIN_ADVERTIZED} => sticky synced from remote device + */ + + /* Remote FDB related info */ + string remote_ip; + string esi; + unsigned int vni; +}; + struct SavedFdbEntry { - FdbEntry entry; + MacAddress mac; + unsigned short vlanId; string type; + FdbOrigin origin; + string remote_ip; + string esi; + unsigned int vni; + bool operator==(const SavedFdbEntry& other) const + { + return tie(mac, vlanId) == tie(other.mac, other.vlanId); + } }; typedef unordered_map> fdb_entries_by_port_t; @@ -36,7 +77,7 @@ class FdbOrch: public Orch, public Subject, public Observer { public: - FdbOrch(TableConnector applDbConnector, TableConnector stateDbConnector, PortsOrch *port); + FdbOrch(DBConnector* applDbConnector, vector appFdbTables, TableConnector stateDbFdbConnector, PortsOrch *port); ~FdbOrch() { @@ -47,12 +88,19 @@ class FdbOrch: public Orch, public Subject, public Observer void update(sai_fdb_event_t, const sai_fdb_entry_t *, sai_object_id_t); void update(SubjectType type, void *cntx); bool getPort(const MacAddress&, uint16_t, Port&); + bool flushFdbByPortVlan(const string &, const string &, bool flush_static); + bool flushFdbByVlan(const string &, bool flush_static); + bool flushFdbByPort(const string &, bool flush_static); + bool flushFdbAll(bool flush_static); + bool removeFdbEntry(const FdbEntry& entry, FdbOrigin origin=FDB_ORIGIN_PROVISIONED); + + static const int fdborch_pri; private: PortsOrch *m_portsOrch; - set m_entries; + map m_entries; fdb_entries_by_port_t saved_fdb_entries; - Table m_table; + vector m_appTables; Table m_fdbStateTable; NotificationConsumer* m_flushNotificationsConsumer; NotificationConsumer* m_fdbNotificationConsumer; @@ -62,12 +110,11 @@ class FdbOrch: public Orch, public Subject, public Observer void updateVlanMember(const VlanMemberUpdate&); void updatePortOperState(const PortOperStateUpdate&); - bool addFdbEntry(const FdbEntry&, const string&); - bool removeFdbEntry(const FdbEntry&); - void flushFDBEntries(sai_object_id_t bridge_port_oid, - sai_object_id_t vlan_oid); + bool addFdbEntry(const FdbEntry&, const string&, const string&, FdbOrigin origin, const string& remote_ip="", unsigned int vni=0, const string& esi=""); + void deleteFdbEntryFromSavedFDB(const MacAddress &mac, const unsigned short &vlanId, FdbOrigin origin, const string portName=""); bool storeFdbEntryState(const FdbUpdate& update); + void notifyTunnelOrch(Port& port); }; #endif /* SWSS_FDBORCH_H */ diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 99fbc0a7f6..923bc7b80e 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -85,11 +85,15 @@ bool OrchDaemon::init() { APP_LAG_MEMBER_TABLE_NAME, portsorch_base_pri } }; + vector app_fdb_tables = { + { APP_FDB_TABLE_NAME, FdbOrch::fdborch_pri}, + { APP_VXLAN_FDB_TABLE_NAME, FdbOrch::fdborch_pri} + }; + gCrmOrch = new CrmOrch(m_configDb, CFG_CRM_TABLE_NAME); gPortsOrch = new PortsOrch(m_applDb, ports_tables); - TableConnector applDbFdb(m_applDb, APP_FDB_TABLE_NAME); TableConnector stateDbFdb(m_stateDb, STATE_FDB_TABLE_NAME); - gFdbOrch = new FdbOrch(applDbFdb, stateDbFdb, gPortsOrch); + gFdbOrch = new FdbOrch(m_applDb, app_fdb_tables, stateDbFdb, gPortsOrch); vector vnet_tables = { APP_VNET_RT_TABLE_NAME, diff --git a/tests/test_evpn_fdb.py b/tests/test_evpn_fdb.py new file mode 100644 index 0000000000..469494a279 --- /dev/null +++ b/tests/test_evpn_fdb.py @@ -0,0 +1,680 @@ +from swsscommon import swsscommon +import os +import sys +import time +import json +import pytest +from distutils.version import StrictVersion + +def create_entry(tbl, key, pairs): + fvs = swsscommon.FieldValuePairs(pairs) + tbl.set(key, fvs) + + # FIXME: better to wait until DB create them + time.sleep(1) + +def create_entry_tbl(db, table, key, pairs): + tbl = swsscommon.Table(db, table) + create_entry(tbl, key, pairs) + +def create_entry_pst(db, table, key, pairs): + tbl = swsscommon.ProducerStateTable(db, table) + create_entry(tbl, key, pairs) + +def delete_entry_pst(db, table, key): + tbl = swsscommon.ProducerStateTable(db, table) + tbl._del(key) + +def how_many_entries_exist(db, table): + tbl = swsscommon.Table(db, table) + return len(tbl.getKeys()) + +def remove_mac(db, table, mac, vlan): + tbl = swsscommon.Table(db, table) + tbl._del("Vlan" + vlan + "|" + mac.lower()) + time.sleep(1) + +def delete_entry_tbl(db, table, key): + tbl = swsscommon.Table(db, table) + tbl._del(key) + time.sleep(1) + +def create_evpn_nvo(db, nvoname, tnlname): + #conf_db = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0) + attrs = [ + ("source_vtep", tnlname), + ] + + # create the VXLAN tunnel Term entry in Config DB + create_entry_tbl( + db, + "EVPN_NVO", nvoname, + attrs, + ) + +def remove_evpn_nvo(db, nvoname): + #conf_db = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0) + delete_entry_tbl(db,"EVPN_NVO", nvoname,) + +def create_vxlan_tunnel(db, name, src_ip): + #conf_db = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0) + + attrs = [ + ("src_ip", src_ip), + ] + + # create the VXLAN tunnel Term entry in Config DB + create_entry_tbl( + db, + "VXLAN_TUNNEL", name, + attrs, + ) + +def remove_vxlan_tunnel(db, tnlname): + #conf_db = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0) + + # create the VXLAN tunnel Term entry in Config DB + delete_entry_tbl( + db, + "VXLAN_TUNNEL", tnlname, + ) + +def create_vxlan_tunnel_map(db, tnlname, mapname, vni_id, vlan_id): + #conf_db = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0) + + attrs = [ + ("vni", vni_id), + ("vlan", vlan_id), + ] + + # create the VXLAN tunnel Term entry in Config DB + create_entry_tbl( + db, + "VXLAN_TUNNEL_MAP", "%s|%s" % (tnlname, mapname), + attrs, + ) + +def remove_vxlan_tunnel_map(db, tnlname, mapname,vni_id, vlan_id): + #conf_db = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0) + + attrs = [ + ("vni", vni_id), + ("vlan", vlan_id), + ] + + # create the VXLAN tunnel Term entry in Config DB + delete_entry_tbl( + db, + "VXLAN_TUNNEL_MAP", "%s|%s" % (tnlname, mapname), + ) + +def create_evpn_remote_vni(db, vlan_id, remotevtep, vnid): + #app_db = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0) + create_entry_pst( + db, + "EVPN_REMOTE_VNI_TABLE", "%s:%s" % (vlan_id, remotevtep), + [ + ("vni", vnid), + ], + ) + +def remove_evpn_remote_vni(db, vlan_id, remotevtep ): + #app_db = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0) + delete_entry_pst( + db, + "EVPN_REMOTE_VNI_TABLE", "%s:%s" % (vlan_id, remotevtep), + ) + +def get_vxlan_p2p_tunnel_bp(db, remote_ip): + tnl_id = None + bp = None + print("remote_ip = " + remote_ip) + attributes = [("SAI_TUNNEL_TERM_TABLE_ENTRY_ATTR_TYPE", "SAI_TUNNEL_TERM_TABLE_ENTRY_TYPE_P2P"), + ("SAI_TUNNEL_TERM_TABLE_ENTRY_ATTR_TUNNEL_TYPE", "SAI_TUNNEL_TYPE_VXLAN"), + ("SAI_TUNNEL_TERM_TABLE_ENTRY_ATTR_SRC_IP", remote_ip) + ] + tbl = swsscommon.Table(db, "ASIC_STATE:SAI_OBJECT_TYPE_TUNNEL_TERM_TABLE_ENTRY") + keys = tbl.getKeys() + for key in keys: + status, fvs = tbl.get(key) + assert status, "Error reading from table ASIC_STATE:SAI_OBJECT_TYPE_TUNNEL_TERM_TABLE_ENTRY" + attrs = dict(attributes) + num_match = 0 + for k, v in fvs: + print("attr:value="+str(k)+":"+str(v)) + if k in attrs and attrs[k] == v: + num_match += 1 + if k == "SAI_TUNNEL_TERM_TABLE_ENTRY_ATTR_ACTION_TUNNEL_ID": + tnl_id = v + if num_match == len(attributes): + break + else: + tnl_id = None + + print("tnl_id = "+str(tnl_id)) + if tnl_id != None: + tbl = swsscommon.Table(db, "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT") + keys = tbl.getKeys() + for key in keys: + status, fvs = tbl.get(key) + assert status, "Error reading from table ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT" + for k, v in fvs: + print("attr:value="+str(k)+":"+str(v)) + if k == "SAI_BRIDGE_PORT_ATTR_TUNNEL_ID" and tnl_id == v: + bp = key + break + if bp != None: + break + else: + pass + print("bp = "+str(bp)) + return bp + + +def test_evpnFdb(dvs, testlog): + dvs.setup_db() + + dvs.runcmd("sonic-clear fdb all") + time.sleep(2) + + #Find switch_id + switch_id = dvs.getSwitchOid() + print("Switch_id="+str(switch_id)) + + vlan_before = how_many_entries_exist(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_VLAN") + + # create vlan + print("Creating Vlan3") + #dvs.runcmd("config vlan add 3") + dvs.create_vlan("3") + time.sleep(2) + + vlan_after = how_many_entries_exist(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_VLAN") + assert vlan_after - vlan_before == 1, "The Vlan3 wasn't created" + print("Vlan3 is created") + + # Find the vlan_oid to be used in DB communications + vlan_oid_3 = dvs.getVlanOid("3") + assert vlan_oid_3 is not None, "Could not find Vlan_oid" + print("Vlan-3 vlan_oid="+str(vlan_oid_3)) + + bp_before = how_many_entries_exist(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT") + vm_before = how_many_entries_exist(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_VLAN_MEMBER") + + print("Making Ethernet0 as a member of Vlan3") + #dvs.runcmd("config vlan member add 3 Ethernet0") + dvs.create_vlan_member("3", "Ethernet0") + time.sleep(2) + + # check that the vlan information was propagated + bp_after = how_many_entries_exist(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT") + vm_after = how_many_entries_exist(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_VLAN_MEMBER") + + assert bp_after - bp_before == 1, "The bridge port wasn't created" + assert vm_after - vm_before == 1, "The vlan member wasn't added" + print("Ethernet0 is a member of Vlan3") + + # Get mapping between interface name and its bridge port_id + iface_2_bridge_port_id = dvs.get_map_iface_bridge_port_id(dvs.adb) + + #create SIP side of tunnel + source_tnl_name = "source_vtep_name" + source_tnl_ip = "7.7.7.7" + create_vxlan_tunnel(dvs.cdb, source_tnl_name, source_tnl_ip) + time.sleep(1) + + nvo_name = "evpn_nvo" + create_evpn_nvo(dvs.cdb, nvo_name, source_tnl_name) + time.sleep(1) + + map_name_vlan_3 = "map_3_3" + create_vxlan_tunnel_map(dvs.cdb, source_tnl_name, map_name_vlan_3, "3", "Vlan3") + time.sleep(1) + + remote_ip_6 = "6.6.6.6" + create_evpn_remote_vni(dvs.pdb, "Vlan3", remote_ip_6, "3") + remote_ip_8 = "8.8.8.8" + create_evpn_remote_vni(dvs.pdb, "Vlan3", remote_ip_8, "3") + time.sleep(1) + + #UT-1 Evpn Mac add from remote when tunnels are already created + mac = "52:54:00:25:06:E9" + print("Creating Evpn FDB Vlan3:"+mac.lower()+":6.6.6.6 in APP-DB") + create_entry_pst( + dvs.pdb, + "VXLAN_FDB_TABLE", "Vlan3:"+mac.lower(), + [ + ("remote_vtep", remote_ip_6), + ("type", "dynamic"), + ("vni", "3") + ] + ) + time.sleep(1) + + tnl_bp_oid_6 = get_vxlan_p2p_tunnel_bp(dvs.adb, remote_ip_6) + tnl_bp_oid_8 = get_vxlan_p2p_tunnel_bp(dvs.adb, remote_ip_8) + + # check that the FDB entry is inserted into ASIC DB + ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", + [("mac", mac), ("bvid", vlan_oid_3)], + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC_MACMOVE"), + ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_6), + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_6)), + ] + ) + assert ok == True, str(extra) + print("EVPN FDB Vlan3:"+mac.lower()+":"+remote_ip_6+" is created in ASIC-DB") + + time.sleep(1) + + #UT-2 Evpn Mac del from remote + mac = "52:54:00:25:06:E9" + print("Deleting Evpn FDB Vlan3:"+mac.lower()+":6.6.6.6 in APP-DB") + delete_entry_pst( + dvs.pdb, + "VXLAN_FDB_TABLE", "Vlan3:"+mac.lower() + ) + time.sleep(1) + + # check that the FDB entry is deleted from ASIC DB + ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", + [("mac", mac), ("bvid", vlan_oid_3)], + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC_MACMOVE"), + ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_6), + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_6)), + ] + ) + assert ok == False, str(extra) + print("EVPN FDB Vlan3:"+mac.lower()+":"+remote_ip_6+" is deleted from ASIC-DB") + + time.sleep(1) + + #UT-3 Evpn Mac add from remote when local mac is already present + mac = "52:54:00:25:06:E9" + + print("Creating Local dynamic FDB Vlan3:"+mac.lower()+":Ethernet0 in APP-DB") + # Create Dynamic MAC entry in APP DB + create_entry_pst( + dvs.pdb, + "FDB_TABLE", "Vlan3:"+mac.lower(), + [ + ("port", "Ethernet0"), + ("type", "dynamic"), + ] + ) + + time.sleep(1) + #raw_input("Check ASIC_DB.........") + + # check that the FDB entry was added in ASIC DB + ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", + [("mac", mac), ("bvid", vlan_oid_3)], + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_DYNAMIC"), + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", iface_2_bridge_port_id["Ethernet0"])] + ) + assert ok, str(extra) + print("Dynamic FDB Vlan3:"+mac.lower()+":Ethernet0 is created in Asic-DB") + + # check that the FDB entry was added in STATE DB + mac1_found, extra = dvs.is_table_entry_exists(dvs.sdb, "FDB_TABLE", + "Vlan3:"+mac.lower(), + [("port", "Ethernet0"), + ("type", "dynamic"), + ] + ) + assert mac1_found, str(extra) + print("FDB Vlan3:"+mac+":Ethernet0 is created in STATE-DB") + + print("Creating Evpn FDB Vlan3:"+mac.lower()+":6.6.6.6 in APP-DB") + create_entry_pst( + dvs.pdb, + "VXLAN_FDB_TABLE", "Vlan3:"+mac.lower(), + [ + ("remote_vtep", remote_ip_6), + ("type", "dynamic"), + ("vni", "3") + ] + ) + time.sleep(1) + + #raw_input("Check ASIC_DB.........") + + # check that the FDB entry is inserted into ASIC DB + ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", + [("mac", mac), ("bvid", str(dvs.getVlanOid("3")))], + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC_MACMOVE"), + ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_6), + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_6)), + ] + ) + assert ok, str(extra) + print("EVPN FDB Vlan3:"+mac.lower()+":"+remote_ip_6+" is created in ASIC-DB") + + # check that the Local FDB entry is deleted from STATE DB + mac1_found, extra = dvs.is_table_entry_exists(dvs.sdb, "FDB_TABLE", + "Vlan3:"+mac.lower(), + [("port", "Ethernet0"), + ("type", "dynamic"), + ] + ) + assert mac1_found == False, str(extra) + print("FDB Vlan3:"+mac+":Ethernet0 is deleted from STATE-DB") + + time.sleep(1) + + #UT-4 Evpn Sticky Mac add from remote + mac = "52:54:00:25:06:E9" + print("Creating Evpn Sticky FDB Vlan3:"+mac.lower()+":6.6.6.6 in APP-DB") + create_entry_pst( + dvs.pdb, + "VXLAN_FDB_TABLE", "Vlan3:"+mac.lower(), + [ + ("remote_vtep", remote_ip_6), + ("type", "static"), + ("vni", "3") + ] + ) + time.sleep(1) + + #raw_input("Check ASIC_DB.........") + + # check that the FDB entry is inserted into ASIC DB + ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", + [("mac", mac), ("bvid", str(dvs.getVlanOid("3")))], + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), + ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_6), + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_6)), + ] + ) + assert ok, str(extra) + print("EVPN Sticky FDB Vlan3:"+mac.lower()+":"+remote_ip_6+" is created in ASIC-DB") + + #raw_input("Check ASIC_DB.........") + + #UT-5 create a Static FDB entry + mac = "52:54:00:25:06:E9" + print("Creating static FDB Vlan3:"+mac.lower()+":Ethernet0 in CONFIG-DB") + create_entry_tbl( + dvs.cdb, + "FDB", "Vlan3|"+mac.lower(), + [ + ("port", "Ethernet0"), + ] + ) + time.sleep(2) + + #raw_input("Check ASIC_DB.........") + + # check that the FDB entry was added in APP DB + mac1_found, extra = dvs.is_table_entry_exists(dvs.pdb, "FDB_TABLE", + "Vlan3:"+mac.lower(), + [("port", "Ethernet0"), + ("type", "static"), + ] + ) + assert mac1_found, str(extra) + print("Static FDB Vlan3:"+mac.lower()+"Ethernet0 is created in APP-DB") + + # check that the FDB entry is now inserted into ASIC DB + ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", + [("mac", mac.upper()), ("bvid", vlan_oid_3)], + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", iface_2_bridge_port_id["Ethernet0"])] + ) + assert ok, str(extra) + print("Static FDB Vlan3:"+mac.lower()+":Ethernet0 is created in ASIC-DB") + + # check that the FDB entry was added in STATE DB + mac1_found, extra = dvs.is_table_entry_exists(dvs.sdb, "FDB_TABLE", + "Vlan3:"+mac.lower(), + [("port", "Ethernet0"), + ("type", "static"), + ] + ) + assert mac1_found, str(extra) + print("Static FDB Vlan3:"+mac.lower()+":Ethernet0 is created in STATE-DB") + + #raw_input("Check ASIC_DB.........") + + #UT-6 Evpn Mac del from remote when only local is present; local mac should not get affected + mac = "52:54:00:25:06:E9" + print("Deleting Evpn FDB Vlan3:"+mac.lower()+":6.6.6.6 in APP-DB") + delete_entry_pst( + dvs.pdb, + "VXLAN_FDB_TABLE", "Vlan3:"+mac.lower() + ) + time.sleep(1) + + #raw_input("Check ASIC_DB.........") + + # check that the existing local fdb entry is not deleted and still available in ASIC DB + ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", + [("mac", mac), ("bvid", vlan_oid_3)], + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", iface_2_bridge_port_id["Ethernet0"]), + ] + ) + assert ok, str(extra) + print("Local FDB Vlan3:"+mac.lower()+":Ethernet0 is not deleted from ASIC-DB") + + # check that the local FDB entry is still available in STATE DB + mac1_found, extra = dvs.is_table_entry_exists(dvs.sdb, "FDB_TABLE", + "Vlan3:"+mac.lower(), + [("port", "Ethernet0"), + ("type", "static"), + ] + ) + assert mac1_found, str(extra) + print("Local FDB Vlan3:"+mac.lower()+":Ethernet0 is not deleted STATE-DB") + + time.sleep(1) + + #UT-7 Evpn Mac add from remote when local static mac is already available + mac = "52:54:00:25:06:E9" + print("Creating Evpn dynamic FDB Vlan3:"+mac.lower()+":6.6.6.6 in APP-DB") + create_entry_pst( + dvs.pdb, + "VXLAN_FDB_TABLE", "Vlan3:"+mac.lower(), + [ + ("remote_vtep", remote_ip_6), + ("type", "dynamic"), + ("vni", "3") + ] + ) + time.sleep(1) + + #raw_input("Check ASIC_DB.........") + + # check that the FDB entry is not inserted into ASIC DB + ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", + [("mac", mac), ("bvid", vlan_oid_3)], + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC_MACMOVE"), + ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_6), + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_6)), + ] + ) + assert ok == False, str(extra) + print("EVPN dynamic FDB Vlan3:"+mac.lower()+":"+remote_ip_6+" is not created in ASIC-DB") + + time.sleep(1) + + print("Creating Evpn sticky FDB Vlan3:"+mac.lower()+":6.6.6.6 in APP-DB") + create_entry_pst( + dvs.pdb, + "VXLAN_FDB_TABLE", "Vlan3:"+mac.lower(), + [ + ("remote_vtep", remote_ip_6), + ("type", "static"), + ("vni", "3") + ] + ) + time.sleep(1) + + #raw_input("Check ASIC_DB.........") + + # check that the FDB entry is not inserted into ASIC DB + ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", + [("mac", mac), ("bvid", vlan_oid_3)], + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), + ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_6), + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_6)), + ] + ) + assert ok == False, str(extra) + print("EVPN sticky FDB Vlan3:"+mac.lower()+":"+remote_ip_6+" is not created in ASIC-DB") + + # Delete Static FDB entry + mac = "52:54:00:25:06:E9" + print("Deleting static FDB Vlan3:"+mac.lower()+":Ethernet0 from CONFIG-DB") + delete_entry_tbl( + dvs.cdb, + "FDB", "Vlan3|"+mac.lower() + ) + time.sleep(2) + + #raw_input("Check ASIC_DB.........") + + # check that the FDB entry was deleted in APP DB + mac1_found, extra = dvs.is_table_entry_exists(dvs.pdb, "FDB_TABLE", + "Vlan3:"+mac.lower(), + [("port", "Ethernet0"), + ("type", "static"), + ] + ) + assert mac1_found == False, str(extra) + print("Static FDB Vlan3:"+mac.lower()+"Ethernet0 is deleted from APP-DB") + + # check that the FDB entry is deleted in ASIC DB + ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", + [("mac", mac.upper()), ("bvid", vlan_oid_3)], + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", iface_2_bridge_port_id["Ethernet0"])] + ) + assert ok == False, str(extra) + print("Static FDB Vlan3:"+mac.lower()+":Ethernet0 is deleted in ASIC-DB") + + # check that the FDB entry was deleted from STATE DB + mac1_found, extra = dvs.is_table_entry_exists(dvs.sdb, "FDB_TABLE", + "Vlan3:"+mac.lower(), + [("port", "Ethernet0"), + ("type", "static"), + ] + ) + assert mac1_found == False, str(extra) + print("Static FDB Vlan3:"+mac.lower()+":Ethernet0 is deleted from STATE-DB") + + #raw_input("Check ASIC_DB.........") + + #UT-8 Evpn Mac add from remote when tunnels are already created + mac = "52:54:00:25:06:E9" + print("Creating Evpn FDB Vlan3:"+mac.lower()+":6.6.6.6 in APP-DB") + create_entry_pst( + dvs.pdb, + "VXLAN_FDB_TABLE", "Vlan3:"+mac.lower(), + [ + ("remote_vtep", remote_ip_6), + ("type", "dynamic"), + ("vni", "3") + ] + ) + time.sleep(1) + + #raw_input("Check ASIC_DB.........") + + # check that the FDB entry is inserted into ASIC DB + ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", + [("mac", mac), ("bvid", vlan_oid_3)], + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC_MACMOVE"), + ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_6), + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_6)), + ] + ) + assert ok == True, str(extra) + print("EVPN FDB Vlan3:"+mac.lower()+":"+remote_ip_6+" is created in ASIC-DB") + + time.sleep(1) + + tnl_bp_oid_8 = get_vxlan_p2p_tunnel_bp(dvs.adb, remote_ip_8) + + print("Creating Evpn FDB Vlan3:"+mac.lower()+":8.8.8.8 in APP-DB") + create_entry_pst( + dvs.pdb, + "VXLAN_FDB_TABLE", "Vlan3:"+mac.lower(), + [ + ("remote_vtep", remote_ip_8), + ("type", "dynamic"), + ("vni", "3") + ] + ) + time.sleep(1) + + #raw_input("Check ASIC_DB.........") + + # check that the FDB entry is inserted into ASIC DB + ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", + [("mac", mac), ("bvid", vlan_oid_3)], + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC_MACMOVE"), + ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_8), + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_8)), + ] + ) + assert ok == True, str(extra) + print("EVPN FDB Vlan3:"+mac.lower()+":"+remote_ip_8+" is created in ASIC-DB") + + time.sleep(1) + + #UT-9 Local mac move (delete and learn) when remote is already added + mac = "52:54:00:25:06:E9" + print("Deleting FDB Vlan3:52-54-00-25-06-E9:8.8.8.8 in ASIC-DB") + delete_entry_tbl(dvs.adb, "ASIC_STATE", "SAI_OBJECT_TYPE_FDB_ENTRY:{\"bvid\":\""+vlan_oid_3+"\",\"mac\":\""+mac+"\",\"switch_id\":\""+switch_id+"\"}") + + ntf = swsscommon.NotificationProducer(dvs.adb, "FDB_NOTIFICATIONS") + fvp = swsscommon.FieldValuePairs() + ntf_data = "[{\"fdb_entry\":\"{\\\"bvid\\\":\\\""+vlan_oid_3+"\\\",\\\"mac\\\":\\\""+mac+"\\\",\\\"switch_id\\\":\\\""+switch_id+"\\\"}\",\"fdb_event\":\"SAI_FDB_EVENT_AGED\",\"list\":[{\"id\":\"SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID\",\"value\":\""+str(tnl_bp_oid_8)+"\"}]}]" + ntf.send("fdb_event", ntf_data, fvp) + + time.sleep(2) + + #raw_input("Check ASIC_DB.........") + + print("Creating FDB Vlan3:52-54-00-25-06-E9:Ethernet0 in ASIC-DB") + create_entry_tbl( + dvs.adb, + "ASIC_STATE", "SAI_OBJECT_TYPE_FDB_ENTRY:{\"bvid\":\""+vlan_oid_3+"\",\"mac\":\"52:54:00:25:06:E9\",\"switch_id\":\""+switch_id+"\"}", + [ + ("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_DYNAMIC"), + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", iface_2_bridge_port_id["Ethernet0"]), + ] + ) + + ntf = swsscommon.NotificationProducer(dvs.adb, "FDB_NOTIFICATIONS") + fvp = swsscommon.FieldValuePairs() + ntf_data = "[{\"fdb_entry\":\"{\\\"bvid\\\":\\\""+vlan_oid_3+"\\\",\\\"mac\\\":\\\"52:54:00:25:06:E9\\\",\\\"switch_id\\\":\\\""+switch_id+"\\\"}\",\"fdb_event\":\"SAI_FDB_EVENT_LEARNED\",\"list\":[{\"id\":\"SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID\",\"value\":\""+iface_2_bridge_port_id["Ethernet0"]+"\"}]}]" + ntf.send("fdb_event", ntf_data, fvp) + + time.sleep(2) + + #raw_input("Check ASIC_DB.........") + + # check that the FDB entry was added in ASIC DB + ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", + [("mac", "52:54:00:25:06:E9"), ("bvid", vlan_oid_3)], + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_DYNAMIC"), + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", iface_2_bridge_port_id["Ethernet0"])] + ) + assert ok, str(extra) + print("FDB Vlan3:52-54-00-25-06-E9:Ethernet0 is created in ASIC-DB") + + # check that the FDB entry was added in STATE DB + mac1_found, extra = dvs.is_table_entry_exists(dvs.sdb, "FDB_TABLE", + "Vlan3:52:54:00:25:06:e9", + [("port", "Ethernet0"), + ("type", "dynamic"), + ] + ) + assert mac1_found, str(extra) + print("FDB Vlan3:52-54-00-25-06-E9:Ethernet0 is created in STATE-DB") + + + dvs.remove_vlan_member("3", "Ethernet0") + dvs.remove_vlan("3") From 84642fc3ce2248c9bf78d6cc6930f57a0c5acb84 Mon Sep 17 00:00:00 2001 From: Pankaj Jain Date: Tue, 23 Jun 2020 04:59:50 -0700 Subject: [PATCH 02/21] This commit contains: 1. Review comments incorporated 2. Dependency for PR #885 is resolved 3. Added test_evpn_fdb.py pytest script to verify EVPN-VXLAN-FDB 4. compiled and verified the FDB functionality with EVPN 5. Changes to handle the static mac and aging from CLI --- cfgmgr/vlanmgr.cpp | 220 +++++++++++++++++-- cfgmgr/vlanmgr.h | 7 + cfgmgr/vlanmgrd.cpp | 2 + orchagent/fdborch.cpp | 26 ++- orchagent/port.h | 5 + orchagent/portsorch.cpp | 359 ++++++++++++++++++++++++-------- orchagent/portsorch.h | 18 +- tests/conftest.py | 18 ++ tests/mock_tests/aclorch_ut.cpp | 8 +- tests/test_evpn_fdb.py | 51 +++-- tests/test_fdb.py | 4 +- tests/test_fdb_update.py | 6 +- 12 files changed, 585 insertions(+), 139 deletions(-) diff --git a/cfgmgr/vlanmgr.cpp b/cfgmgr/vlanmgr.cpp index 7d7a59abeb..509fbea47e 100644 --- a/cfgmgr/vlanmgr.cpp +++ b/cfgmgr/vlanmgr.cpp @@ -13,10 +13,12 @@ using namespace swss; #define DOT1Q_BRIDGE_NAME "Bridge" #define VLAN_PREFIX "Vlan" +#define SWITCH_STR "switch" #define LAG_PREFIX "PortChannel" #define DEFAULT_VLAN_ID "1" #define DEFAULT_MTU_STR "9100" #define VLAN_HLEN 4 +#define MAX_VLAN_ID 4095 extern MacAddress gMacAddress; @@ -28,6 +30,8 @@ VlanMgr::VlanMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c m_stateLagTable(stateDb, STATE_LAG_TABLE_NAME), m_stateVlanTable(stateDb, STATE_VLAN_TABLE_NAME), m_stateVlanMemberTable(stateDb, STATE_VLAN_MEMBER_TABLE_NAME), + m_appFdbTableProducer(appDb, APP_FDB_TABLE_NAME), + m_appSwitchTableProducer(appDb, APP_SWITCH_TABLE_NAME), m_appVlanTableProducer(appDb, APP_VLAN_TABLE_NAME), m_appVlanMemberTableProducer(appDb, APP_VLAN_MEMBER_TABLE_NAME) { @@ -88,6 +92,10 @@ VlanMgr::VlanMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c EXEC_WITH_ERROR_THROW(echo_cmd_backup, res); } + /* vlan state notification from portsorch */ + m_VlanStateNotificationConsumer = new swss::NotificationConsumer(appDb, "VLANSTATE"); + auto vlanStatusNotificatier = new Notifier(m_VlanStateNotificationConsumer, this, "VLANSTATE"); + Orch::addExecutor(vlanStatusNotificatier); } bool VlanMgr::addHostVlan(int vlan_id) @@ -101,7 +109,7 @@ bool VlanMgr::addHostVlan(int vlan_id) + BASH_CMD + " -c \"" + BRIDGE_CMD + " vlan add vid " + std::to_string(vlan_id) + " dev " + DOT1Q_BRIDGE_NAME + " self && " + IP_CMD + " link add link " + DOT1Q_BRIDGE_NAME - + " up" + + " down" + " name " + VLAN_PREFIX + std::to_string(vlan_id) + " address " + gMacAddress.to_string() + " type vlan id " + std::to_string(vlan_id) + "\""; @@ -179,14 +187,29 @@ bool VlanMgr::addHostVlanMember(int vlan_id, const string &port_alias, const str // /bin/bash -c "/sbin/ip link set {{port_alias}} master Bridge && // /sbin/bridge vlan del vid 1 dev {{ port_alias }} && // /sbin/bridge vlan add vid {{vlan_id}} dev {{port_alias}} {{tagging_mode}}" - ostringstream cmds, inner; - inner << IP_CMD " link set " << shellquote(port_alias) << " master " DOT1Q_BRIDGE_NAME " && " - BRIDGE_CMD " vlan del vid " DEFAULT_VLAN_ID " dev " << shellquote(port_alias) << " && " - BRIDGE_CMD " vlan add vid " + std::to_string(vlan_id) + " dev " << shellquote(port_alias) << " " + tagging_cmd; - cmds << BASH_CMD " -c " << shellquote(inner.str()); + + const std::string key = std::string("") + "Vlan1|" + port_alias; - std::string res; - EXEC_WITH_ERROR_THROW(cmds.str(), res); + if (isVlanMemberStateOk(key)) { + ostringstream cmds, inner; + inner << IP_CMD " link set " << shellquote(port_alias) << " master " DOT1Q_BRIDGE_NAME " && " + BRIDGE_CMD " vlan add vid " + std::to_string(vlan_id) + " dev " << shellquote(port_alias) << " " + tagging_cmd; + cmds << BASH_CMD " -c " << shellquote(inner.str()); + + std::string res; + EXEC_WITH_ERROR_THROW(cmds.str(), res); + } + else + { + ostringstream cmds, inner; + inner << IP_CMD " link set " << shellquote(port_alias) << " master " DOT1Q_BRIDGE_NAME " && " + BRIDGE_CMD " vlan del vid " DEFAULT_VLAN_ID " dev " << shellquote(port_alias) << " && " + BRIDGE_CMD " vlan add vid " + std::to_string(vlan_id) + " dev " << shellquote(port_alias) << " " + tagging_cmd; + cmds << BASH_CMD " -c " << shellquote(inner.str()); + + std::string res; + EXEC_WITH_ERROR_THROW(cmds.str(), res); + } return true; } @@ -224,6 +247,137 @@ bool VlanMgr::isVlanMacOk() return !!gMacAddress; } +void VlanMgr::doSwitchTask(Consumer &consumer) +{ + SWSS_LOG_ENTER(); + auto it = consumer.m_toSync.begin(); + + while (it != consumer.m_toSync.end()) + { + KeyOpFieldsValuesTuple t = it->second; + + string key = kfvKey(t); + string op = kfvOp(t); + + /* Ensure the key starts with "switch" otherwise ignore */ + if (key != SWITCH_STR) + { + SWSS_LOG_NOTICE("Ignoring SWITCH key %s", key.c_str()); + it = consumer.m_toSync.erase(it); + continue; + } + + SWSS_LOG_DEBUG("key:switch"); + + for (auto i : kfvFieldsValues(t)) + { + if (fvField(i) == "fdb_aging_time") + { + long agingTime = 0; + SWSS_LOG_DEBUG("attribute:fdb_aging_time"); + if (op == SET_COMMAND) + { + SWSS_LOG_DEBUG("operation:set"); + agingTime = strtol(fvValue(i).c_str(), NULL, 0); + if (agingTime < 0) + { + SWSS_LOG_ERROR("Invalid fdb_aging_time %s", fvValue(i).c_str()); + break; + } + SWSS_LOG_DEBUG("value:%s",fvValue(i).c_str()); + } + else if (op == DEL_COMMAND) + { + SWSS_LOG_DEBUG("operation:del"); + agingTime = 0; + } + else + { + SWSS_LOG_ERROR("Unknown operation type %s", op.c_str()); + break; + } + + vector fvVector; + FieldValueTuple aging_time("fdb_aging_time", to_string(agingTime)); + fvVector.push_back(aging_time); + m_appSwitchTableProducer.set(key, fvVector); + break; + } + } + + it = consumer.m_toSync.erase(it); + } +} + +void VlanMgr::doFdbTask(Consumer &consumer) +{ + auto it = consumer.m_toSync.begin(); + + while (it != consumer.m_toSync.end()) + { + KeyOpFieldsValuesTuple t = it->second; + + /* format: | */ + vector keys = tokenize(kfvKey(t), config_db_key_delimiter, 1); + /* keys[0] is vlan as (Vlan10) and keys[1] is mac as (00-00-00-00-00-00) */ + string op = kfvOp(t); + + /* Ensure the key starts with "Vlan" otherwise ignore */ + if (strncmp(keys[0].c_str(), VLAN_PREFIX, 4)) + { + SWSS_LOG_ERROR("Invalid key format. No 'Vlan' prefix: %s", keys[0].c_str()); + it = consumer.m_toSync.erase(it); + continue; + } + + unsigned long vlan_id; + vlan_id = strtoul(keys[0].substr(strlen(VLAN_PREFIX)).c_str(), NULL, 0); + + if ((vlan_id <= 0) || (vlan_id > MAX_VLAN_ID)) + { + SWSS_LOG_ERROR("Invalid key format. Vlan is out of range: %s", keys[0].c_str()); + it = consumer.m_toSync.erase(it); + continue; + } + + MacAddress mac = MacAddress(keys[1]); + + string key = VLAN_PREFIX + to_string(vlan_id); + key += DEFAULT_KEY_SEPARATOR; + key += mac.to_string(); + + if (op == SET_COMMAND) + { + string port; + for (auto i : kfvFieldsValues(t)) + { + if (fvField(i) == "port") + { + port = fvValue(i); + break; + } + } + + vector fvVector; + FieldValueTuple p("port", port); + fvVector.push_back(p); + FieldValueTuple t("type", "static"); + fvVector.push_back(t); + + m_appFdbTableProducer.set(key, fvVector); + } + else if (op == DEL_COMMAND) + { + m_appFdbTableProducer.del(key); + } + else + { + SWSS_LOG_ERROR("Unknown operation type %s", op.c_str()); + } + it = consumer.m_toSync.erase(it); + } +} + void VlanMgr::doVlanTask(Consumer &consumer) { if (!isVlanMacOk()) @@ -296,12 +450,12 @@ void VlanMgr::doVlanTask(Consumer &consumer) { /* Set vlan admin status */ if (fvField(i) == "admin_status") - { - admin_status = fvValue(i); - setHostVlanAdminState(vlan_id, admin_status); - fvVector.push_back(i); - } - /* Set vlan mtu */ + { + admin_status = fvValue(i); + setHostVlanAdminState(vlan_id, admin_status); + fvVector.push_back(i); + } + /* Set vlan mtu */ else if (fvField(i) == "mtu") { mtu = fvValue(i); @@ -593,9 +747,47 @@ void VlanMgr::doTask(Consumer &consumer) { doVlanMemberTask(consumer); } + else if (table_name == CFG_FDB_TABLE_NAME) + { + doFdbTask(consumer); + } + else if (table_name == CFG_SWITCH_TABLE_NAME) + { + SWSS_LOG_DEBUG("Table:SWITCH"); + doSwitchTask(consumer); + } else { SWSS_LOG_ERROR("Unknown config table %s ", table_name.c_str()); throw runtime_error("VlanMgr doTask failure."); } } + +void VlanMgr::doTask(NotificationConsumer &consumer) +{ + std::string op; + std::string data; + std::vector values; + + if (&consumer != m_VlanStateNotificationConsumer) + { + SWSS_LOG_WARN("received incorrect notification message"); + return; + } + + consumer.pop(op, data, values); + + unsigned long vlan_id = strtoul(data.substr(strlen(VLAN_PREFIX)).c_str(), NULL, 0); + + SWSS_LOG_NOTICE("vlanmgr received port status notification state %s vlan %s", + op.c_str(), data.c_str()); + + if (isVlanStateOk(data)) + { + setHostVlanAdminState((int)vlan_id, op); + } + else + { + SWSS_LOG_ERROR("received state update for vlan %s not existing", data.c_str()); + } +} diff --git a/cfgmgr/vlanmgr.h b/cfgmgr/vlanmgr.h index 469f16ccaf..861254e36c 100644 --- a/cfgmgr/vlanmgr.h +++ b/cfgmgr/vlanmgr.h @@ -4,6 +4,7 @@ #include "dbconnector.h" #include "producerstatetable.h" #include "orch.h" +#include "notifier.h" #include #include @@ -19,14 +20,20 @@ class VlanMgr : public Orch private: ProducerStateTable m_appVlanTableProducer, m_appVlanMemberTableProducer; + ProducerStateTable m_appFdbTableProducer; + ProducerStateTable m_appSwitchTableProducer; Table m_cfgVlanTable, m_cfgVlanMemberTable; Table m_statePortTable, m_stateLagTable; Table m_stateVlanTable, m_stateVlanMemberTable; std::set m_vlans; + NotificationConsumer* m_VlanStateNotificationConsumer; void doTask(Consumer &consumer); + void doTask(NotificationConsumer &consumer); void doVlanTask(Consumer &consumer); void doVlanMemberTask(Consumer &consumer); + void doFdbTask(Consumer &consumer); + void doSwitchTask(Consumer &consumer); void processUntaggedVlanMembers(std::string vlan, const std::string &members); bool addHostVlan(int vlan_id); diff --git a/cfgmgr/vlanmgrd.cpp b/cfgmgr/vlanmgrd.cpp index 88e4745758..23d25df819 100644 --- a/cfgmgr/vlanmgrd.cpp +++ b/cfgmgr/vlanmgrd.cpp @@ -51,6 +51,8 @@ int main(int argc, char **argv) vector cfg_vlan_tables = { CFG_VLAN_TABLE_NAME, CFG_VLAN_MEMBER_TABLE_NAME, + CFG_FDB_TABLE_NAME, + CFG_SWITCH_TABLE_NAME, }; DBConnector cfgDb("CONFIG_DB", 0); diff --git a/orchagent/fdborch.cpp b/orchagent/fdborch.cpp index 00301d511e..4321d1965b 100644 --- a/orchagent/fdborch.cpp +++ b/orchagent/fdborch.cpp @@ -469,7 +469,7 @@ bool FdbOrch::getPort(const MacAddress& mac, uint16_t vlan, Port& port) if (!m_portsOrch->getPortByBridgePortId(bridge_port_id, port)) { - SWSS_LOG_ERROR("Failed to get port by bridge port ID 0x%" PRIx64, attr.value.oid); + SWSS_LOG_ERROR("Failed to get port by bridge port ID 0x%" PRIx64, bridge_port_id); return false; } @@ -866,7 +866,7 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const attr.id = SAI_FDB_ENTRY_ATTR_TYPE; if (origin == FDB_ORIGIN_VXLAN_ADVERTIZED) { - attr.value.s32 = (type == "dynamic") ? SAI_FDB_ENTRY_TYPE_STATIC_MACMOVE : SAI_FDB_ENTRY_TYPE_STATIC; + attr.value.s32 = SAI_FDB_ENTRY_TYPE_STATIC; } else { @@ -874,6 +874,13 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const } attrs.push_back(attr); + if ((origin == FDB_ORIGIN_VXLAN_ADVERTIZED) && (type == "dynamic")) + { + attr.id = SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE; + attr.value.booldata = true; + 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); @@ -896,7 +903,9 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const attr.value.ipaddr = ipaddr; attrs.push_back(attr); } - else if(macUpdate && (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED) && (origin != oldOrigin)) + else if(macUpdate + && (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED) + && (origin != oldOrigin)) { /* origin is changed from Remote-advertized to Local-provisioned * Remove the end-point ip attribute from fdb entry @@ -909,7 +918,16 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const attrs.push_back(attr); } - string key = "Vlan" + to_string(vlan.m_vlan_info.vlan_id) + ":" + entry.mac.to_string(); + if(macUpdate && (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED)) + { + if((origin != oldOrigin) + || ((oldType == "dynamic") && (oldType != type))) + { + attr.id = SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE; + attr.value.booldata = false; + attrs.push_back(attr); + } + } if(macUpdate) diff --git a/orchagent/port.h b/orchagent/port.h index 74f805ddec..26d0bcc4c8 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -24,6 +24,7 @@ namespace swss { struct VlanMemberEntry { + std::string alias; sai_object_id_t vlan_member_id; sai_vlan_tagging_mode_t vlan_mode; }; @@ -82,6 +83,7 @@ class Port sai_port_fec_mode_t m_fec_mode = SAI_PORT_FEC_MODE_NONE; VlanInfo m_vlan_info; sai_object_id_t m_bridge_port_id = 0; // TODO: port could have multiple bridge port IDs + sai_object_id_t m_bridge_port_admin_state = 0; // TODO: port could have multiple bridge port IDs sai_vlan_id_t m_port_vlan_id = DEFAULT_PORT_VLAN_ID; // Port VLAN ID sai_object_id_t m_rif_id = 0; sai_object_id_t m_vr_id = 0; @@ -102,6 +104,9 @@ class Port uint8_t m_pfc_bitmask = 0; uint32_t m_nat_zone_id = 0; + uint32_t m_fdb_count = 0; + uint32_t m_up_member_count = 0; + /* * Following two bit vectors are used to lock * the PG/queue from being changed in BufferOrch. diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 5ceed76760..bfffa4614e 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -25,6 +25,7 @@ #include "crmorch.h" #include "countercheckorch.h" #include "notifier.h" +#include "fdborch.h" #include "redisclient.h" extern sai_switch_api_t *sai_switch_api; @@ -41,6 +42,8 @@ extern IntfsOrch *gIntfsOrch; extern NeighOrch *gNeighOrch; extern CrmOrch *gCrmOrch; extern BufferOrch *gBufferOrch; +extern FdbOrch *gFdbOrch; + #define VLAN_PREFIX "Vlan" #define DEFAULT_VLAN_ID 1 @@ -202,15 +205,15 @@ PortsOrch::PortsOrch(DBConnector *db, vector &tableNames) /* Initialize counter table */ m_counter_db = shared_ptr(new DBConnector("COUNTERS_DB", 0)); m_counterTable = unique_ptr(new Table(m_counter_db.get(), COUNTERS_PORT_NAME_MAP)); - m_counterLagTable = unique_ptr
(new Table(m_counter_db.get(), COUNTERS_LAG_NAME_MAP)); FieldValueTuple tuple("", ""); vector defaultLagFv; defaultLagFv.push_back(tuple); m_counterLagTable->set("", defaultLagFv); - /* Initialize port table */ + /* Initialize port and vlan table */ m_portTable = unique_ptr
(new Table(db, APP_PORT_TABLE_NAME)); + m_vlanTable = unique_ptr
(new Table(db, APP_VLAN_TABLE_NAME)); /* Initialize gearbox */ m_gearboxTable = unique_ptr
(new Table(db, "_GEARBOX_TABLE")); @@ -231,6 +234,7 @@ PortsOrch::PortsOrch(DBConnector *db, vector &tableNames) m_flexCounterGroupTable = unique_ptr(new ProducerTable(m_flex_db.get(), FLEX_COUNTER_GROUP_TABLE)); initGearbox(); + notifications = new swss::NotificationProducer(db, "VLANSTATE"); string queueWmSha, pgWmSha; string queueWmPluginName = "watermark_queue.lua"; @@ -365,7 +369,19 @@ PortsOrch::PortsOrch(DBConnector *db, vector &tableNames) } m_default1QBridge = attrs[0].value.oid; - m_defaultVlan = attrs[1].value.oid; + m_defaultVlan_ObjId = attrs[1].value.oid; + + memset(&attr, 0x00, sizeof(attr)); + attr.id = SAI_VLAN_ATTR_VLAN_ID; + + status = sai_vlan_api->get_vlan_attribute(m_defaultVlan_ObjId, 1, &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to get default VLAN ID, rv:%d", status); + throw runtime_error("PortsOrch initialization failure"); + } + + m_defaultVlan_Id = attr.value.u16; removeDefaultVlanMembers(); removeDefaultBridgePorts(); @@ -387,7 +403,7 @@ void PortsOrch::removeDefaultVlanMembers() attr.value.objlist.count = (uint32_t)vlan_member_list.size(); attr.value.objlist.list = vlan_member_list.data(); - sai_status_t status = sai_vlan_api->get_vlan_attribute(m_defaultVlan, 1, &attr); + sai_status_t status = sai_vlan_api->get_vlan_attribute(m_defaultVlan_ObjId, 1, &attr); if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to get VLAN member list in default VLAN, rv:%d", status); @@ -546,34 +562,12 @@ bool PortsOrch::getPort(sai_object_id_t id, Port &port) { SWSS_LOG_ENTER(); - for (const auto& portIter: m_portList) - { - switch (portIter.second.m_type) - { - case Port::PHY: - if(portIter.second.m_port_id == id) - { - port = portIter.second; - return true; - } - break; - case Port::LAG: - if(portIter.second.m_lag_id == id) - { - port = portIter.second; - return true; - } - break; - case Port::VLAN: - if (portIter.second.m_vlan_info.vlan_oid == id) - { - port = portIter.second; - return true; - } - break; - default: - continue; - } + auto itr = m_portOidToName.find(id); + if (itr == m_portOidToName.end()) + return false; + else { + getPort(itr->second, port); + return true; } return false; @@ -595,13 +589,12 @@ bool PortsOrch::getPortByBridgePortId(sai_object_id_t bridge_port_id, Port &port { SWSS_LOG_ENTER(); - for (auto &it: m_portList) - { - if (it.second.m_bridge_port_id == bridge_port_id) - { - port = it.second; - return true; - } + auto itr = m_portOidToName.find(bridge_port_id); + if (itr == m_portOidToName.end()) + return false; + else { + getPort(itr->second, port); + return true; } return false; @@ -1735,6 +1728,21 @@ void PortsOrch::updateDbPortOperStatus(const Port& port, sai_port_oper_status_t m_portTable->set(port.m_alias, tuples); } +void PortsOrch::updateDbVlanOperStatus(const Port& vlan, string status) const +{ + SWSS_LOG_NOTICE("vlan %s status %s", vlan.m_alias.c_str(), status.c_str()); + + vector tuples; + FieldValueTuple tuple("oper_status", status); + tuples.push_back(tuple); + + std::vector entry; + + SWSS_LOG_NOTICE("sending oper state notification to VlanMgr"); + + notifications->send(status, vlan.m_alias, entry); +} + bool PortsOrch::addPort(const set &lane_set, uint32_t speed, int an, string fec_mode) { SWSS_LOG_ENTER(); @@ -1838,8 +1846,9 @@ bool PortsOrch::initPort(const string &alias, const int index, const set &l /* Add port to port list */ m_portList[alias] = p; - m_port_ref_count[alias] = 0; m_portOidToIndex[id] = index; + m_portOidToName[id] = alias; + m_port_ref_count[alias] = 0; /* Add port name map to counter table */ FieldValueTuple tuple(p.m_alias, sai_serialize_object_id(p.m_port_id)); @@ -2767,24 +2776,23 @@ void PortsOrch::doVlanMemberTask(Consumer &consumer) } else if (op == DEL_COMMAND) { + int ret = true; if (vlan.m_members.find(port_alias) != vlan.m_members.end()) { - if (removeVlanMember(vlan, port)) - { - if (port.m_vlan_members.empty()) - { - removeBridgePort(port); - } - it = consumer.m_toSync.erase(it); - } - else - { - it++; - } + ret = removeVlanMember(vlan, port); } - else - /* Cannot locate the VLAN */ + if ((ret) && port.m_vlan_members.empty()) + { + ret = removeBridgePort(port); + } + if (ret) + { it = consumer.m_toSync.erase(it); + } + else + { + it++; + } } else { @@ -2810,6 +2818,7 @@ void PortsOrch::doLagTask(Consumer &consumer) { // Retrieve attributes uint32_t mtu = 0; + string learn_mode; bool operation_status_changed = false; string operation_status; @@ -2846,15 +2855,7 @@ void PortsOrch::doLagTask(Consumer &consumer) } } - // Create a new LAG when the new alias comes - if (m_portList.find(alias) == m_portList.end()) - { - if (!addLag(alias)) - { - it++; - continue; - } - } + auto status = addLag(alias); // Process attributes Port l; @@ -2864,7 +2865,6 @@ void PortsOrch::doLagTask(Consumer &consumer) } else { - if (!operation_status.empty()) { l.m_oper_status = string_oper_status.at(operation_status); @@ -2877,7 +2877,7 @@ void PortsOrch::doLagTask(Consumer &consumer) update.operStatus = string_oper_status.at(operation_status); notify(SUBJECT_TYPE_PORT_OPER_STATE_CHANGE, static_cast(&update)); } - if (mtu != 0) + if ((mtu != 0) && (mtu != l.m_mtu)) { l.m_mtu = mtu; m_portList[alias] = l; @@ -2887,6 +2887,9 @@ void PortsOrch::doLagTask(Consumer &consumer) } } + sai_port_oper_status_t status = (operation_status == "up") ? SAI_PORT_OPER_STATUS_UP : SAI_PORT_OPER_STATUS_DOWN; + updateLagOperStatus(l, status); + if (!learn_mode.empty() && (l.m_learn_mode != learn_mode)) { if (l.m_bridge_port_id != SAI_NULL_OBJECT_ID) @@ -2913,8 +2916,15 @@ void PortsOrch::doLagTask(Consumer &consumer) } } } - - it = consumer.m_toSync.erase(it); + if (!status) + { + it++; + continue; + } + else + { + it = consumer.m_toSync.erase(it); + } } else if (op == DEL_COMMAND) { @@ -3336,8 +3346,44 @@ bool PortsOrch::addBridgePort(Port &port) { SWSS_LOG_ENTER(); + if (port.m_rif_id) + { + SWSS_LOG_ERROR("Adding router interface %s as bridge port is not allowed", port.m_alias.c_str()); + return false; + } if (port.m_bridge_port_id != SAI_NULL_OBJECT_ID) { + /* If the port is being added to the first VLAN, + * set bridge port admin status to UP. + * This can happen if the port was just removed from + * last VLAN and fdb flush is still in progress. + */ + if (port.m_vlan_members.empty()) + { + if (port.m_fdb_count > 0) + { + return false; + } + sai_attribute_t attr; + attr.id = SAI_BRIDGE_PORT_ATTR_ADMIN_STATE; + attr.value.booldata = true; + + sai_status_t status = sai_bridge_api->set_bridge_port_attribute(port.m_bridge_port_id, &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to set bridge port %s admin status to UP, rv:%d", + port.m_alias.c_str(), status); + return false; + } + port.m_bridge_port_admin_state = true; + m_portList[port.m_alias] = port; + if (!setHostIntfsStripTag(port, SAI_HOSTIF_VLAN_TAG_KEEP)) + { + SWSS_LOG_ERROR("Failed to set %s for hostif of port %s", + hostif_vlan_tag[SAI_HOSTIF_VLAN_TAG_KEEP], port.m_alias.c_str()); + return false; + } + } return true; } @@ -3390,6 +3436,13 @@ bool PortsOrch::addBridgePort(Port &port) port.m_alias.c_str(), status); return false; } + if (!setPortPvid(port, 0)) + { + SWSS_LOG_ERROR("Failed to set pvid for port %s, rv:%d", + port.m_alias.c_str(), status); + return false; + } + port.m_bridge_port_admin_state = true; if (!setHostIntfsStripTag(port, SAI_HOSTIF_VLAN_TAG_KEEP)) { @@ -3398,11 +3451,13 @@ bool PortsOrch::addBridgePort(Port &port) return false; } m_portList[port.m_alias] = port; + m_portOidToName[port.m_bridge_port_id] = port.m_alias; SWSS_LOG_NOTICE("Add bridge port %s to default 1Q bridge", port.m_alias.c_str()); return true; } + bool PortsOrch::removeBridgePort(Port &port) { SWSS_LOG_ENTER(); @@ -3411,34 +3466,45 @@ bool PortsOrch::removeBridgePort(Port &port) { return true; } - /* Set bridge port admin status to DOWN */ - sai_attribute_t attr; - attr.id = SAI_BRIDGE_PORT_ATTR_ADMIN_STATE; - attr.value.booldata = false; - - sai_status_t status = sai_bridge_api->set_bridge_port_attribute(port.m_bridge_port_id, &attr); - if (status != SAI_STATUS_SUCCESS) + if (port.m_bridge_port_admin_state) { - SWSS_LOG_ERROR("Failed to set bridge port %s admin status to DOWN, rv:%d", - port.m_alias.c_str(), status); - return false; + /* Set bridge port admin status to DOWN */ + sai_attribute_t attr; + attr.id = SAI_BRIDGE_PORT_ATTR_ADMIN_STATE; + attr.value.booldata = false; + + sai_status_t status = sai_bridge_api->set_bridge_port_attribute(port.m_bridge_port_id, &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to set bridge port %s admin status to DOWN, rv:%d", + port.m_alias.c_str(), status); + return false; + } + port.m_bridge_port_admin_state = false; + m_portList[port.m_alias] = port; + + if (!setHostIntfsStripTag(port, SAI_HOSTIF_VLAN_TAG_STRIP)) + { + SWSS_LOG_ERROR("Failed to set %s for hostif of port %s", + hostif_vlan_tag[SAI_HOSTIF_VLAN_TAG_STRIP], port.m_alias.c_str()); + return false; + } } - if (!setHostIntfsStripTag(port, SAI_HOSTIF_VLAN_TAG_STRIP)) - { - SWSS_LOG_ERROR("Failed to set %s for hostif of port %s", - hostif_vlan_tag[SAI_HOSTIF_VLAN_TAG_STRIP], port.m_alias.c_str()); + if (port.m_fdb_count != 0) { + //SWSS_LOG_NOTICE("Port still has fdb entries, will not remove bridge port for %s", port.m_alias.c_str()); return false; } /* Remove bridge port */ - status = sai_bridge_api->remove_bridge_port(port.m_bridge_port_id); + sai_status_t status = sai_bridge_api->remove_bridge_port(port.m_bridge_port_id); if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to remove bridge port %s from default 1Q bridge, rv:%d", port.m_alias.c_str(), status); return false; } + m_portOidToName.erase(port.m_bridge_port_id); port.m_bridge_port_id = SAI_NULL_OBJECT_ID; SWSS_LOG_NOTICE("Remove bridge port %s from default 1Q bridge", port.m_alias.c_str()); @@ -3491,7 +3557,17 @@ bool PortsOrch::addVlan(string vlan_alias) sai_attribute_t attr; attr.id = SAI_VLAN_ATTR_VLAN_ID; attr.value.u16 = vlan_id; - sai_status_t status = sai_vlan_api->create_vlan(&vlan_oid, gSwitchId, 1, &attr); + sai_status_t status = SAI_STATUS_SUCCESS; + + /* Do not create default VLAN. It is already created by default */ + if (vlan_id != m_defaultVlan_Id) + { + status = sai_vlan_api->create_vlan(&vlan_oid, gSwitchId, 1, &attr); + } + else + { + vlan_oid = m_defaultVlan_ObjId; /* use the default VLAN object id instead */ + } if (status != SAI_STATUS_SUCCESS) { @@ -3506,6 +3582,7 @@ bool PortsOrch::addVlan(string vlan_alias) vlan.m_vlan_info.vlan_id = vlan_id; vlan.m_members = set(); m_portList[vlan_alias] = vlan; + m_portOidToName[vlan_oid] = vlan_alias; m_port_ref_count[vlan_alias] = 0; return true; @@ -3514,6 +3591,7 @@ bool PortsOrch::addVlan(string vlan_alias) bool PortsOrch::removeVlan(Port vlan) { SWSS_LOG_ENTER(); + if (m_port_ref_count[vlan.m_alias] > 0) { SWSS_LOG_ERROR("Failed to remove ref count %d VLAN %s", @@ -3521,6 +3599,12 @@ bool PortsOrch::removeVlan(Port vlan) vlan.m_alias.c_str()); return false; } + /* If there are still fdb entries associated with the VLAN, + return false for retry */ + if (vlan.m_fdb_count > 0) + { + return false; + } /* Vlan removing is not allowed when the VLAN still has members */ if (vlan.m_members.size() > 0) @@ -3529,12 +3613,16 @@ bool PortsOrch::removeVlan(Port vlan) return false; } - sai_status_t status = sai_vlan_api->remove_vlan(vlan.m_vlan_info.vlan_oid); - if (status != SAI_STATUS_SUCCESS) + /* Do not delete default VLAN from driver, but clear internal state */ + if (vlan.m_vlan_info.vlan_id != m_defaultVlan_Id) { + sai_status_t status = sai_vlan_api->remove_vlan(vlan.m_vlan_info.vlan_oid); + if (status != SAI_STATUS_SUCCESS) + { SWSS_LOG_ERROR("Failed to remove VLAN %s vid:%hu", vlan.m_alias.c_str(), vlan.m_vlan_info.vlan_id); return false; + } } removeAclTableGroup(vlan); @@ -3542,6 +3630,7 @@ bool PortsOrch::removeVlan(Port vlan) SWSS_LOG_NOTICE("Remove VLAN %s vid:%hu", vlan.m_alias.c_str(), vlan.m_vlan_info.vlan_id); + m_portOidToName.erase(vlan.m_vlan_info.vlan_oid); m_portList.erase(vlan.m_alias); m_port_ref_count.erase(vlan.m_alias); @@ -3612,10 +3701,19 @@ bool PortsOrch::addVlanMember(Port &vlan, Port &port, string &tagging_mode) } /* a physical port may join multiple vlans */ - VlanMemberEntry vme = {vlan_member_id, sai_tagging_mode}; + VlanMemberEntry vme = {vlan.m_alias, vlan_member_id, sai_tagging_mode}; port.m_vlan_members[vlan.m_vlan_info.vlan_id] = vme; m_portList[port.m_alias] = port; vlan.m_members.insert(port.m_alias); + if (port.m_oper_status == SAI_PORT_OPER_STATUS_UP) + { + auto old_count = vlan.m_up_member_count; + vlan.m_up_member_count++; + if (old_count == 0) + { + updateDbVlanOperStatus(vlan, "up"); + } + } m_portList[vlan.m_alias] = vlan; VlanMemberUpdate update = { vlan, port, true }; @@ -3651,7 +3749,7 @@ bool PortsOrch::removeVlanMember(Port &vlan, Port &port) /* Restore to default pvid if this port joined this VLAN in untagged mode previously */ if (sai_tagging_mode == SAI_VLAN_TAGGING_MODE_UNTAGGED) { - if (!setPortPvid(port, DEFAULT_PORT_VLAN_ID)) + if (!setPortPvid(port, 0)) { return false; } @@ -3659,6 +3757,14 @@ bool PortsOrch::removeVlanMember(Port &vlan, Port &port) m_portList[port.m_alias] = port; vlan.m_members.erase(port.m_alias); + if (port.m_oper_status == SAI_PORT_OPER_STATUS_UP) + { + vlan.m_up_member_count--; + if (vlan.m_up_member_count == 0) + { + updateDbVlanOperStatus(vlan, "down"); + } + } m_portList[vlan.m_alias] = vlan; VlanMemberUpdate update = { vlan, port, false }; @@ -3671,6 +3777,17 @@ bool PortsOrch::addLag(string lag_alias) { SWSS_LOG_ENTER(); + auto lagport = m_portList.find(lag_alias); + if (lagport != m_portList.end()) + { + if ((m_portList[lag_alias].m_bridge_port_id != SAI_NULL_OBJECT_ID) && + (m_portList[lag_alias].m_vlan_members.empty())) + { + return false; + } + return true; + } + sai_object_id_t lag_id; sai_status_t status = sai_lag_api->create_lag(&lag_id, gSwitchId, 0, NULL); @@ -3686,16 +3803,18 @@ bool PortsOrch::addLag(string lag_alias) lag.m_lag_id = lag_id; lag.m_members = set(); m_portList[lag_alias] = lag; + m_portOidToName[lag_id] = lag_alias; m_port_ref_count[lag_alias] = 0; - PortUpdate update = { lag, true }; - notify(SUBJECT_TYPE_PORT_CHANGE, static_cast(&update)); - + /* Add lag name map to counter table */ FieldValueTuple tuple(lag_alias, sai_serialize_object_id(lag_id)); vector fields; fields.push_back(tuple); m_counterLagTable->set("", fields); + PortUpdate update = { lag, true }; + notify(SUBJECT_TYPE_PORT_CHANGE, static_cast(&update)); + return true; } @@ -3722,6 +3841,10 @@ bool PortsOrch::removeLag(Port lag) SWSS_LOG_ERROR("Failed to remove LAG %s, it is still in VLAN", lag.m_alias.c_str()); return false; } + if (lag.m_bridge_port_id != SAI_NULL_OBJECT_ID) + { + return false; + } sai_status_t status = sai_lag_api->remove_lag(lag.m_lag_id); if (status != SAI_STATUS_SUCCESS) @@ -3730,8 +3853,10 @@ bool PortsOrch::removeLag(Port lag) return false; } + SWSS_LOG_NOTICE("Remove LAG %s lid:%" PRIx64, lag.m_alias.c_str(), lag.m_lag_id); + m_portOidToName.erase(lag.m_lag_id); m_portList.erase(lag.m_alias); m_port_ref_count.erase(lag.m_alias); @@ -4102,6 +4227,25 @@ void PortsOrch::updatePortOperStatus(Port &port, sai_port_oper_status_t status) port.m_oper_status = status; bool isUp = status == SAI_PORT_OPER_STATUS_UP; + + for(auto vlan_member: port.m_vlan_members) + { + auto Vlan = m_portList[vlan_member.second.alias]; + auto old_count = Vlan.m_up_member_count; + isUp ? Vlan.m_up_member_count++ : Vlan.m_up_member_count--; + if (Vlan.m_up_member_count == 0) + { + updateDbVlanOperStatus(Vlan, "down"); + } + else if ((old_count == 0) && (Vlan.m_up_member_count == 1)) + { + updateDbVlanOperStatus(Vlan, "up"); + } + SWSS_LOG_NOTICE("Vlan %s Port %s state %s m_up_member_count %d", + vlan_member.second.alias.c_str(), port.m_alias.c_str(), oper_status_strings.at(port.m_oper_status).c_str(), Vlan.m_up_member_count); + + m_portList[Vlan.m_alias] = Vlan; + } if (!setHostIntfsOperStatus(port, isUp)) { SWSS_LOG_ERROR("Failed to set host interface %s operational status %s", port.m_alias.c_str(), @@ -4116,6 +4260,41 @@ void PortsOrch::updatePortOperStatus(Port &port, sai_port_oper_status_t status) notify(SUBJECT_TYPE_PORT_OPER_STATE_CHANGE, static_cast(&update)); } +void PortsOrch::updateLagOperStatus(Port &port, sai_port_oper_status_t status) +{ + if (status == port.m_oper_status) + { + return ; + } + SWSS_LOG_NOTICE("Port %s oper state set from %s to %s", + port.m_alias.c_str(), oper_status_strings.at(port.m_oper_status).c_str(), + oper_status_strings.at(status).c_str()); + + port.m_oper_status = status; + m_portList[port.m_alias] = port; + + bool isUp = status == SAI_PORT_OPER_STATUS_UP; + + for(auto vlan_member: port.m_vlan_members) + { + auto Vlan = m_portList[vlan_member.second.alias]; + auto old_count = Vlan.m_up_member_count; + isUp ? Vlan.m_up_member_count++ : Vlan.m_up_member_count--; + if (Vlan.m_up_member_count == 0) + { + updateDbVlanOperStatus(Vlan, "down"); + } + else if ((old_count == 0) && (Vlan.m_up_member_count == 1)) + { + updateDbVlanOperStatus(Vlan, "up"); + } + SWSS_LOG_NOTICE("Vlan %s Port %s state %s m_up_member_count %d", + vlan_member.second.alias.c_str(), port.m_alias.c_str(), oper_status_strings.at(port.m_oper_status).c_str(), Vlan.m_up_member_count); + + m_portList[Vlan.m_alias] = Vlan; + } +} + /* * sync up orchagent with libsai/ASIC for port state. * diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 936f1ff07e..d89899cdd5 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -9,6 +9,7 @@ #include "observer.h" #include "macaddress.h" #include "producertable.h" +#include "notificationproducer.h" #include "flex_counter_manager.h" #include "gearboxutils.h" #include "saihelper.h" @@ -91,12 +92,15 @@ class PortsOrch : public Orch, public Subject void decreasePortRefCount(const string &alias); bool getPortByBridgePortId(sai_object_id_t bridge_port_id, Port &port); void setPort(string alias, Port port); + void erasePort(string alias); void getCpuPort(Port &port); bool getVlanByVlanId(sai_vlan_id_t vlan_id, Port &vlan); bool getAclBindPortId(string alias, sai_object_id_t &port_id); bool setHostIntfsOperStatus(const Port& port, bool up) const; void updateDbPortOperStatus(const Port& port, sai_port_oper_status_t status) const; + void updateDbVlanOperStatus(const Port& port, string status) const; + bool createBindAclTableGroup(sai_object_id_t port_oid, sai_object_id_t acl_table_oid, sai_object_id_t &group_oid, @@ -132,6 +136,7 @@ class PortsOrch : public Orch, public Subject unique_ptr
m_counterLagTable; unique_ptr
m_portTable; unique_ptr
m_gearboxTable; + unique_ptr
m_vlanTable; unique_ptr
m_queueTable; unique_ptr
m_queuePortTable; unique_ptr
m_queueIndexTable; @@ -141,6 +146,7 @@ class PortsOrch : public Orch, public Subject unique_ptr
m_pgIndexTable; unique_ptr m_flexCounterTable; unique_ptr m_flexCounterGroupTable; + NotificationProducer* notifications; std::string getQueueWatermarkFlexCounterTableKey(std::string s); std::string getPriorityGroupWatermarkFlexCounterTableKey(std::string s); @@ -159,7 +165,8 @@ class PortsOrch : public Orch, public Subject Port m_cpuPort; // TODO: Add Bridge/Vlan class sai_object_id_t m_default1QBridge; - sai_object_id_t m_defaultVlan; + sai_object_id_t m_defaultVlan_ObjId; + sai_vlan_id_t m_defaultVlan_Id; typedef enum { @@ -188,6 +195,14 @@ class PortsOrch : public Orch, public Subject map, tuple> m_lanesAliasSpeedMap; map m_portList; unordered_map m_portOidToIndex; + + /* mapping from SAI object ID to Name for faster + retrieval of Port/VLAN from object ID for events + + coming from SAI + */ + unordered_map m_portOidToName; + map m_port_ref_count; unordered_set m_pendingPortSet; @@ -273,6 +288,7 @@ class PortsOrch : public Orch, public Subject bool setPortSerdesAttribute(sai_object_id_t port_id, sai_attr_id_t attr_id, vector &serdes_val); + void updateLagOperStatus(Port &port, sai_port_oper_status_t status); bool getSaiAclBindPointType(Port::Type type, sai_acl_bind_point_type_t &sai_acl_bind_type); void initGearbox(); diff --git a/tests/conftest.py b/tests/conftest.py index 8793c086e3..8ea0bef694 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1019,6 +1019,24 @@ def setup_db(self): self.cdb = swsscommon.DBConnector(4, self.redis_sock, 0) self.sdb = swsscommon.DBConnector(6, self.redis_sock, 0) + def getSwitchOid(self): + tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_SWITCH") + keys = tbl.getKeys() + return str(keys[0]) + + def getVlanOid(self, vlanId): + tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_VLAN") + vlan_oid = None + keys = tbl.getKeys() + for k in keys: + (status, fvs) = tbl.get(k) + assert status == True, "Could not read vlan from DB" + for fv in fvs: + if fv[0] == "SAI_VLAN_ATTR_VLAN_ID" and fv[1] == str(vlanId): + vlan_oid = str(k) + break + return vlan_oid + def getCrmCounterValue(self, key, counter): counters_db = swsscommon.DBConnector(swsscommon.COUNTERS_DB, self.redis_sock, 0) crm_stats_table = swsscommon.Table(counters_db, 'CRM') diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index cc0ca8941f..2058018aff 100644 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -318,11 +318,15 @@ namespace aclorch_test ASSERT_EQ(gRouteOrch, nullptr); gRouteOrch = new RouteOrch(m_app_db.get(), APP_ROUTE_TABLE_NAME, gSwitchOrch, gNeighOrch, gIntfsOrch, gVrfOrch); - TableConnector applDbFdb(m_app_db.get(), APP_FDB_TABLE_NAME); + vector app_fdb_tables = { + { APP_FDB_TABLE_NAME, FdbOrch::fdborch_pri}, + { APP_VXLAN_FDB_TABLE_NAME, FdbOrch::fdborch_pri} + }; + TableConnector stateDbFdb(m_state_db.get(), STATE_FDB_TABLE_NAME); ASSERT_EQ(gFdbOrch, nullptr); - gFdbOrch = new FdbOrch(applDbFdb, stateDbFdb, gPortsOrch); + gFdbOrch = new FdbOrch(m_app_db.get(), app_fdb_tables, stateDbFdb, gPortsOrch); PolicerOrch *policer_orch = new PolicerOrch(m_config_db.get(), "POLICER"); diff --git a/tests/test_evpn_fdb.py b/tests/test_evpn_fdb.py index 469494a279..0091b19b5b 100644 --- a/tests/test_evpn_fdb.py +++ b/tests/test_evpn_fdb.py @@ -48,13 +48,13 @@ def create_evpn_nvo(db, nvoname, tnlname): # create the VXLAN tunnel Term entry in Config DB create_entry_tbl( db, - "EVPN_NVO", nvoname, + "VXLAN_EVPN_NVO", nvoname, attrs, ) def remove_evpn_nvo(db, nvoname): #conf_db = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0) - delete_entry_tbl(db,"EVPN_NVO", nvoname,) + delete_entry_tbl(db,"VXLAN_EVPN_NVO", nvoname,) def create_vxlan_tunnel(db, name, src_ip): #conf_db = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0) @@ -112,7 +112,7 @@ def create_evpn_remote_vni(db, vlan_id, remotevtep, vnid): #app_db = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0) create_entry_pst( db, - "EVPN_REMOTE_VNI_TABLE", "%s:%s" % (vlan_id, remotevtep), + "VXLAN_REMOTE_VNI_TABLE", "%s:%s" % (vlan_id, remotevtep), [ ("vni", vnid), ], @@ -122,31 +122,29 @@ def remove_evpn_remote_vni(db, vlan_id, remotevtep ): #app_db = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0) delete_entry_pst( db, - "EVPN_REMOTE_VNI_TABLE", "%s:%s" % (vlan_id, remotevtep), + "VXLAN_REMOTE_VNI_TABLE", "%s:%s" % (vlan_id, remotevtep), ) def get_vxlan_p2p_tunnel_bp(db, remote_ip): tnl_id = None bp = None print("remote_ip = " + remote_ip) - attributes = [("SAI_TUNNEL_TERM_TABLE_ENTRY_ATTR_TYPE", "SAI_TUNNEL_TERM_TABLE_ENTRY_TYPE_P2P"), - ("SAI_TUNNEL_TERM_TABLE_ENTRY_ATTR_TUNNEL_TYPE", "SAI_TUNNEL_TYPE_VXLAN"), - ("SAI_TUNNEL_TERM_TABLE_ENTRY_ATTR_SRC_IP", remote_ip) + attributes = [("SAI_TUNNEL_ATTR_TYPE", "SAI_TUNNEL_TYPE_VXLAN"), + ("SAI_TUNNEL_ATTR_ENCAP_DST_IP", remote_ip) ] - tbl = swsscommon.Table(db, "ASIC_STATE:SAI_OBJECT_TYPE_TUNNEL_TERM_TABLE_ENTRY") + tbl = swsscommon.Table(db, "ASIC_STATE:SAI_OBJECT_TYPE_TUNNEL") keys = tbl.getKeys() for key in keys: status, fvs = tbl.get(key) - assert status, "Error reading from table ASIC_STATE:SAI_OBJECT_TYPE_TUNNEL_TERM_TABLE_ENTRY" + assert status, "Error reading from table ASIC_STATE:SAI_OBJECT_TYPE_TUNNEL" attrs = dict(attributes) num_match = 0 for k, v in fvs: print("attr:value="+str(k)+":"+str(v)) if k in attrs and attrs[k] == v: num_match += 1 - if k == "SAI_TUNNEL_TERM_TABLE_ENTRY_ATTR_ACTION_TUNNEL_ID": - tnl_id = v if num_match == len(attributes): + tnl_id = str(key) break else: tnl_id = None @@ -223,14 +221,17 @@ def test_evpnFdb(dvs, testlog): create_vxlan_tunnel(dvs.cdb, source_tnl_name, source_tnl_ip) time.sleep(1) + nvo_name = "evpn_nvo" create_evpn_nvo(dvs.cdb, nvo_name, source_tnl_name) time.sleep(1) + map_name_vlan_3 = "map_3_3" create_vxlan_tunnel_map(dvs.cdb, source_tnl_name, map_name_vlan_3, "3", "Vlan3") time.sleep(1) + remote_ip_6 = "6.6.6.6" create_evpn_remote_vni(dvs.pdb, "Vlan3", remote_ip_6, "3") remote_ip_8 = "8.8.8.8" @@ -254,10 +255,13 @@ def test_evpnFdb(dvs, testlog): tnl_bp_oid_6 = get_vxlan_p2p_tunnel_bp(dvs.adb, remote_ip_6) tnl_bp_oid_8 = get_vxlan_p2p_tunnel_bp(dvs.adb, remote_ip_8) + + # check that the FDB entry is inserted into ASIC DB ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", [("mac", mac), ("bvid", vlan_oid_3)], - [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC_MACMOVE"), + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), + ("SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE", "true"), ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_6), ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_6)), ] @@ -279,7 +283,8 @@ def test_evpnFdb(dvs, testlog): # check that the FDB entry is deleted from ASIC DB ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", [("mac", mac), ("bvid", vlan_oid_3)], - [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC_MACMOVE"), + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), + ("SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE", "true"), ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_6), ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_6)), ] @@ -337,12 +342,12 @@ def test_evpnFdb(dvs, testlog): ) time.sleep(1) - #raw_input("Check ASIC_DB.........") # check that the FDB entry is inserted into ASIC DB ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", [("mac", mac), ("bvid", str(dvs.getVlanOid("3")))], - [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC_MACMOVE"), + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), + ("SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE", "true"), ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_6), ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_6)), ] @@ -362,6 +367,7 @@ def test_evpnFdb(dvs, testlog): time.sleep(1) + #UT-4 Evpn Sticky Mac add from remote mac = "52:54:00:25:06:E9" print("Creating Evpn Sticky FDB Vlan3:"+mac.lower()+":6.6.6.6 in APP-DB") @@ -376,7 +382,6 @@ def test_evpnFdb(dvs, testlog): ) time.sleep(1) - #raw_input("Check ASIC_DB.........") # check that the FDB entry is inserted into ASIC DB ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", @@ -399,6 +404,7 @@ def test_evpnFdb(dvs, testlog): "FDB", "Vlan3|"+mac.lower(), [ ("port", "Ethernet0"), + ("type", "static"), ] ) time.sleep(2) @@ -488,7 +494,8 @@ def test_evpnFdb(dvs, testlog): # check that the FDB entry is not inserted into ASIC DB ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", [("mac", mac), ("bvid", vlan_oid_3)], - [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC_MACMOVE"), + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), + ("SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE", "true"), ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_6), ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_6)), ] @@ -584,7 +591,8 @@ def test_evpnFdb(dvs, testlog): # check that the FDB entry is inserted into ASIC DB ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", [("mac", mac), ("bvid", vlan_oid_3)], - [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC_MACMOVE"), + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), + ("SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE", "true"), ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_6), ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_6)), ] @@ -613,7 +621,8 @@ def test_evpnFdb(dvs, testlog): # check that the FDB entry is inserted into ASIC DB ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", [("mac", mac), ("bvid", vlan_oid_3)], - [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC_MACMOVE"), + [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), + ("SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE", "true"), ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_8), ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_8)), ] @@ -628,7 +637,7 @@ def test_evpnFdb(dvs, testlog): print("Deleting FDB Vlan3:52-54-00-25-06-E9:8.8.8.8 in ASIC-DB") delete_entry_tbl(dvs.adb, "ASIC_STATE", "SAI_OBJECT_TYPE_FDB_ENTRY:{\"bvid\":\""+vlan_oid_3+"\",\"mac\":\""+mac+"\",\"switch_id\":\""+switch_id+"\"}") - ntf = swsscommon.NotificationProducer(dvs.adb, "FDB_NOTIFICATIONS") + ntf = swsscommon.NotificationProducer(dvs.adb, "NOTIFICATIONS") fvp = swsscommon.FieldValuePairs() ntf_data = "[{\"fdb_entry\":\"{\\\"bvid\\\":\\\""+vlan_oid_3+"\\\",\\\"mac\\\":\\\""+mac+"\\\",\\\"switch_id\\\":\\\""+switch_id+"\\\"}\",\"fdb_event\":\"SAI_FDB_EVENT_AGED\",\"list\":[{\"id\":\"SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID\",\"value\":\""+str(tnl_bp_oid_8)+"\"}]}]" ntf.send("fdb_event", ntf_data, fvp) @@ -647,7 +656,7 @@ def test_evpnFdb(dvs, testlog): ] ) - ntf = swsscommon.NotificationProducer(dvs.adb, "FDB_NOTIFICATIONS") + ntf = swsscommon.NotificationProducer(dvs.adb, "NOTIFICATIONS") fvp = swsscommon.FieldValuePairs() ntf_data = "[{\"fdb_entry\":\"{\\\"bvid\\\":\\\""+vlan_oid_3+"\\\",\\\"mac\\\":\\\"52:54:00:25:06:E9\\\",\\\"switch_id\\\":\\\""+switch_id+"\\\"}\",\"fdb_event\":\"SAI_FDB_EVENT_LEARNED\",\"list\":[{\"id\":\"SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID\",\"value\":\""+iface_2_bridge_port_id["Ethernet0"]+"\"}]}]" ntf.send("fdb_event", ntf_data, fvp) diff --git a/tests/test_fdb.py b/tests/test_fdb.py index 1f46727f0d..9893a4e3b0 100644 --- a/tests/test_fdb.py +++ b/tests/test_fdb.py @@ -374,9 +374,7 @@ def test_FdbAddedAfterMemberCreated(self, dvs, testlog): ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", [("mac", "52:54:00:25:06:E9"), ("bvid", bvid)], [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_DYNAMIC"), - ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", iface_2_bridge_port_id["Ethernet0"]), - ('SAI_FDB_ENTRY_ATTR_PACKET_ACTION', 'SAI_PACKET_ACTION_FORWARD')] - ) + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", iface_2_bridge_port_id["Ethernet0"])]) assert ok, str(extra) dvs.runcmd("sonic-clear fdb all") diff --git a/tests/test_fdb_update.py b/tests/test_fdb_update.py index 2c8e9b0c8e..5daf27804e 100644 --- a/tests/test_fdb_update.py +++ b/tests/test_fdb_update.py @@ -117,8 +117,7 @@ def test_FDBAddedAndUpdated(self, dvs, testlog): ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", [("mac", "52:54:00:25:06:E9"), ("bvid", bvid)], [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_DYNAMIC"), - ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", iface_2_bridge_port_id["Ethernet0"]), - ('SAI_FDB_ENTRY_ATTR_PACKET_ACTION', 'SAI_PACKET_ACTION_FORWARD')]) + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", iface_2_bridge_port_id["Ethernet0"])]) assert ok, str(extra) # create vlan member entry in Config DB @@ -146,8 +145,7 @@ def test_FDBAddedAndUpdated(self, dvs, testlog): ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", [("mac", "52:54:00:25:06:E9"), ("bvid", bvid)], [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_DYNAMIC"), - ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", iface_2_bridge_port_id["Ethernet4"]), - ('SAI_FDB_ENTRY_ATTR_PACKET_ACTION', 'SAI_PACKET_ACTION_FORWARD')]) + ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", iface_2_bridge_port_id["Ethernet4"])]) assert ok, str(extra) # remove FDB entry from Application DB From 4d9bd0a3ce5c0ceeeff5df61c708a739d5ab521a Mon Sep 17 00:00:00 2001 From: Pankaj Jain Date: Wed, 24 Jun 2020 00:18:06 -0700 Subject: [PATCH 03/21] Fixed LGTM Warnings --- tests/test_evpn_fdb.py | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/tests/test_evpn_fdb.py b/tests/test_evpn_fdb.py index 0091b19b5b..c5974ba055 100644 --- a/tests/test_evpn_fdb.py +++ b/tests/test_evpn_fdb.py @@ -1,10 +1,5 @@ from swsscommon import swsscommon -import os -import sys import time -import json -import pytest -from distutils.version import StrictVersion def create_entry(tbl, key, pairs): fvs = swsscommon.FieldValuePairs(pairs) @@ -94,14 +89,8 @@ def create_vxlan_tunnel_map(db, tnlname, mapname, vni_id, vlan_id): attrs, ) -def remove_vxlan_tunnel_map(db, tnlname, mapname,vni_id, vlan_id): +def remove_vxlan_tunnel_map(db, tnlname, mapname): #conf_db = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0) - - attrs = [ - ("vni", vni_id), - ("vlan", vlan_id), - ] - # create the VXLAN tunnel Term entry in Config DB delete_entry_tbl( db, @@ -253,9 +242,6 @@ def test_evpnFdb(dvs, testlog): time.sleep(1) tnl_bp_oid_6 = get_vxlan_p2p_tunnel_bp(dvs.adb, remote_ip_6) - tnl_bp_oid_8 = get_vxlan_p2p_tunnel_bp(dvs.adb, remote_ip_8) - - # check that the FDB entry is inserted into ASIC DB ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", From 187388ec644118d7754f645836343a3fb48e25c8 Mon Sep 17 00:00:00 2001 From: Pankaj Jain Date: Wed, 28 Oct 2020 11:29:57 -0700 Subject: [PATCH 04/21] [Orchagent]: FdbOrch changes for EVPN VXLAN Review comments incorporated. --- orchagent/fdborch.cpp | 157 +++++++++++++++++++++--------------------- orchagent/fdborch.h | 5 +- 2 files changed, 82 insertions(+), 80 deletions(-) diff --git a/orchagent/fdborch.cpp b/orchagent/fdborch.cpp index 4321d1965b..f69f5fd1e3 100644 --- a/orchagent/fdborch.cpp +++ b/orchagent/fdborch.cpp @@ -188,12 +188,6 @@ void FdbOrch::update(sai_fdb_event_t type, return; } - if (!m_portsOrch->getPortByBridgePortId(bridge_port_id, update.port)) - { - SWSS_LOG_ERROR("FdbOrch LEARN notification: Failed to get port by bridge port ID 0x%" PRIx64, bridge_port_id); - return; - } - // we already have such entries auto existing_entry = m_entries.find(update.entry); if (existing_entry != m_entries.end()) @@ -234,11 +228,6 @@ void FdbOrch::update(sai_fdb_event_t type, break; } - if (!m_portsOrch->getPortByBridgePortId(bridge_port_id, update.port)) - { - SWSS_LOG_NOTICE("FdbOrch AGE notification: Failed to get port by bridge port ID 0x%lx", bridge_port_id); - } - if (existing_entry->second.bridge_port_id != bridge_port_id) { SWSS_LOG_NOTICE("FdbOrch AGE notification: Stale aging event received for mac-bv_id %s-0x%lx with bp=0x%lx existing bp=0x%lx", update.entry.mac.to_string().c_str(), entry->bv_id, bridge_port_id, existing_entry->second.bridge_port_id); @@ -257,12 +246,15 @@ void FdbOrch::update(sai_fdb_event_t type, if (vlan.m_members.find(update.port.m_alias) == vlan.m_members.end()) { - saved_fdb_entries[update.port.m_alias].push_back({existing_entry->first.mac, - vlan.m_vlan_info.vlan_id, update.type, - existing_entry->second.origin, - existing_entry->second.remote_ip, - existing_entry->second.esi, - existing_entry->second.vni}); + FdbData fdbData; + fdbData.bridge_port_id = SAI_NULL_OBJECT_ID; + fdbData.type = update.type; + fdbData.origin = existing_entry->second.origin; + fdbData.remote_ip = existing_entry->second.remote_ip; + fdbData.esi = existing_entry->second.esi; + fdbData.vni = existing_entry->second.vni; + saved_fdb_entries[update.port.m_alias].push_back( + {existing_entry->first.mac, vlan.m_vlan_info.vlan_id, fdbData}); } else { @@ -321,12 +313,6 @@ void FdbOrch::update(sai_fdb_event_t type, return; } - if (!m_portsOrch->getPortByBridgePortId(bridge_port_id, update.port)) - { - SWSS_LOG_ERROR("FdbOrch MOVE notification: Failed to get port by bridge port ID 0x%lx", bridge_port_id); - return; - } - // We should already have such entry if (existing_entry == m_entries.end()) { @@ -605,7 +591,14 @@ void FdbOrch::doTask(Consumer& consumer) } - if (addFdbEntry(entry, port, type, origin, remote_ip, vni, esi)) + FdbData fdbData; + fdbData.bridge_port_id = SAI_NULL_OBJECT_ID; + fdbData.type = type; + fdbData.origin = origin; + fdbData.remote_ip = remote_ip; + fdbData.esi = esi; + fdbData.vni = vni; + if (addFdbEntry(entry, port, fdbData)) it = consumer.m_toSync.erase(it); else it++; @@ -738,8 +731,7 @@ void FdbOrch::updateVlanMember(const VlanMemberUpdate& update) FdbEntry entry; entry.mac = fdb.mac; entry.bv_id = update.vlan.m_vlan_info.vlan_oid; - (void)addFdbEntry(entry, port_name, fdb.type, fdb.origin, - fdb.remote_ip, fdb.vni, fdb.esi); + (void)addFdbEntry(entry, port_name, fdb.fdbData); } else { @@ -749,13 +741,16 @@ void FdbOrch::updateVlanMember(const VlanMemberUpdate& update) } } -bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const string& type, FdbOrigin origin, const string& remote_ip, unsigned int vni, const string& esi) +bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, + FdbData fdbData) { Port vlan; Port port; SWSS_LOG_ENTER(); - SWSS_LOG_NOTICE("mac=%s bv_id=0x%lx port_name=%s type=%s origin=%d", entry.mac.to_string().c_str(), entry.bv_id, port_name.c_str(), type.c_str(), origin); + SWSS_LOG_NOTICE("mac=%s bv_id=0x%lx port_name=%s type=%s origin=%d", + entry.mac.to_string().c_str(), entry.bv_id, port_name.c_str(), + fdbData.type.c_str(), fdbData.origin); if (!m_portsOrch->getPort(entry.bv_id, vlan)) { @@ -768,7 +763,7 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const { SWSS_LOG_INFO("Saving a fdb entry until port %s becomes active", port_name.c_str()); saved_fdb_entries[port_name].push_back({entry.mac, - vlan.m_vlan_info.vlan_id, type, origin, remote_ip, esi, vni}); + vlan.m_vlan_info.vlan_id, fdbData}); return true; } @@ -777,7 +772,7 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const { 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, - vlan.m_vlan_info.vlan_id, type, origin, remote_ip, esi, vni}); + vlan.m_vlan_info.vlan_id, fdbData}); return true; } @@ -804,13 +799,15 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const return false; } - if((oldOrigin == origin) && (oldType == 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)) { /* Duplicate Mac */ - SWSS_LOG_NOTICE("FdbOrch: mac=%s %s port=%s type=%s origin=%d is duplicate", entry.mac.to_string().c_str(), vlan.m_alias.c_str(), port_name.c_str(), type.c_str(), origin); + SWSS_LOG_NOTICE("FdbOrch: mac=%s %s port=%s type=%s origin=%d is duplicate", entry.mac.to_string().c_str(), + vlan.m_alias.c_str(), port_name.c_str(), + fdbData.type.c_str(), fdbData.origin); return true; } - else if(origin != oldOrigin) + else if(fdbData.origin != oldOrigin) { /* Mac origin has changed */ if((oldType == "static") && (oldOrigin == FDB_ORIGIN_PROVISIONED)) @@ -820,11 +817,12 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const "Received same MAC from peer:%s; " "Peer mac ignored", entry.mac.to_string().c_str(), vlan.m_vlan_info.vlan_id, - remote_ip.c_str()); + fdbData.remote_ip.c_str()); return true; } - else if((oldType == "static") && (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED) && (type == "dynamic")) + else if((oldType == "static") && (oldOrigin == + FDB_ORIGIN_VXLAN_ADVERTIZED) && (fdbData.type == "dynamic")) { /* old mac was static and received from remote, it can not be changed by dynamic locally provisioned Mac */ SWSS_LOG_NOTICE("Already existing static MAC:%s in Vlan:%d " @@ -836,7 +834,7 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const } else if(oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED) { - if((oldType == "static") && (type == "static")) + if((oldType == "static") && (fdbData.type == "static")) { SWSS_LOG_WARN("You have just overwritten existing static MAC:%s " "in Vlan:%d from Peer:%s, " @@ -847,7 +845,7 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const } } } - else /* (origin == oldOrigin) */ + else /* (fdbData.origin == oldOrigin) */ { /* Mac origin is same, all changes are allowed */ /* Allowed @@ -864,17 +862,17 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const vector attrs; attr.id = SAI_FDB_ENTRY_ATTR_TYPE; - if (origin == FDB_ORIGIN_VXLAN_ADVERTIZED) + if (fdbData.origin == FDB_ORIGIN_VXLAN_ADVERTIZED) { attr.value.s32 = SAI_FDB_ENTRY_TYPE_STATIC; } else { - attr.value.s32 = (type == "dynamic") ? SAI_FDB_ENTRY_TYPE_DYNAMIC : SAI_FDB_ENTRY_TYPE_STATIC; + attr.value.s32 = (fdbData.type == "dynamic") ? SAI_FDB_ENTRY_TYPE_DYNAMIC : SAI_FDB_ENTRY_TYPE_STATIC; } attrs.push_back(attr); - if ((origin == FDB_ORIGIN_VXLAN_ADVERTIZED) && (type == "dynamic")) + if ((fdbData.origin == FDB_ORIGIN_VXLAN_ADVERTIZED) && (fdbData.type == "dynamic")) { attr.id = SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE; attr.value.booldata = true; @@ -885,9 +883,9 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const attr.value.oid = port.m_bridge_port_id; attrs.push_back(attr); - if(origin == FDB_ORIGIN_VXLAN_ADVERTIZED) + if(fdbData.origin == FDB_ORIGIN_VXLAN_ADVERTIZED) { - IpAddress remote = IpAddress(remote_ip); + IpAddress remote = IpAddress(fdbData.remote_ip); sai_ip_address_t ipaddr; if(remote.isV4()) { @@ -905,7 +903,7 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const } else if(macUpdate && (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED) - && (origin != oldOrigin)) + && (fdbData.origin != oldOrigin)) { /* origin is changed from Remote-advertized to Local-provisioned * Remove the end-point ip attribute from fdb entry @@ -920,8 +918,8 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const if(macUpdate && (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED)) { - if((origin != oldOrigin) - || ((oldType == "dynamic") && (oldType != type))) + if((fdbData.origin != oldOrigin) + || ((oldType == "dynamic") && (oldType != fdbData.type))) { attr.id = SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE; attr.value.booldata = false; @@ -934,7 +932,8 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const { SWSS_LOG_NOTICE("MAC-Update FDB %s in %s on from-%s:to-%s from-%s:to-%s origin-%d-to-%d", entry.mac.to_string().c_str(), vlan.m_alias.c_str(), oldPort.m_alias.c_str(), - port_name.c_str(), oldType.c_str(), type.c_str(), oldOrigin, origin); + port_name.c_str(), oldType.c_str(), fdbData.type.c_str(), + oldOrigin, fdbData.origin); for(auto itr : attrs) { status = sai_fdb_api->set_fdb_entry_attribute(&fdb_entry, &itr); @@ -955,13 +954,13 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const } else { - SWSS_LOG_NOTICE("MAC-Create %s FDB %s in %s on %s", type.c_str(), entry.mac.to_string().c_str(), vlan.m_alias.c_str(), port_name.c_str()); + SWSS_LOG_NOTICE("MAC-Create %s FDB %s in %s on %s", fdbData.type.c_str(), entry.mac.to_string().c_str(), vlan.m_alias.c_str(), port_name.c_str()); status = sai_fdb_api->create_fdb_entry(&fdb_entry, (uint32_t)attrs.size(), attrs.data()); if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to create %s FDB %s in %s on %s, rv:%d", - type.c_str(), entry.mac.to_string().c_str(), + fdbData.type.c_str(), entry.mac.to_string().c_str(), vlan.m_alias.c_str(), port_name.c_str(), status); return false; //FIXME: it should be based on status. Some could be retried, some not } @@ -971,28 +970,23 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const m_portsOrch->setPort(vlan.m_alias, vlan); } - FdbData fdbData; - fdbData.bridge_port_id = port.m_bridge_port_id; - fdbData.type = type; - fdbData.origin = origin; - fdbData.remote_ip = remote_ip; - fdbData.esi = esi; - fdbData.vni = vni; + FdbData storeFdbData = fdbData; + storeFdbData.bridge_port_id = port.m_bridge_port_id; - m_entries[entry] = fdbData; + m_entries[entry] = storeFdbData; string key = "Vlan" + to_string(vlan.m_vlan_info.vlan_id) + ":" + entry.mac.to_string(); - if (origin != FDB_ORIGIN_VXLAN_ADVERTIZED) + if (fdbData.origin != FDB_ORIGIN_VXLAN_ADVERTIZED) { /* State-DB is updated only for Local Mac addresses */ // Write to StateDb std::vector fvs; fvs.push_back(FieldValueTuple("port", port_name)); - if (type == "dynamic_local") + if (fdbData.type == "dynamic_local") fvs.push_back(FieldValueTuple("type", "dynamic")); else - fvs.push_back(FieldValueTuple("type", type)); + fvs.push_back(FieldValueTuple("type", fdbData.type)); m_fdbStateTable.set(key, fvs); } else if (macUpdate && (oldOrigin != FDB_ORIGIN_VXLAN_ADVERTIZED)) @@ -1012,7 +1006,7 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const FdbUpdate update; update.entry = entry; update.port = port; - update.type = type; + update.type = fdbData.type; update.add = true; notify(SUBJECT_TYPE_FDB_CHANGE, &update); @@ -1144,6 +1138,7 @@ bool FdbOrch::flushFdbByPort(const string &alias, bool flush_static) sai_status_t status; Port port; sai_attribute_t port_attr[2]; + int port_attr_count=0; if (!m_portsOrch->getPort(alias, port)) { @@ -1161,16 +1156,16 @@ bool FdbOrch::flushFdbByPort(const string &alias, bool flush_static) port_attr[0].id = SAI_FDB_FLUSH_ATTR_BRIDGE_PORT_ID; port_attr[0].value.oid = port.m_bridge_port_id; + port_attr_count++; + if (!flush_static) { port_attr[1].id = SAI_FDB_FLUSH_ATTR_ENTRY_TYPE; port_attr[1].value.s32 = SAI_FDB_FLUSH_ENTRY_TYPE_DYNAMIC; - status = sai_fdb_api->flush_fdb_entries(gSwitchId, 2, port_attr); - } - else - { - status = sai_fdb_api->flush_fdb_entries(gSwitchId, 1, port_attr); + port_attr_count++; } + + status = sai_fdb_api->flush_fdb_entries(gSwitchId, port_attr_count, port_attr); if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Flush fdb failed, return code %x", status); @@ -1184,6 +1179,7 @@ bool FdbOrch::flushFdbByVlan(const string &alias, bool flush_static) sai_status_t status; Port vlan; sai_attribute_t vlan_attr[2]; + int vlan_attr_count=0; if (!m_portsOrch->getPort(alias, vlan)) { @@ -1194,16 +1190,16 @@ bool FdbOrch::flushFdbByVlan(const string &alias, bool flush_static) vlan_attr[0].id = SAI_FDB_FLUSH_ATTR_BV_ID; vlan_attr[0].value.oid = vlan.m_vlan_info.vlan_oid; + vlan_attr_count++; + if (!flush_static) { vlan_attr[1].id = SAI_FDB_FLUSH_ATTR_ENTRY_TYPE; vlan_attr[1].value.s32 = SAI_FDB_FLUSH_ENTRY_TYPE_DYNAMIC; - status = sai_fdb_api->flush_fdb_entries(gSwitchId, 2, vlan_attr); - } - else - { - status = sai_fdb_api->flush_fdb_entries(gSwitchId, 1, vlan_attr); + vlan_attr_count++; } + + status = sai_fdb_api->flush_fdb_entries(gSwitchId, vlan_attr_count, vlan_attr); if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Flush fdb failed, return code %x", status); @@ -1220,6 +1216,7 @@ bool FdbOrch::flushFdbByPortVlan(const string &port_alias, const string &vlan_al Port vlan; Port port; sai_attribute_t port_vlan_attr[3]; + int port_vlan_attr_count=0; SWSS_LOG_NOTICE("port %s vlan %s", port_alias.c_str(), vlan_alias.c_str()); @@ -1244,18 +1241,20 @@ bool FdbOrch::flushFdbByPortVlan(const string &port_alias, const string &vlan_al port_vlan_attr[0].id = SAI_FDB_FLUSH_ATTR_BV_ID; port_vlan_attr[0].value.oid = vlan.m_vlan_info.vlan_oid; + port_vlan_attr_count++; + port_vlan_attr[1].id = SAI_FDB_FLUSH_ATTR_BRIDGE_PORT_ID; port_vlan_attr[1].value.oid = port.m_bridge_port_id; + port_vlan_attr_count++; + if (!flush_static) { port_vlan_attr[2].id = SAI_FDB_FLUSH_ATTR_ENTRY_TYPE; port_vlan_attr[2].value.s32 = SAI_FDB_FLUSH_ENTRY_TYPE_DYNAMIC; - status = sai_fdb_api->flush_fdb_entries(gSwitchId, 3, port_vlan_attr); - } - else - { - status = sai_fdb_api->flush_fdb_entries(gSwitchId, 2, port_vlan_attr); + port_vlan_attr_count++; } + + status = sai_fdb_api->flush_fdb_entries(gSwitchId, port_vlan_attr_count, port_vlan_attr); if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Flush fdb failed, return code %x", status); @@ -1272,9 +1271,9 @@ void FdbOrch::deleteFdbEntryFromSavedFDB(const MacAddress &mac, SavedFdbEntry entry; entry.mac = mac; entry.vlanId = vlanId; - entry.type = "static"; + entry.fdbData.type = "static"; /* Below members are unused during delete compare */ - entry.origin = origin; + entry.fdbData.origin = origin; for (auto& itr: saved_fdb_entries) { @@ -1285,7 +1284,7 @@ void FdbOrch::deleteFdbEntryFromSavedFDB(const MacAddress &mac, { if (*iter == entry) { - if(iter->origin == origin) + if(iter->fdbData.origin == origin) { SWSS_LOG_NOTICE("FDB entry found in saved fdb. deleting..." "mac=%s vlan_id=0x%x origin:%d port:%s", @@ -1302,7 +1301,7 @@ void FdbOrch::deleteFdbEntryFromSavedFDB(const MacAddress &mac, "different mac=%s vlan_id=0x%x reqOrigin:%d " "foundOrigin:%d port:%s, IGNORED", mac.to_string().c_str(), vlanId, origin, - iter->origin, itr.first.c_str()); + iter->fdbData.origin, itr.first.c_str()); } } iter++; diff --git a/orchagent/fdborch.h b/orchagent/fdborch.h index e405fc4c88..b63aa3f880 100644 --- a/orchagent/fdborch.h +++ b/orchagent/fdborch.h @@ -60,11 +60,14 @@ struct SavedFdbEntry { MacAddress mac; unsigned short vlanId; + FdbData fdbData; + /* string type; FdbOrigin origin; string remote_ip; string esi; unsigned int vni; + */ bool operator==(const SavedFdbEntry& other) const { return tie(mac, vlanId) == tie(other.mac, other.vlanId); @@ -110,7 +113,7 @@ class FdbOrch: public Orch, public Subject, public Observer void updateVlanMember(const VlanMemberUpdate&); void updatePortOperState(const PortOperStateUpdate&); - bool addFdbEntry(const FdbEntry&, const string&, const string&, FdbOrigin origin, const string& remote_ip="", unsigned int vni=0, const string& esi=""); + bool addFdbEntry(const FdbEntry&, const string&, FdbData fdbData); void deleteFdbEntryFromSavedFDB(const MacAddress &mac, const unsigned short &vlanId, FdbOrigin origin, const string portName=""); bool storeFdbEntryState(const FdbUpdate& update); From fb8561834402502a0c5f168048ce865b9e6995e1 Mon Sep 17 00:00:00 2001 From: Pankaj Jain Date: Wed, 28 Oct 2020 11:48:05 -0700 Subject: [PATCH 05/21] [Orchagent]: FdbOrch changes for EVPN VXLAN removed commented code. --- orchagent/fdborch.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/orchagent/fdborch.h b/orchagent/fdborch.h index b63aa3f880..f8270b749f 100644 --- a/orchagent/fdborch.h +++ b/orchagent/fdborch.h @@ -61,13 +61,6 @@ struct SavedFdbEntry MacAddress mac; unsigned short vlanId; FdbData fdbData; - /* - string type; - FdbOrigin origin; - string remote_ip; - string esi; - unsigned int vni; - */ bool operator==(const SavedFdbEntry& other) const { return tie(mac, vlanId) == tie(other.mac, other.vlanId); From 073da4a26455a9db96e463a44061664d1627f2a9 Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Wed, 16 Dec 2020 14:03:27 -0800 Subject: [PATCH 06/21] reverted vlanmgr changes --- cfgmgr/vlanmgr.cpp | 220 +++----------------------------------------- cfgmgr/vlanmgr.h | 7 -- cfgmgr/vlanmgrd.cpp | 2 - 3 files changed, 14 insertions(+), 215 deletions(-) diff --git a/cfgmgr/vlanmgr.cpp b/cfgmgr/vlanmgr.cpp index d70984ca81..4570374cb4 100644 --- a/cfgmgr/vlanmgr.cpp +++ b/cfgmgr/vlanmgr.cpp @@ -13,12 +13,10 @@ using namespace swss; #define DOT1Q_BRIDGE_NAME "Bridge" #define VLAN_PREFIX "Vlan" -#define SWITCH_STR "switch" #define LAG_PREFIX "PortChannel" #define DEFAULT_VLAN_ID "1" #define DEFAULT_MTU_STR "9100" #define VLAN_HLEN 4 -#define MAX_VLAN_ID 4095 extern MacAddress gMacAddress; @@ -30,8 +28,6 @@ VlanMgr::VlanMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c m_stateLagTable(stateDb, STATE_LAG_TABLE_NAME), m_stateVlanTable(stateDb, STATE_VLAN_TABLE_NAME), m_stateVlanMemberTable(stateDb, STATE_VLAN_MEMBER_TABLE_NAME), - m_appFdbTableProducer(appDb, APP_FDB_TABLE_NAME), - m_appSwitchTableProducer(appDb, APP_SWITCH_TABLE_NAME), m_appVlanTableProducer(appDb, APP_VLAN_TABLE_NAME), m_appVlanMemberTableProducer(appDb, APP_VLAN_MEMBER_TABLE_NAME) { @@ -94,10 +90,6 @@ VlanMgr::VlanMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c EXEC_WITH_ERROR_THROW(echo_cmd_backup, res); } - /* vlan state notification from portsorch */ - m_VlanStateNotificationConsumer = new swss::NotificationConsumer(appDb, "VLANSTATE"); - auto vlanStatusNotificatier = new Notifier(m_VlanStateNotificationConsumer, this, "VLANSTATE"); - Orch::addExecutor(vlanStatusNotificatier); } bool VlanMgr::addHostVlan(int vlan_id) @@ -111,7 +103,7 @@ bool VlanMgr::addHostVlan(int vlan_id) + BASH_CMD + " -c \"" + BRIDGE_CMD + " vlan add vid " + std::to_string(vlan_id) + " dev " + DOT1Q_BRIDGE_NAME + " self && " + IP_CMD + " link add link " + DOT1Q_BRIDGE_NAME - + " down" + + " up" + " name " + VLAN_PREFIX + std::to_string(vlan_id) + " address " + gMacAddress.to_string() + " type vlan id " + std::to_string(vlan_id) + "\""; @@ -204,29 +196,14 @@ bool VlanMgr::addHostVlanMember(int vlan_id, const string &port_alias, const str // /bin/bash -c "/sbin/ip link set {{port_alias}} master Bridge && // /sbin/bridge vlan del vid 1 dev {{ port_alias }} && // /sbin/bridge vlan add vid {{vlan_id}} dev {{port_alias}} {{tagging_mode}}" - - const std::string key = std::string("") + "Vlan1|" + port_alias; - - if (isVlanMemberStateOk(key)) { - ostringstream cmds, inner; - inner << IP_CMD " link set " << shellquote(port_alias) << " master " DOT1Q_BRIDGE_NAME " && " - BRIDGE_CMD " vlan add vid " + std::to_string(vlan_id) + " dev " << shellquote(port_alias) << " " + tagging_cmd; - cmds << BASH_CMD " -c " << shellquote(inner.str()); - - std::string res; - EXEC_WITH_ERROR_THROW(cmds.str(), res); - } - else - { - ostringstream cmds, inner; - inner << IP_CMD " link set " << shellquote(port_alias) << " master " DOT1Q_BRIDGE_NAME " && " - BRIDGE_CMD " vlan del vid " DEFAULT_VLAN_ID " dev " << shellquote(port_alias) << " && " - BRIDGE_CMD " vlan add vid " + std::to_string(vlan_id) + " dev " << shellquote(port_alias) << " " + tagging_cmd; - cmds << BASH_CMD " -c " << shellquote(inner.str()); + ostringstream cmds, inner; + inner << IP_CMD " link set " << shellquote(port_alias) << " master " DOT1Q_BRIDGE_NAME " && " + BRIDGE_CMD " vlan del vid " DEFAULT_VLAN_ID " dev " << shellquote(port_alias) << " && " + BRIDGE_CMD " vlan add vid " + std::to_string(vlan_id) + " dev " << shellquote(port_alias) << " " + tagging_cmd; + cmds << BASH_CMD " -c " << shellquote(inner.str()); - std::string res; - EXEC_WITH_ERROR_THROW(cmds.str(), res); - } + std::string res; + EXEC_WITH_ERROR_THROW(cmds.str(), res); return true; } @@ -264,137 +241,6 @@ bool VlanMgr::isVlanMacOk() return !!gMacAddress; } -void VlanMgr::doSwitchTask(Consumer &consumer) -{ - SWSS_LOG_ENTER(); - auto it = consumer.m_toSync.begin(); - - while (it != consumer.m_toSync.end()) - { - KeyOpFieldsValuesTuple t = it->second; - - string key = kfvKey(t); - string op = kfvOp(t); - - /* Ensure the key starts with "switch" otherwise ignore */ - if (key != SWITCH_STR) - { - SWSS_LOG_NOTICE("Ignoring SWITCH key %s", key.c_str()); - it = consumer.m_toSync.erase(it); - continue; - } - - SWSS_LOG_DEBUG("key:switch"); - - for (auto i : kfvFieldsValues(t)) - { - if (fvField(i) == "fdb_aging_time") - { - long agingTime = 0; - SWSS_LOG_DEBUG("attribute:fdb_aging_time"); - if (op == SET_COMMAND) - { - SWSS_LOG_DEBUG("operation:set"); - agingTime = strtol(fvValue(i).c_str(), NULL, 0); - if (agingTime < 0) - { - SWSS_LOG_ERROR("Invalid fdb_aging_time %s", fvValue(i).c_str()); - break; - } - SWSS_LOG_DEBUG("value:%s",fvValue(i).c_str()); - } - else if (op == DEL_COMMAND) - { - SWSS_LOG_DEBUG("operation:del"); - agingTime = 0; - } - else - { - SWSS_LOG_ERROR("Unknown operation type %s", op.c_str()); - break; - } - - vector fvVector; - FieldValueTuple aging_time("fdb_aging_time", to_string(agingTime)); - fvVector.push_back(aging_time); - m_appSwitchTableProducer.set(key, fvVector); - break; - } - } - - it = consumer.m_toSync.erase(it); - } -} - -void VlanMgr::doFdbTask(Consumer &consumer) -{ - auto it = consumer.m_toSync.begin(); - - while (it != consumer.m_toSync.end()) - { - KeyOpFieldsValuesTuple t = it->second; - - /* format: | */ - vector keys = tokenize(kfvKey(t), config_db_key_delimiter, 1); - /* keys[0] is vlan as (Vlan10) and keys[1] is mac as (00-00-00-00-00-00) */ - string op = kfvOp(t); - - /* Ensure the key starts with "Vlan" otherwise ignore */ - if (strncmp(keys[0].c_str(), VLAN_PREFIX, 4)) - { - SWSS_LOG_ERROR("Invalid key format. No 'Vlan' prefix: %s", keys[0].c_str()); - it = consumer.m_toSync.erase(it); - continue; - } - - unsigned long vlan_id; - vlan_id = strtoul(keys[0].substr(strlen(VLAN_PREFIX)).c_str(), NULL, 0); - - if ((vlan_id <= 0) || (vlan_id > MAX_VLAN_ID)) - { - SWSS_LOG_ERROR("Invalid key format. Vlan is out of range: %s", keys[0].c_str()); - it = consumer.m_toSync.erase(it); - continue; - } - - MacAddress mac = MacAddress(keys[1]); - - string key = VLAN_PREFIX + to_string(vlan_id); - key += DEFAULT_KEY_SEPARATOR; - key += mac.to_string(); - - if (op == SET_COMMAND) - { - string port; - for (auto i : kfvFieldsValues(t)) - { - if (fvField(i) == "port") - { - port = fvValue(i); - break; - } - } - - vector fvVector; - FieldValueTuple p("port", port); - fvVector.push_back(p); - FieldValueTuple t("type", "static"); - fvVector.push_back(t); - - m_appFdbTableProducer.set(key, fvVector); - } - else if (op == DEL_COMMAND) - { - m_appFdbTableProducer.del(key); - } - else - { - SWSS_LOG_ERROR("Unknown operation type %s", op.c_str()); - } - it = consumer.m_toSync.erase(it); - } -} - void VlanMgr::doVlanTask(Consumer &consumer) { if (!isVlanMacOk()) @@ -468,12 +314,12 @@ void VlanMgr::doVlanTask(Consumer &consumer) { /* Set vlan admin status */ if (fvField(i) == "admin_status") - { - admin_status = fvValue(i); - setHostVlanAdminState(vlan_id, admin_status); - fvVector.push_back(i); - } - /* Set vlan mtu */ + { + admin_status = fvValue(i); + setHostVlanAdminState(vlan_id, admin_status); + fvVector.push_back(i); + } + /* Set vlan mtu */ else if (fvField(i) == "mtu") { mtu = fvValue(i); @@ -773,47 +619,9 @@ void VlanMgr::doTask(Consumer &consumer) { doVlanMemberTask(consumer); } - else if (table_name == CFG_FDB_TABLE_NAME) - { - doFdbTask(consumer); - } - else if (table_name == CFG_SWITCH_TABLE_NAME) - { - SWSS_LOG_DEBUG("Table:SWITCH"); - doSwitchTask(consumer); - } else { SWSS_LOG_ERROR("Unknown config table %s ", table_name.c_str()); throw runtime_error("VlanMgr doTask failure."); } } - -void VlanMgr::doTask(NotificationConsumer &consumer) -{ - std::string op; - std::string data; - std::vector values; - - if (&consumer != m_VlanStateNotificationConsumer) - { - SWSS_LOG_WARN("received incorrect notification message"); - return; - } - - consumer.pop(op, data, values); - - unsigned long vlan_id = strtoul(data.substr(strlen(VLAN_PREFIX)).c_str(), NULL, 0); - - SWSS_LOG_NOTICE("vlanmgr received port status notification state %s vlan %s", - op.c_str(), data.c_str()); - - if (isVlanStateOk(data)) - { - setHostVlanAdminState((int)vlan_id, op); - } - else - { - SWSS_LOG_ERROR("received state update for vlan %s not existing", data.c_str()); - } -} diff --git a/cfgmgr/vlanmgr.h b/cfgmgr/vlanmgr.h index 34ddd00eb4..7c2f0c68b1 100644 --- a/cfgmgr/vlanmgr.h +++ b/cfgmgr/vlanmgr.h @@ -4,7 +4,6 @@ #include "dbconnector.h" #include "producerstatetable.h" #include "orch.h" -#include "notifier.h" #include #include @@ -20,20 +19,14 @@ class VlanMgr : public Orch private: ProducerStateTable m_appVlanTableProducer, m_appVlanMemberTableProducer; - ProducerStateTable m_appFdbTableProducer; - ProducerStateTable m_appSwitchTableProducer; Table m_cfgVlanTable, m_cfgVlanMemberTable; Table m_statePortTable, m_stateLagTable; Table m_stateVlanTable, m_stateVlanMemberTable; std::set m_vlans; - NotificationConsumer* m_VlanStateNotificationConsumer; void doTask(Consumer &consumer); - void doTask(NotificationConsumer &consumer); void doVlanTask(Consumer &consumer); void doVlanMemberTask(Consumer &consumer); - void doFdbTask(Consumer &consumer); - void doSwitchTask(Consumer &consumer); void processUntaggedVlanMembers(std::string vlan, const std::string &members); bool addHostVlan(int vlan_id); diff --git a/cfgmgr/vlanmgrd.cpp b/cfgmgr/vlanmgrd.cpp index 23d25df819..88e4745758 100644 --- a/cfgmgr/vlanmgrd.cpp +++ b/cfgmgr/vlanmgrd.cpp @@ -51,8 +51,6 @@ int main(int argc, char **argv) vector cfg_vlan_tables = { CFG_VLAN_TABLE_NAME, CFG_VLAN_MEMBER_TABLE_NAME, - CFG_FDB_TABLE_NAME, - CFG_SWITCH_TABLE_NAME, }; DBConnector cfgDb("CONFIG_DB", 0); From 7a8315497189686212c1154795970c54e3f4f3b8 Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Wed, 16 Dec 2020 14:53:03 -0800 Subject: [PATCH 07/21] fix build issue --- orchagent/port.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/orchagent/port.h b/orchagent/port.h index 7bde290f1e..14b5c297b5 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -110,8 +110,6 @@ class Port uint32_t m_nat_zone_id = 0; uint32_t m_vnid = VNID_NONE; uint32_t m_fdb_count = 0; - - uint32_t m_fdb_count = 0; uint32_t m_up_member_count = 0; /* From 88f487ec48480e79cc0162ac3a5eef8551ec1ef7 Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Wed, 16 Dec 2020 15:51:02 -0800 Subject: [PATCH 08/21] Update portsorch.cpp --- orchagent/portsorch.cpp | 136 +++++++++++----------------------------- 1 file changed, 38 insertions(+), 98 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index ab6202627b..ec94f45253 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -588,12 +588,34 @@ bool PortsOrch::getPort(sai_object_id_t id, Port &port) { SWSS_LOG_ENTER(); - auto itr = m_portOidToName.find(id); - if (itr == m_portOidToName.end()) - return false; - else { - getPort(itr->second, port); - return true; + for (const auto& portIter: m_portList) + { + switch (portIter.second.m_type) + { + case Port::PHY: + if(portIter.second.m_port_id == id) + { + port = portIter.second; + return true; + } + break; + case Port::LAG: + if(portIter.second.m_lag_id == id) + { + port = portIter.second; + return true; + } + break; + case Port::VLAN: + if (portIter.second.m_vlan_info.vlan_oid == id) + { + port = portIter.second; + return true; + } + break; + default: + continue; + } } return false; @@ -615,12 +637,13 @@ bool PortsOrch::getPortByBridgePortId(sai_object_id_t bridge_port_id, Port &port { SWSS_LOG_ENTER(); - auto itr = m_portOidToName.find(bridge_port_id); - if (itr == m_portOidToName.end()) - return false; - else { - getPort(itr->second, port); - return true; + for (auto &it: m_portList) + { + if (it.second.m_bridge_port_id == bridge_port_id) + { + port = it.second; + return true; + } } return false; @@ -1759,21 +1782,6 @@ void PortsOrch::updateDbPortOperStatus(const Port& port, sai_port_oper_status_t m_portTable->set(port.m_alias, tuples); } -void PortsOrch::updateDbVlanOperStatus(const Port& vlan, string status) const -{ - SWSS_LOG_NOTICE("vlan %s status %s", vlan.m_alias.c_str(), status.c_str()); - - vector tuples; - FieldValueTuple tuple("oper_status", status); - tuples.push_back(tuple); - - std::vector entry; - - SWSS_LOG_NOTICE("sending oper state notification to VlanMgr"); - - notifications->send(status, vlan.m_alias, entry); -} - bool PortsOrch::addPort(const set &lane_set, uint32_t speed, int an, string fec_mode) { SWSS_LOG_ENTER(); @@ -1878,7 +1886,6 @@ bool PortsOrch::initPort(const string &alias, const int index, const set &l /* Add port to port list */ m_portList[alias] = p; m_portOidToIndex[id] = index; - m_portOidToName[id] = alias; m_port_ref_count[alias] = 0; /* Add port name map to counter table */ @@ -2907,13 +2914,6 @@ void PortsOrch::doLagTask(Consumer &consumer) m_portList[alias] = l; } - if (operation_status_changed) - { - PortOperStateUpdate update; - update.port = l; - update.operStatus = string_oper_status.at(operation_status); - notify(SUBJECT_TYPE_PORT_OPER_STATE_CHANGE, static_cast(&update)); - } if ((mtu != 0) && (mtu != l.m_mtu)) { l.m_mtu = mtu; @@ -3511,7 +3511,6 @@ bool PortsOrch::addBridgePort(Port &port) return false; } m_portList[port.m_alias] = port; - m_portOidToName[port.m_bridge_port_id] = port.m_alias; SWSS_LOG_NOTICE("Add bridge port %s to default 1Q bridge", port.m_alias.c_str()); return true; @@ -3568,7 +3567,6 @@ bool PortsOrch::removeBridgePort(Port &port) port.m_alias.c_str(), status); return false; } - m_portOidToName.erase(port.m_bridge_port_id); port.m_bridge_port_id = SAI_NULL_OBJECT_ID; SWSS_LOG_NOTICE("Remove bridge port %s from default 1Q bridge", port.m_alias.c_str()); @@ -3646,7 +3644,6 @@ bool PortsOrch::addVlan(string vlan_alias) vlan.m_vlan_info.vlan_id = vlan_id; vlan.m_members = set(); m_portList[vlan_alias] = vlan; - m_portOidToName[vlan_oid] = vlan_alias; m_port_ref_count[vlan_alias] = 0; return true; @@ -3702,7 +3699,6 @@ bool PortsOrch::removeVlan(Port vlan) SWSS_LOG_NOTICE("Remove VLAN %s vid:%hu", vlan.m_alias.c_str(), vlan.m_vlan_info.vlan_id); - m_portOidToName.erase(vlan.m_vlan_info.vlan_oid); m_portList.erase(vlan.m_alias); m_port_ref_count.erase(vlan.m_alias); @@ -3777,15 +3773,7 @@ bool PortsOrch::addVlanMember(Port &vlan, Port &port, string &tagging_mode) port.m_vlan_members[vlan.m_vlan_info.vlan_id] = vme; m_portList[port.m_alias] = port; vlan.m_members.insert(port.m_alias); - if (port.m_oper_status == SAI_PORT_OPER_STATUS_UP) - { - auto old_count = vlan.m_up_member_count; - vlan.m_up_member_count++; - if (old_count == 0) - { - updateDbVlanOperStatus(vlan, "up"); - } - } + m_portList[vlan.m_alias] = vlan; VlanMemberUpdate update = { vlan, port, true }; @@ -3821,7 +3809,7 @@ bool PortsOrch::removeVlanMember(Port &vlan, Port &port) /* Restore to default pvid if this port joined this VLAN in untagged mode previously */ if (sai_tagging_mode == SAI_VLAN_TAGGING_MODE_UNTAGGED) { - if (!setPortPvid(port, 0)) + if (!setPortPvid(port, DEFAULT_PORT_VLAN_ID)) { return false; } @@ -3829,14 +3817,7 @@ bool PortsOrch::removeVlanMember(Port &vlan, Port &port) m_portList[port.m_alias] = port; vlan.m_members.erase(port.m_alias); - if (port.m_oper_status == SAI_PORT_OPER_STATUS_UP) - { - vlan.m_up_member_count--; - if (vlan.m_up_member_count == 0) - { - updateDbVlanOperStatus(vlan, "down"); - } - } + m_portList[vlan.m_alias] = vlan; VlanMemberUpdate update = { vlan, port, false }; @@ -3883,7 +3864,6 @@ bool PortsOrch::addLag(string lag_alias) lag.m_lag_id = lag_id; lag.m_members = set(); m_portList[lag_alias] = lag; - m_portOidToName[lag_id] = lag_alias; m_port_ref_count[lag_alias] = 0; /* Add lag name map to counter table */ @@ -3936,7 +3916,6 @@ bool PortsOrch::removeLag(Port lag) SWSS_LOG_NOTICE("Remove LAG %s lid:%" PRIx64, lag.m_alias.c_str(), lag.m_lag_id); - m_portOidToName.erase(lag.m_lag_id); m_portList.erase(lag.m_alias); m_port_ref_count.erase(lag.m_alias); @@ -4365,24 +4344,6 @@ void PortsOrch::updatePortOperStatus(Port &port, sai_port_oper_status_t status) bool isUp = status == SAI_PORT_OPER_STATUS_UP; - for(auto vlan_member: port.m_vlan_members) - { - auto Vlan = m_portList[vlan_member.second.alias]; - auto old_count = Vlan.m_up_member_count; - isUp ? Vlan.m_up_member_count++ : Vlan.m_up_member_count--; - if (Vlan.m_up_member_count == 0) - { - updateDbVlanOperStatus(Vlan, "down"); - } - else if ((old_count == 0) && (Vlan.m_up_member_count == 1)) - { - updateDbVlanOperStatus(Vlan, "up"); - } - SWSS_LOG_NOTICE("Vlan %s Port %s state %s m_up_member_count %d", - vlan_member.second.alias.c_str(), port.m_alias.c_str(), oper_status_strings.at(port.m_oper_status).c_str(), Vlan.m_up_member_count); - - m_portList[Vlan.m_alias] = Vlan; - } if (port.m_type == Port::PHY) { if (!setHostIntfsOperStatus(port, isUp)) @@ -4419,27 +4380,6 @@ void PortsOrch::updateLagOperStatus(Port &port, sai_port_oper_status_t status) port.m_oper_status = status; m_portList[port.m_alias] = port; - - bool isUp = status == SAI_PORT_OPER_STATUS_UP; - - for(auto vlan_member: port.m_vlan_members) - { - auto Vlan = m_portList[vlan_member.second.alias]; - auto old_count = Vlan.m_up_member_count; - isUp ? Vlan.m_up_member_count++ : Vlan.m_up_member_count--; - if (Vlan.m_up_member_count == 0) - { - updateDbVlanOperStatus(Vlan, "down"); - } - else if ((old_count == 0) && (Vlan.m_up_member_count == 1)) - { - updateDbVlanOperStatus(Vlan, "up"); - } - SWSS_LOG_NOTICE("Vlan %s Port %s state %s m_up_member_count %d", - vlan_member.second.alias.c_str(), port.m_alias.c_str(), oper_status_strings.at(port.m_oper_status).c_str(), Vlan.m_up_member_count); - - m_portList[Vlan.m_alias] = Vlan; - } } /* From 2c0b5f518202e2a46c8efb27e4d91b52fe020336 Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Fri, 18 Dec 2020 12:08:05 -0800 Subject: [PATCH 09/21] fix build issue fix build issue --- orchagent/fdborch.cpp | 39 ++++++++++++++++++--------------------- orchagent/fdborch.h | 1 + 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/orchagent/fdborch.cpp b/orchagent/fdborch.cpp index 36f1eb20d0..75597c6d23 100644 --- a/orchagent/fdborch.cpp +++ b/orchagent/fdborch.cpp @@ -377,8 +377,8 @@ void FdbOrch::update(sai_fdb_event_t type, no member to indicate the fdb entry type, if there is static mac added, here will have issue. */ - update.entry.mac = itr->mac; - update.entry.bv_id = itr->bv_id; + update.entry.mac = itr->first.mac; + update.entry.bv_id = itr->first.bv_id; update.add = false; itr++; @@ -402,8 +402,8 @@ void FdbOrch::update(sai_fdb_event_t type, auto next_item = std::next(itr); if (itr->port_name == update.port.m_alias) { - update.entry.mac = itr->mac; - update.entry.bv_id = itr->bv_id; + update.entry.mac = itr->first.mac; + update.entry.bv_id = itr->first.bv_id; update.add = false; storeFdbEntryState(update); @@ -778,13 +778,13 @@ void FdbOrch::notifyObserversFDBFlush(Port &port, sai_object_id_t& bvid) for (auto itr = m_entries.begin(); itr != m_entries.end(); ++itr) { if ((itr->port_name == port.m_alias) && - (itr->bv_id == bvid)) + (itr->first.bv_id == bvid)) { SWSS_LOG_INFO("Adding MAC learnt on [ port:%s , bvid:0x%" PRIx64 "]\ to ARP flush", port.m_alias.c_str(), bvid); FdbEntry entry; - entry.mac = itr->mac; - entry.bv_id = itr->bv_id; + entry.mac = itr->first.mac; + entry.bv_id = itr->first.bv_id; flushUpdate.entries.push_back(entry); } } @@ -856,21 +856,18 @@ void FdbOrch::updateVlanMember(const VlanMemberUpdate& update) saved_fdb_entries[port_name].clear(); for (const auto& fdb: fdb_list) { - for (const auto& fdb: fdb_list) + // try to insert an FDB entry. If the FDB entry is not ready to be inserted yet, + // it would be added back to the saved_fdb_entries structure by addFDBEntry() + if(fdb.vlanId == update.vlan.m_vlan_info.vlan_id) { - // try to insert an FDB entry. If the FDB entry is not ready to be inserted yet, - // it would be added back to the saved_fdb_entries structure by addFDBEntry() - if(fdb.vlanId == update.vlan.m_vlan_info.vlan_id) - { - FdbEntry entry; - entry.mac = fdb.mac; - entry.bv_id = update.vlan.m_vlan_info.vlan_oid; - (void)addFdbEntry(entry, port_name, fdb.fdbData); - } - else - { - saved_fdb_entries[port_name].push_back(fdb); - } + FdbEntry entry; + entry.mac = fdb.mac; + entry.bv_id = update.vlan.m_vlan_info.vlan_oid; + (void)addFdbEntry(entry, port_name, fdb.fdbData); + } + else + { + saved_fdb_entries[port_name].push_back(fdb); } } } diff --git a/orchagent/fdborch.h b/orchagent/fdborch.h index e18d00c07b..378794b741 100644 --- a/orchagent/fdborch.h +++ b/orchagent/fdborch.h @@ -17,6 +17,7 @@ struct FdbEntry { MacAddress mac; sai_object_id_t bv_id; + std::string port_name; bool operator<(const FdbEntry& other) const { From e119e4f69fbb6e8b201094f6a100d62ffd0757f8 Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Fri, 18 Dec 2020 12:24:43 -0800 Subject: [PATCH 10/21] Update fdborch.cpp --- orchagent/fdborch.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/orchagent/fdborch.cpp b/orchagent/fdborch.cpp index 75597c6d23..6e98f6ae7f 100644 --- a/orchagent/fdborch.cpp +++ b/orchagent/fdborch.cpp @@ -400,7 +400,7 @@ void FdbOrch::update(sai_fdb_event_t type, for (auto itr = m_entries.begin(); itr != m_entries.end();) { auto next_item = std::next(itr); - if (itr->port_name == update.port.m_alias) + if (itr->first.port_name == update.port.m_alias) { update.entry.mac = itr->first.mac; update.entry.bv_id = itr->first.bv_id; @@ -777,7 +777,7 @@ void FdbOrch::notifyObserversFDBFlush(Port &port, sai_object_id_t& bvid) for (auto itr = m_entries.begin(); itr != m_entries.end(); ++itr) { - if ((itr->port_name == port.m_alias) && + if ((itr->first.port_name == port.m_alias) && (itr->first.bv_id == bvid)) { SWSS_LOG_INFO("Adding MAC learnt on [ port:%s , bvid:0x%" PRIx64 "]\ From 4c58722e1f05f9e114c00c999f8c11061fac4c7c Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Fri, 18 Dec 2020 14:45:04 -0800 Subject: [PATCH 11/21] Update aclorch_ut.cpp --- tests/mock_tests/aclorch_ut.cpp | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index e932cf29be..4c5c565483 100644 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -316,8 +316,13 @@ namespace aclorch_test TableConnector applDbFdb(m_app_db.get(), APP_FDB_TABLE_NAME); TableConnector stateDbFdb(m_state_db.get(), STATE_FDB_TABLE_NAME); + vector app_fdb_tables = { + { APP_FDB_TABLE_NAME, FdbOrch::fdborch_pri}, + { APP_VXLAN_FDB_TABLE_NAME, FdbOrch::fdborch_pri} + }; + ASSERT_EQ(gFdbOrch, nullptr); - gFdbOrch = new FdbOrch(applDbFdb, stateDbFdb, gPortsOrch); + gFdbOrch = new FdbOrch(m_app_db.get(), app_fdb_tables, stateDbFdb, gPortsOrch); ASSERT_EQ(gNeighOrch, nullptr); gNeighOrch = new NeighOrch(m_app_db.get(), APP_NEIGH_TABLE_NAME, gIntfsOrch, gFdbOrch, gPortsOrch); @@ -333,16 +338,6 @@ namespace aclorch_test ASSERT_EQ(gRouteOrch, nullptr); gRouteOrch = new RouteOrch(m_app_db.get(), APP_ROUTE_TABLE_NAME, gSwitchOrch, gNeighOrch, gIntfsOrch, gVrfOrch, gFgNhgOrch); - vector app_fdb_tables = { - { APP_FDB_TABLE_NAME, FdbOrch::fdborch_pri}, - { APP_VXLAN_FDB_TABLE_NAME, FdbOrch::fdborch_pri} - }; - - TableConnector stateDbFdb(m_state_db.get(), STATE_FDB_TABLE_NAME); - - ASSERT_EQ(gFdbOrch, nullptr); - gFdbOrch = new FdbOrch(m_app_db.get(), app_fdb_tables, stateDbFdb, gPortsOrch); - PolicerOrch *policer_orch = new PolicerOrch(m_config_db.get(), "POLICER"); TableConnector stateDbMirrorSession(m_state_db.get(), STATE_MIRROR_SESSION_TABLE_NAME); From e84411e3d43800061a7ed09c41ffe6f87f6e7fba Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Fri, 18 Dec 2020 16:06:53 -0800 Subject: [PATCH 12/21] fix lgtm --- orchagent/portsorch.cpp | 2 +- orchagent/portsorch.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index c979421796..b26a42bd8b 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -260,7 +260,7 @@ PortsOrch::PortsOrch(DBConnector *db, vector &tableNames) m_flexCounterGroupTable = unique_ptr(new ProducerTable(m_flex_db.get(), FLEX_COUNTER_GROUP_TABLE)); initGearbox(); - notifications = new swss::NotificationProducer(db, "VLANSTATE"); + notifications = unique_ptr(new swss::NotificationProducer(db, "VLANSTATE")); string queueWmSha, pgWmSha; string queueWmPluginName = "watermark_queue.lua"; diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 90e98ef7ff..6f78930291 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -156,7 +156,7 @@ class PortsOrch : public Orch, public Subject unique_ptr
m_pgIndexTable; unique_ptr m_flexCounterTable; unique_ptr m_flexCounterGroupTable; - NotificationProducer* notifications; + unique_ptr notifications; std::string getQueueWatermarkFlexCounterTableKey(std::string s); std::string getPriorityGroupWatermarkFlexCounterTableKey(std::string s); From e2097501021434a1e862160919522cc068aabd6c Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Mon, 21 Dec 2020 11:08:54 -0800 Subject: [PATCH 13/21] Update test_evpn_fdb.py --- tests/test_evpn_fdb.py | 46 ------------------------------------------ 1 file changed, 46 deletions(-) diff --git a/tests/test_evpn_fdb.py b/tests/test_evpn_fdb.py index c5974ba055..cc62b3b56d 100644 --- a/tests/test_evpn_fdb.py +++ b/tests/test_evpn_fdb.py @@ -382,52 +382,6 @@ def test_evpnFdb(dvs, testlog): #raw_input("Check ASIC_DB.........") - #UT-5 create a Static FDB entry - mac = "52:54:00:25:06:E9" - print("Creating static FDB Vlan3:"+mac.lower()+":Ethernet0 in CONFIG-DB") - create_entry_tbl( - dvs.cdb, - "FDB", "Vlan3|"+mac.lower(), - [ - ("port", "Ethernet0"), - ("type", "static"), - ] - ) - time.sleep(2) - - #raw_input("Check ASIC_DB.........") - - # check that the FDB entry was added in APP DB - mac1_found, extra = dvs.is_table_entry_exists(dvs.pdb, "FDB_TABLE", - "Vlan3:"+mac.lower(), - [("port", "Ethernet0"), - ("type", "static"), - ] - ) - assert mac1_found, str(extra) - print("Static FDB Vlan3:"+mac.lower()+"Ethernet0 is created in APP-DB") - - # check that the FDB entry is now inserted into ASIC DB - ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", - [("mac", mac.upper()), ("bvid", vlan_oid_3)], - [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), - ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", iface_2_bridge_port_id["Ethernet0"])] - ) - assert ok, str(extra) - print("Static FDB Vlan3:"+mac.lower()+":Ethernet0 is created in ASIC-DB") - - # check that the FDB entry was added in STATE DB - mac1_found, extra = dvs.is_table_entry_exists(dvs.sdb, "FDB_TABLE", - "Vlan3:"+mac.lower(), - [("port", "Ethernet0"), - ("type", "static"), - ] - ) - assert mac1_found, str(extra) - print("Static FDB Vlan3:"+mac.lower()+":Ethernet0 is created in STATE-DB") - - #raw_input("Check ASIC_DB.........") - #UT-6 Evpn Mac del from remote when only local is present; local mac should not get affected mac = "52:54:00:25:06:E9" print("Deleting Evpn FDB Vlan3:"+mac.lower()+":6.6.6.6 in APP-DB") From eedde3cdd84e83a25d078c968eb9b29c2c6f6539 Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Mon, 21 Dec 2020 15:14:31 -0800 Subject: [PATCH 14/21] Update test_evpn_fdb.py --- tests/test_evpn_fdb.py | 135 ----------------------------------------- 1 file changed, 135 deletions(-) diff --git a/tests/test_evpn_fdb.py b/tests/test_evpn_fdb.py index cc62b3b56d..4c1f2ef247 100644 --- a/tests/test_evpn_fdb.py +++ b/tests/test_evpn_fdb.py @@ -393,141 +393,6 @@ def test_evpnFdb(dvs, testlog): #raw_input("Check ASIC_DB.........") - # check that the existing local fdb entry is not deleted and still available in ASIC DB - ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", - [("mac", mac), ("bvid", vlan_oid_3)], - [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), - ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", iface_2_bridge_port_id["Ethernet0"]), - ] - ) - assert ok, str(extra) - print("Local FDB Vlan3:"+mac.lower()+":Ethernet0 is not deleted from ASIC-DB") - - # check that the local FDB entry is still available in STATE DB - mac1_found, extra = dvs.is_table_entry_exists(dvs.sdb, "FDB_TABLE", - "Vlan3:"+mac.lower(), - [("port", "Ethernet0"), - ("type", "static"), - ] - ) - assert mac1_found, str(extra) - print("Local FDB Vlan3:"+mac.lower()+":Ethernet0 is not deleted STATE-DB") - - time.sleep(1) - - #UT-7 Evpn Mac add from remote when local static mac is already available - mac = "52:54:00:25:06:E9" - print("Creating Evpn dynamic FDB Vlan3:"+mac.lower()+":6.6.6.6 in APP-DB") - create_entry_pst( - dvs.pdb, - "VXLAN_FDB_TABLE", "Vlan3:"+mac.lower(), - [ - ("remote_vtep", remote_ip_6), - ("type", "dynamic"), - ("vni", "3") - ] - ) - time.sleep(1) - - #raw_input("Check ASIC_DB.........") - - # check that the FDB entry is not inserted into ASIC DB - ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", - [("mac", mac), ("bvid", vlan_oid_3)], - [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), - ("SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE", "true"), - ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_6), - ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_6)), - ] - ) - assert ok == False, str(extra) - print("EVPN dynamic FDB Vlan3:"+mac.lower()+":"+remote_ip_6+" is not created in ASIC-DB") - - time.sleep(1) - - print("Creating Evpn sticky FDB Vlan3:"+mac.lower()+":6.6.6.6 in APP-DB") - create_entry_pst( - dvs.pdb, - "VXLAN_FDB_TABLE", "Vlan3:"+mac.lower(), - [ - ("remote_vtep", remote_ip_6), - ("type", "static"), - ("vni", "3") - ] - ) - time.sleep(1) - - #raw_input("Check ASIC_DB.........") - - # check that the FDB entry is not inserted into ASIC DB - ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", - [("mac", mac), ("bvid", vlan_oid_3)], - [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), - ("SAI_FDB_ENTRY_ATTR_ENDPOINT_IP", remote_ip_6), - ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", str(tnl_bp_oid_6)), - ] - ) - assert ok == False, str(extra) - print("EVPN sticky FDB Vlan3:"+mac.lower()+":"+remote_ip_6+" is not created in ASIC-DB") - - # Delete Static FDB entry - mac = "52:54:00:25:06:E9" - print("Deleting static FDB Vlan3:"+mac.lower()+":Ethernet0 from CONFIG-DB") - delete_entry_tbl( - dvs.cdb, - "FDB", "Vlan3|"+mac.lower() - ) - time.sleep(2) - - #raw_input("Check ASIC_DB.........") - - # check that the FDB entry was deleted in APP DB - mac1_found, extra = dvs.is_table_entry_exists(dvs.pdb, "FDB_TABLE", - "Vlan3:"+mac.lower(), - [("port", "Ethernet0"), - ("type", "static"), - ] - ) - assert mac1_found == False, str(extra) - print("Static FDB Vlan3:"+mac.lower()+"Ethernet0 is deleted from APP-DB") - - # check that the FDB entry is deleted in ASIC DB - ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", - [("mac", mac.upper()), ("bvid", vlan_oid_3)], - [("SAI_FDB_ENTRY_ATTR_TYPE", "SAI_FDB_ENTRY_TYPE_STATIC"), - ("SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID", iface_2_bridge_port_id["Ethernet0"])] - ) - assert ok == False, str(extra) - print("Static FDB Vlan3:"+mac.lower()+":Ethernet0 is deleted in ASIC-DB") - - # check that the FDB entry was deleted from STATE DB - mac1_found, extra = dvs.is_table_entry_exists(dvs.sdb, "FDB_TABLE", - "Vlan3:"+mac.lower(), - [("port", "Ethernet0"), - ("type", "static"), - ] - ) - assert mac1_found == False, str(extra) - print("Static FDB Vlan3:"+mac.lower()+":Ethernet0 is deleted from STATE-DB") - - #raw_input("Check ASIC_DB.........") - - #UT-8 Evpn Mac add from remote when tunnels are already created - mac = "52:54:00:25:06:E9" - print("Creating Evpn FDB Vlan3:"+mac.lower()+":6.6.6.6 in APP-DB") - create_entry_pst( - dvs.pdb, - "VXLAN_FDB_TABLE", "Vlan3:"+mac.lower(), - [ - ("remote_vtep", remote_ip_6), - ("type", "dynamic"), - ("vni", "3") - ] - ) - time.sleep(1) - - #raw_input("Check ASIC_DB.........") - # check that the FDB entry is inserted into ASIC DB ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", [("mac", mac), ("bvid", vlan_oid_3)], From ef3adf85365f2c9b5db70842c5567f39176f5031 Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Mon, 21 Dec 2020 18:38:21 -0800 Subject: [PATCH 15/21] Update test_evpn_fdb.py --- tests/test_evpn_fdb.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/test_evpn_fdb.py b/tests/test_evpn_fdb.py index 4c1f2ef247..0989815b9d 100644 --- a/tests/test_evpn_fdb.py +++ b/tests/test_evpn_fdb.py @@ -382,12 +382,17 @@ def test_evpnFdb(dvs, testlog): #raw_input("Check ASIC_DB.........") - #UT-6 Evpn Mac del from remote when only local is present; local mac should not get affected + #UT-8 Evpn Mac add from remote when tunnels are already created mac = "52:54:00:25:06:E9" - print("Deleting Evpn FDB Vlan3:"+mac.lower()+":6.6.6.6 in APP-DB") - delete_entry_pst( + print("Creating Evpn FDB Vlan3:"+mac.lower()+":6.6.6.6 in APP-DB") + create_entry_pst( dvs.pdb, - "VXLAN_FDB_TABLE", "Vlan3:"+mac.lower() + "VXLAN_FDB_TABLE", "Vlan3:"+mac.lower(), + [ + ("remote_vtep", remote_ip_6), + ("type", "dynamic"), + ("vni", "3") + ] ) time.sleep(1) From 5f38e64c1bec0167d581fdd7b36aa98bd9337a92 Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Wed, 23 Dec 2020 15:55:54 -0800 Subject: [PATCH 16/21] remove fdb flush related changes removed fdb flush and related changes as per discussion with MSFT --- orchagent/fdborch.cpp | 274 ++++++---------------------------------- orchagent/fdborch.h | 4 - orchagent/port.h | 1 - orchagent/portsorch.cpp | 233 ++++++++-------------------------- orchagent/portsorch.h | 14 +- 5 files changed, 91 insertions(+), 435 deletions(-) diff --git a/orchagent/fdborch.cpp b/orchagent/fdborch.cpp index 6e98f6ae7f..1679fa8cb1 100644 --- a/orchagent/fdborch.cpp +++ b/orchagent/fdborch.cpp @@ -200,10 +200,6 @@ void FdbOrch::update(sai_fdb_event_t type, update.add = true; update.type = "dynamic"; - update.port.m_fdb_count++; - m_portsOrch->setPort(update.port.m_alias, update.port); - vlan.m_fdb_count++; - m_portsOrch->setPort(vlan.m_alias, vlan); storeFdbEntryState(update); notify(SUBJECT_TYPE_FDB_CHANGE, &update); @@ -282,16 +278,6 @@ void FdbOrch::update(sai_fdb_event_t type, } update.add = false; - if (!update.port.m_alias.empty()) - { - update.port.m_fdb_count--; - m_portsOrch->setPort(update.port.m_alias, update.port); - } - if (!vlan.m_alias.empty()) - { - vlan.m_fdb_count--; - m_portsOrch->setPort(vlan.m_alias, vlan); - } storeFdbEntryState(update); @@ -327,14 +313,6 @@ void FdbOrch::update(sai_fdb_event_t type, update.add = true; - if (!port_old.m_alias.empty()) - { - port_old.m_fdb_count--; - m_portsOrch->setPort(port_old.m_alias, port_old); - } - update.port.m_fdb_count++; - m_portsOrch->setPort(update.port.m_alias, update.port); - storeFdbEntryState(update); notify(SUBJECT_TYPE_FDB_CHANGE, &update); @@ -465,7 +443,6 @@ void FdbOrch::update(SubjectType type, void *cntx) bool FdbOrch::getPort(const MacAddress& mac, uint16_t vlan, Port& port) { - sai_object_id_t bridge_port_id; SWSS_LOG_ENTER(); if (!m_portsOrch->getVlanByVlanId(vlan, port)) @@ -474,22 +451,25 @@ bool FdbOrch::getPort(const MacAddress& mac, uint16_t vlan, Port& port) return false; } - FdbEntry entry; + sai_fdb_entry_t entry; + entry.switch_id = gSwitchId; + memcpy(entry.mac_address, mac.getMac(), sizeof(sai_mac_t)); entry.bv_id = port.m_vlan_info.vlan_oid; - entry.mac = mac; - auto it= m_entries.find(entry); - if(it != m_entries.end()) - { - bridge_port_id = it->second.bridge_port_id; - } - else + + sai_attribute_t attr; + attr.id = SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID; + + sai_status_t status = sai_fdb_api->get_fdb_entry_attribute(&entry, 1, &attr); + if (status != SAI_STATUS_SUCCESS) { + SWSS_LOG_ERROR("Failed to get bridge port ID for FDB entry %s, rv:%d", + mac.to_string().c_str(), status); return false; } - if (!m_portsOrch->getPortByBridgePortId(bridge_port_id, port)) + if (!m_portsOrch->getPortByBridgePortId(attr.value.oid, port)) { - SWSS_LOG_ERROR("Failed to get port by bridge port ID 0x%" PRIx64, bridge_port_id); + SWSS_LOG_ERROR("Failed to get port by bridge port ID 0x%" PRIx64, attr.value.oid); return false; } @@ -662,6 +642,7 @@ void FdbOrch::doTask(NotificationConsumer& consumer) return; } + sai_status_t status; std::string op; std::string data; std::vector values; @@ -672,17 +653,28 @@ void FdbOrch::doTask(NotificationConsumer& consumer) { if (op == "ALL") { - flushFdbAll(0); + /* + * so far only support flush all the FDB entris + * flush per port and flush per vlan will be added later. + */ + status = sai_fdb_api->flush_fdb_entries(gSwitchId, 0, NULL); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Flush fdb failed, return code %x", status); + } + return; } else if (op == "PORT") { - flushFdbByPort(data, 0); + /*place holder for flush port fdb*/ + SWSS_LOG_ERROR("Received unsupported flush port fdb request"); return; } else if (op == "VLAN") { - flushFdbByVlan(data, 0); + /*place holder for flush vlan fdb*/ + SWSS_LOG_ERROR("Received unsupported flush vlan fdb request"); return; } else @@ -826,49 +818,25 @@ void FdbOrch::updatePortOperState(const PortOperStateUpdate& update) void FdbOrch::updateVlanMember(const VlanMemberUpdate& update) { - Port port; - - string port_name = update.member.m_alias; - string vlan_name = update.vlan.m_alias; - SWSS_LOG_ENTER(); - if (!m_portsOrch->getPort(port_name, port)) - { - SWSS_LOG_ERROR("could not locate port from alias %s", port_name.c_str()); - return; - } - if (!update.add) { - if(port.m_type != Port::TUNNEL) - { - flushFdbByPortVlan(port_name, vlan_name, 1); - swss::Port vlan = update.vlan; - swss::Port port = update.member; - flushFDBEntries(port.m_bridge_port_id, vlan.m_vlan_info.vlan_oid); - notifyObserversFDBFlush(port, vlan.m_vlan_info.vlan_oid); - return; - } + swss::Port vlan = update.vlan; + swss::Port port = update.member; + flushFDBEntries(port.m_bridge_port_id, vlan.m_vlan_info.vlan_oid); + notifyObserversFDBFlush(port, vlan.m_vlan_info.vlan_oid); + return; } + string port_name = update.member.m_alias; auto fdb_list = std::move(saved_fdb_entries[port_name]); saved_fdb_entries[port_name].clear(); for (const auto& fdb: fdb_list) { // try to insert an FDB entry. If the FDB entry is not ready to be inserted yet, // it would be added back to the saved_fdb_entries structure by addFDBEntry() - if(fdb.vlanId == update.vlan.m_vlan_info.vlan_id) - { - FdbEntry entry; - entry.mac = fdb.mac; - entry.bv_id = update.vlan.m_vlan_info.vlan_oid; - (void)addFdbEntry(entry, port_name, fdb.fdbData); - } - else - { - saved_fdb_entries[port_name].push_back(fdb); - } + (void)addFdbEntry(fdb.entry, port_name, fdb.fdbData); } } @@ -1075,13 +1043,6 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, return false; } } - if (oldPort.m_bridge_port_id != port.m_bridge_port_id) - { - oldPort.m_fdb_count--; - m_portsOrch->setPort(oldPort.m_alias, oldPort); - port.m_fdb_count++; - m_portsOrch->setPort(port.m_alias, port); - } } else { @@ -1095,10 +1056,6 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, vlan.m_alias.c_str(), port_name.c_str(), status); return false; //FIXME: it should be based on status. Some could be retried, some not } - port.m_fdb_count++; - m_portsOrch->setPort(port.m_alias, port); - vlan.m_fdb_count++; - m_portsOrch->setPort(vlan.m_alias, vlan); } FdbData storeFdbData = fdbData; @@ -1214,10 +1171,6 @@ bool FdbOrch::removeFdbEntry(const FdbEntry& entry, FdbOrigin origin) SWSS_LOG_NOTICE("Removed mac=%s bv_id=0x%lx port:%s", entry.mac.to_string().c_str(), entry.bv_id, port.m_alias.c_str()); - port.m_fdb_count--; - m_portsOrch->setPort(port.m_alias, port); - vlan.m_fdb_count--; - m_portsOrch->setPort(vlan.m_alias, vlan); (void)m_entries.erase(entry); // Remove in StateDb @@ -1241,160 +1194,6 @@ bool FdbOrch::removeFdbEntry(const FdbEntry& entry, FdbOrigin origin) return true; } -bool FdbOrch::flushFdbAll(bool flush_static) -{ - sai_status_t status; - sai_attribute_t port_attr; - - if (!flush_static) - { - port_attr.id = SAI_FDB_FLUSH_ATTR_ENTRY_TYPE; - port_attr.value.s32 = SAI_FDB_FLUSH_ENTRY_TYPE_DYNAMIC; - status = sai_fdb_api->flush_fdb_entries(gSwitchId, 1, &port_attr); - } - else - { - status = sai_fdb_api->flush_fdb_entries(gSwitchId, 0, NULL); - } - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Flush fdb failed, return code %x", status); - return false; - } - return true; -} - -bool FdbOrch::flushFdbByPort(const string &alias, bool flush_static) -{ - sai_status_t status; - Port port; - sai_attribute_t port_attr[2]; - int port_attr_count=0; - - if (!m_portsOrch->getPort(alias, port)) - { - SWSS_LOG_ERROR("could not locate port from alias %s", alias.c_str()); - return false; - } - - if ((port.m_bridge_port_id == SAI_NULL_OBJECT_ID) || !port.m_fdb_count) - { - /* port is not an L2 port or no macs to flush */ - return true; - } - - SWSS_LOG_NOTICE("m_bridge_port_id 0x%lx flush_static %d m_fdb_count %u", port.m_bridge_port_id, flush_static, port.m_fdb_count); - - port_attr[0].id = SAI_FDB_FLUSH_ATTR_BRIDGE_PORT_ID; - port_attr[0].value.oid = port.m_bridge_port_id; - port_attr_count++; - - if (!flush_static) - { - port_attr[1].id = SAI_FDB_FLUSH_ATTR_ENTRY_TYPE; - port_attr[1].value.s32 = SAI_FDB_FLUSH_ENTRY_TYPE_DYNAMIC; - port_attr_count++; - } - - status = sai_fdb_api->flush_fdb_entries(gSwitchId, port_attr_count, port_attr); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Flush fdb failed, return code %x", status); - return false; - } - return true; -} - -bool FdbOrch::flushFdbByVlan(const string &alias, bool flush_static) -{ - sai_status_t status; - Port vlan; - sai_attribute_t vlan_attr[2]; - int vlan_attr_count=0; - - if (!m_portsOrch->getPort(alias, vlan)) - { - SWSS_LOG_ERROR("could not locate vlan from alias %s", alias.c_str()); - return false; - } - SWSS_LOG_NOTICE("vlan_oid 0x%lx flush_static %d", vlan.m_vlan_info.vlan_oid, flush_static); - - vlan_attr[0].id = SAI_FDB_FLUSH_ATTR_BV_ID; - vlan_attr[0].value.oid = vlan.m_vlan_info.vlan_oid; - vlan_attr_count++; - - if (!flush_static) - { - vlan_attr[1].id = SAI_FDB_FLUSH_ATTR_ENTRY_TYPE; - vlan_attr[1].value.s32 = SAI_FDB_FLUSH_ENTRY_TYPE_DYNAMIC; - vlan_attr_count++; - } - - status = sai_fdb_api->flush_fdb_entries(gSwitchId, vlan_attr_count, vlan_attr); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Flush fdb failed, return code %x", status); - return false; - } - - return true; -} - -bool FdbOrch::flushFdbByPortVlan(const string &port_alias, const string &vlan_alias, bool flush_static) -{ - - sai_status_t status; - Port vlan; - Port port; - sai_attribute_t port_vlan_attr[3]; - int port_vlan_attr_count=0; - - SWSS_LOG_NOTICE("port %s vlan %s", port_alias.c_str(), vlan_alias.c_str()); - - if (!m_portsOrch->getPort(port_alias, port)) - { - SWSS_LOG_ERROR("could not locate port from alias %s", port_alias.c_str()); - return false; - } - if (!m_portsOrch->getPort(vlan_alias, vlan)) - { - SWSS_LOG_NOTICE("FdbOrch notification: Failed to locate vlan %s", vlan_alias.c_str()); - return false; - } - - if ((port.m_bridge_port_id == SAI_NULL_OBJECT_ID) || !port.m_fdb_count) - { - /* port is not an L2 port or no macs to flush */ - return true; - } - - SWSS_LOG_NOTICE("vlan_oid 0x%lx m_bridge_port_id 0x%lx flush_static %d m_fdb_count %u", vlan.m_vlan_info.vlan_oid, port.m_bridge_port_id, flush_static, port.m_fdb_count); - - port_vlan_attr[0].id = SAI_FDB_FLUSH_ATTR_BV_ID; - port_vlan_attr[0].value.oid = vlan.m_vlan_info.vlan_oid; - port_vlan_attr_count++; - - port_vlan_attr[1].id = SAI_FDB_FLUSH_ATTR_BRIDGE_PORT_ID; - port_vlan_attr[1].value.oid = port.m_bridge_port_id; - port_vlan_attr_count++; - - if (!flush_static) - { - port_vlan_attr[2].id = SAI_FDB_FLUSH_ATTR_ENTRY_TYPE; - port_vlan_attr[2].value.s32 = SAI_FDB_FLUSH_ENTRY_TYPE_DYNAMIC; - port_vlan_attr_count++; - } - - status = sai_fdb_api->flush_fdb_entries(gSwitchId, port_vlan_attr_count, port_vlan_attr); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Flush fdb failed, return code %x", status); - return false; - } - - return true; -} - void FdbOrch::deleteFdbEntryFromSavedFDB(const MacAddress &mac, const unsigned short &vlanId, FdbOrigin origin, const string portName) { @@ -1448,8 +1247,7 @@ void FdbOrch::notifyTunnelOrch(Port& port) { VxlanTunnelOrch* tunnel_orch = gDirectory.get(); - if((port.m_type != Port::TUNNEL) || - (port.m_fdb_count != 0)) + if(port.m_type != Port::TUNNEL) return; tunnel_orch->deleteTunnelPort(port); diff --git a/orchagent/fdborch.h b/orchagent/fdborch.h index 378794b741..201493f574 100644 --- a/orchagent/fdborch.h +++ b/orchagent/fdborch.h @@ -92,10 +92,6 @@ class FdbOrch: public Orch, public Subject, public Observer void update(SubjectType type, void *cntx); bool getPort(const MacAddress&, uint16_t, Port&); - bool flushFdbByPortVlan(const string &, const string &, bool flush_static); - bool flushFdbByVlan(const string &, bool flush_static); - bool flushFdbByPort(const string &, bool flush_static); - bool flushFdbAll(bool flush_static); bool removeFdbEntry(const FdbEntry& entry, FdbOrigin origin=FDB_ORIGIN_PROVISIONED); static const int fdborch_pri; diff --git a/orchagent/port.h b/orchagent/port.h index bc0d72cb98..0db155428f 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -88,7 +88,6 @@ class Port VlanInfo m_vlan_info; MacAddress m_mac; sai_object_id_t m_bridge_port_id = 0; // TODO: port could have multiple bridge port IDs - sai_object_id_t m_bridge_port_admin_state = 0; // TODO: port could have multiple bridge port IDs sai_vlan_id_t m_port_vlan_id = DEFAULT_PORT_VLAN_ID; // Port VLAN ID sai_object_id_t m_rif_id = 0; sai_object_id_t m_vr_id = 0; diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 3b2f366989..f4a8831627 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -29,7 +29,6 @@ #include "notifier.h" #include "fdborch.h" - extern sai_switch_api_t *sai_switch_api; extern sai_bridge_api_t *sai_bridge_api; extern sai_port_api_t *sai_port_api; @@ -239,7 +238,6 @@ PortsOrch::PortsOrch(DBConnector *db, vector &tableNames) /* Initialize port and vlan table */ m_portTable = unique_ptr
(new Table(db, APP_PORT_TABLE_NAME)); - m_vlanTable = unique_ptr
(new Table(db, APP_VLAN_TABLE_NAME)); /* Initialize gearbox */ m_gearboxTable = unique_ptr
(new Table(db, "_GEARBOX_TABLE")); @@ -260,7 +258,6 @@ PortsOrch::PortsOrch(DBConnector *db, vector &tableNames) m_flexCounterGroupTable = unique_ptr(new ProducerTable(m_flex_db.get(), FLEX_COUNTER_GROUP_TABLE)); initGearbox(); - notifications = unique_ptr(new swss::NotificationProducer(db, "VLANSTATE")); string queueWmSha, pgWmSha; string queueWmPluginName = "watermark_queue.lua"; @@ -395,19 +392,7 @@ PortsOrch::PortsOrch(DBConnector *db, vector &tableNames) } m_default1QBridge = attrs[0].value.oid; - m_defaultVlan_ObjId = attrs[1].value.oid; - - memset(&attr, 0x00, sizeof(attr)); - attr.id = SAI_VLAN_ATTR_VLAN_ID; - - status = sai_vlan_api->get_vlan_attribute(m_defaultVlan_ObjId, 1, &attr); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to get default VLAN ID, rv:%d", status); - throw runtime_error("PortsOrch initialization failure"); - } - - m_defaultVlan_Id = attr.value.u16; + m_defaultVlan = attrs[1].value.oid; removeDefaultVlanMembers(); removeDefaultBridgePorts(); @@ -429,7 +414,7 @@ void PortsOrch::removeDefaultVlanMembers() attr.value.objlist.count = (uint32_t)vlan_member_list.size(); attr.value.objlist.list = vlan_member_list.data(); - sai_status_t status = sai_vlan_api->get_vlan_attribute(m_defaultVlan_ObjId, 1, &attr); + sai_status_t status = sai_vlan_api->get_vlan_attribute(m_defaultVlan, 1, &attr); if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to get VLAN member list in default VLAN, rv:%d", status); @@ -2842,23 +2827,24 @@ void PortsOrch::doVlanMemberTask(Consumer &consumer) } else if (op == DEL_COMMAND) { - int ret = true; if (vlan.m_members.find(port_alias) != vlan.m_members.end()) { - ret = removeVlanMember(vlan, port); - } - if ((ret) && port.m_vlan_members.empty()) - { - ret = removeBridgePort(port); - } - if (ret) - { - it = consumer.m_toSync.erase(it); + if (removeVlanMember(vlan, port)) + { + if (port.m_vlan_members.empty()) + { + removeBridgePort(port); + } + it = consumer.m_toSync.erase(it); + } + else + { + it++; + } } else - { - it++; - } + /* Cannot locate the VLAN */ + it = consumer.m_toSync.erase(it); } else { @@ -2884,7 +2870,6 @@ void PortsOrch::doLagTask(Consumer &consumer) { // Retrieve attributes uint32_t mtu = 0; - string learn_mode; string operation_status; @@ -2910,7 +2895,15 @@ void PortsOrch::doLagTask(Consumer &consumer) } } - auto status = addLag(alias); + // Create a new LAG when the new alias comes + if (m_portList.find(alias) == m_portList.end()) + { + if (!addLag(alias)) + { + it++; + continue; + } + } // Process attributes Port l; @@ -2926,7 +2919,8 @@ void PortsOrch::doLagTask(Consumer &consumer) m_portList[alias] = l; } - if ((mtu != 0) && (mtu != l.m_mtu)) + + if (mtu != 0) { l.m_mtu = mtu; m_portList[alias] = l; @@ -2938,9 +2932,6 @@ void PortsOrch::doLagTask(Consumer &consumer) updateChildPortsMtu(l, mtu); } - sai_port_oper_status_t status = (operation_status == "up") ? SAI_PORT_OPER_STATUS_UP : SAI_PORT_OPER_STATUS_DOWN; - updateLagOperStatus(l, status); - if (!learn_mode.empty() && (l.m_learn_mode != learn_mode)) { if (l.m_bridge_port_id != SAI_NULL_OBJECT_ID) @@ -2967,15 +2958,8 @@ void PortsOrch::doLagTask(Consumer &consumer) } } } - if (!status) - { - it++; - continue; - } - else - { - it = consumer.m_toSync.erase(it); - } + + it = consumer.m_toSync.erase(it); } else if (op == DEL_COMMAND) { @@ -3398,44 +3382,8 @@ bool PortsOrch::addBridgePort(Port &port) { SWSS_LOG_ENTER(); - if (port.m_rif_id) - { - SWSS_LOG_ERROR("Adding router interface %s as bridge port is not allowed", port.m_alias.c_str()); - return false; - } if (port.m_bridge_port_id != SAI_NULL_OBJECT_ID) { - /* If the port is being added to the first VLAN, - * set bridge port admin status to UP. - * This can happen if the port was just removed from - * last VLAN and fdb flush is still in progress. - */ - if (port.m_vlan_members.empty()) - { - if (port.m_fdb_count > 0) - { - return false; - } - sai_attribute_t attr; - attr.id = SAI_BRIDGE_PORT_ATTR_ADMIN_STATE; - attr.value.booldata = true; - - sai_status_t status = sai_bridge_api->set_bridge_port_attribute(port.m_bridge_port_id, &attr); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to set bridge port %s admin status to UP, rv:%d", - port.m_alias.c_str(), status); - return false; - } - port.m_bridge_port_admin_state = true; - m_portList[port.m_alias] = port; - if (!setHostIntfsStripTag(port, SAI_HOSTIF_VLAN_TAG_KEEP)) - { - SWSS_LOG_ERROR("Failed to set %s for hostif of port %s", - hostif_vlan_tag[SAI_HOSTIF_VLAN_TAG_KEEP], port.m_alias.c_str()); - return false; - } - } return true; } @@ -3508,13 +3456,6 @@ bool PortsOrch::addBridgePort(Port &port) port.m_alias.c_str(), status); return false; } - if (!setPortPvid(port, 0)) - { - SWSS_LOG_ERROR("Failed to set pvid for port %s, rv:%d", - port.m_alias.c_str(), status); - return false; - } - port.m_bridge_port_admin_state = true; if (!setHostIntfsStripTag(port, SAI_HOSTIF_VLAN_TAG_KEEP)) { @@ -3528,7 +3469,6 @@ bool PortsOrch::addBridgePort(Port &port) return true; } - bool PortsOrch::removeBridgePort(Port &port) { SWSS_LOG_ENTER(); @@ -3537,33 +3477,23 @@ bool PortsOrch::removeBridgePort(Port &port) { return true; } - if (port.m_bridge_port_admin_state) - { - /* Set bridge port admin status to DOWN */ - sai_attribute_t attr; - attr.id = SAI_BRIDGE_PORT_ATTR_ADMIN_STATE; - attr.value.booldata = false; - - sai_status_t status = sai_bridge_api->set_bridge_port_attribute(port.m_bridge_port_id, &attr); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to set bridge port %s admin status to DOWN, rv:%d", - port.m_alias.c_str(), status); - return false; - } - port.m_bridge_port_admin_state = false; - m_portList[port.m_alias] = port; + /* Set bridge port admin status to DOWN */ + sai_attribute_t attr; + attr.id = SAI_BRIDGE_PORT_ATTR_ADMIN_STATE; + attr.value.booldata = false; - if (!setHostIntfsStripTag(port, SAI_HOSTIF_VLAN_TAG_STRIP)) - { - SWSS_LOG_ERROR("Failed to set %s for hostif of port %s", - hostif_vlan_tag[SAI_HOSTIF_VLAN_TAG_STRIP], port.m_alias.c_str()); - return false; - } + sai_status_t status = sai_bridge_api->set_bridge_port_attribute(port.m_bridge_port_id, &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to set bridge port %s admin status to DOWN, rv:%d", + port.m_alias.c_str(), status); + return false; } - if (port.m_fdb_count != 0) { - //SWSS_LOG_NOTICE("Port still has fdb entries, will not remove bridge port for %s", port.m_alias.c_str()); + if (!setHostIntfsStripTag(port, SAI_HOSTIF_VLAN_TAG_STRIP)) + { + SWSS_LOG_ERROR("Failed to set %s for hostif of port %s", + hostif_vlan_tag[SAI_HOSTIF_VLAN_TAG_STRIP], port.m_alias.c_str()); return false; } @@ -3572,7 +3502,7 @@ bool PortsOrch::removeBridgePort(Port &port) SWSS_LOG_INFO("Flush FDB entries for port %s", port.m_alias.c_str()); /* Remove bridge port */ - sai_status_t status = sai_bridge_api->remove_bridge_port(port.m_bridge_port_id); + status = sai_bridge_api->remove_bridge_port(port.m_bridge_port_id); if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to remove bridge port %s from default 1Q bridge, rv:%d", @@ -3631,17 +3561,7 @@ bool PortsOrch::addVlan(string vlan_alias) sai_attribute_t attr; attr.id = SAI_VLAN_ATTR_VLAN_ID; attr.value.u16 = vlan_id; - sai_status_t status = SAI_STATUS_SUCCESS; - - /* Do not create default VLAN. It is already created by default */ - if (vlan_id != m_defaultVlan_Id) - { - status = sai_vlan_api->create_vlan(&vlan_oid, gSwitchId, 1, &attr); - } - else - { - vlan_oid = m_defaultVlan_ObjId; /* use the default VLAN object id instead */ - } + sai_status_t status = sai_vlan_api->create_vlan(&vlan_oid, gSwitchId, 1, &attr); if (status != SAI_STATUS_SUCCESS) { @@ -3664,7 +3584,6 @@ bool PortsOrch::addVlan(string vlan_alias) bool PortsOrch::removeVlan(Port vlan) { SWSS_LOG_ENTER(); - if (m_port_ref_count[vlan.m_alias] > 0) { SWSS_LOG_ERROR("Failed to remove ref count %d VLAN %s", @@ -3672,12 +3591,6 @@ bool PortsOrch::removeVlan(Port vlan) vlan.m_alias.c_str()); return false; } - /* If there are still fdb entries associated with the VLAN, - return false for retry */ - if (vlan.m_fdb_count > 0) - { - return false; - } /* Vlan removing is not allowed when the VLAN still has members */ if (vlan.m_members.size() > 0) @@ -3694,16 +3607,12 @@ bool PortsOrch::removeVlan(Port vlan) return false; } - /* Do not delete default VLAN from driver, but clear internal state */ - if (vlan.m_vlan_info.vlan_id != m_defaultVlan_Id) + sai_status_t status = sai_vlan_api->remove_vlan(vlan.m_vlan_info.vlan_oid); + if (status != SAI_STATUS_SUCCESS) { - sai_status_t status = sai_vlan_api->remove_vlan(vlan.m_vlan_info.vlan_oid); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to remove VLAN %s vid:%hu", - vlan.m_alias.c_str(), vlan.m_vlan_info.vlan_id); - return false; - } + SWSS_LOG_ERROR("Failed to remove VLAN %s vid:%hu", + vlan.m_alias.c_str(), vlan.m_vlan_info.vlan_id); + return false; } removeAclTableGroup(vlan); @@ -3781,11 +3690,10 @@ bool PortsOrch::addVlanMember(Port &vlan, Port &port, string &tagging_mode) } /* a physical port may join multiple vlans */ - VlanMemberEntry vme = {vlan.m_alias, vlan_member_id, sai_tagging_mode}; + VlanMemberEntry vme = {vlan_member_id, sai_tagging_mode}; port.m_vlan_members[vlan.m_vlan_info.vlan_id] = vme; m_portList[port.m_alias] = port; vlan.m_members.insert(port.m_alias); - m_portList[vlan.m_alias] = vlan; VlanMemberUpdate update = { vlan, port, true }; @@ -3829,7 +3737,6 @@ bool PortsOrch::removeVlanMember(Port &vlan, Port &port) m_portList[port.m_alias] = port; vlan.m_members.erase(port.m_alias); - m_portList[vlan.m_alias] = vlan; VlanMemberUpdate update = { vlan, port, false }; @@ -3850,17 +3757,6 @@ bool PortsOrch::addLag(string lag_alias) { SWSS_LOG_ENTER(); - auto lagport = m_portList.find(lag_alias); - if (lagport != m_portList.end()) - { - if ((m_portList[lag_alias].m_bridge_port_id != SAI_NULL_OBJECT_ID) && - (m_portList[lag_alias].m_vlan_members.empty())) - { - return false; - } - return true; - } - sai_object_id_t lag_id; sai_status_t status = sai_lag_api->create_lag(&lag_id, gSwitchId, 0, NULL); @@ -3878,15 +3774,14 @@ bool PortsOrch::addLag(string lag_alias) m_portList[lag_alias] = lag; m_port_ref_count[lag_alias] = 0; - /* Add lag name map to counter table */ + PortUpdate update = { lag, true }; + notify(SUBJECT_TYPE_PORT_CHANGE, static_cast(&update)); + FieldValueTuple tuple(lag_alias, sai_serialize_object_id(lag_id)); vector fields; fields.push_back(tuple); m_counterLagTable->set("", fields); - PortUpdate update = { lag, true }; - notify(SUBJECT_TYPE_PORT_CHANGE, static_cast(&update)); - return true; } @@ -3913,10 +3808,6 @@ bool PortsOrch::removeLag(Port lag) SWSS_LOG_ERROR("Failed to remove LAG %s, it is still in VLAN", lag.m_alias.c_str()); return false; } - if (lag.m_bridge_port_id != SAI_NULL_OBJECT_ID) - { - return false; - } sai_status_t status = sai_lag_api->remove_lag(lag.m_lag_id); if (status != SAI_STATUS_SUCCESS) @@ -3925,7 +3816,6 @@ bool PortsOrch::removeLag(Port lag) return false; } - SWSS_LOG_NOTICE("Remove LAG %s lid:%" PRIx64, lag.m_alias.c_str(), lag.m_lag_id); m_portList.erase(lag.m_alias); @@ -4355,7 +4245,6 @@ void PortsOrch::updatePortOperStatus(Port &port, sai_port_oper_status_t status) } bool isUp = status == SAI_PORT_OPER_STATUS_UP; - if (port.m_type == Port::PHY) { if (!setHostIntfsOperStatus(port, isUp)) @@ -4380,20 +4269,6 @@ void PortsOrch::updatePortOperStatus(Port &port, sai_port_oper_status_t status) notify(SUBJECT_TYPE_PORT_OPER_STATE_CHANGE, static_cast(&update)); } -void PortsOrch::updateLagOperStatus(Port &port, sai_port_oper_status_t status) -{ - if (status == port.m_oper_status) - { - return ; - } - SWSS_LOG_NOTICE("Port %s oper state set from %s to %s", - port.m_alias.c_str(), oper_status_strings.at(port.m_oper_status).c_str(), - oper_status_strings.at(status).c_str()); - - port.m_oper_status = status; - m_portList[port.m_alias] = port; -} - /* * sync up orchagent with libsai/ASIC for port state. * diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 6f78930291..d4184ca150 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -98,7 +98,6 @@ class PortsOrch : public Orch, public Subject bool setHostIntfsOperStatus(const Port& port, bool up) const; void updateDbPortOperStatus(const Port& port, sai_port_oper_status_t status) const; - void updateDbVlanOperStatus(const Port& port, string status) const; bool createBindAclTableGroup(sai_object_id_t port_oid, sai_object_id_t acl_table_oid, @@ -146,7 +145,6 @@ class PortsOrch : public Orch, public Subject unique_ptr
m_counterLagTable; unique_ptr
m_portTable; unique_ptr
m_gearboxTable; - unique_ptr
m_vlanTable; unique_ptr
m_queueTable; unique_ptr
m_queuePortTable; unique_ptr
m_queueIndexTable; @@ -156,7 +154,6 @@ class PortsOrch : public Orch, public Subject unique_ptr
m_pgIndexTable; unique_ptr m_flexCounterTable; unique_ptr m_flexCounterGroupTable; - unique_ptr notifications; std::string getQueueWatermarkFlexCounterTableKey(std::string s); std::string getPriorityGroupWatermarkFlexCounterTableKey(std::string s); @@ -175,8 +172,7 @@ class PortsOrch : public Orch, public Subject Port m_cpuPort; // TODO: Add Bridge/Vlan class sai_object_id_t m_default1QBridge; - sai_object_id_t m_defaultVlan_ObjId; - sai_vlan_id_t m_defaultVlan_Id; + sai_vlan_id_t m_defaultVlan; typedef enum { @@ -206,13 +202,6 @@ class PortsOrch : public Orch, public Subject map m_portList; unordered_map m_portOidToIndex; - /* mapping from SAI object ID to Name for faster - retrieval of Port/VLAN from object ID for events - - coming from SAI - */ - unordered_map m_portOidToName; - map m_port_ref_count; unordered_set m_pendingPortSet; @@ -294,7 +283,6 @@ class PortsOrch : public Orch, public Subject bool setPortSerdesAttribute(sai_object_id_t port_id, sai_attr_id_t attr_id, vector &serdes_val); - void updateLagOperStatus(Port &port, sai_port_oper_status_t status); bool getSaiAclBindPointType(Port::Type type, sai_acl_bind_point_type_t &sai_acl_bind_type); void initGearbox(); From 41a4a1893b7c0d3a3ac2461d4ebc97bb24583f56 Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Wed, 23 Dec 2020 17:22:32 -0800 Subject: [PATCH 17/21] updated as per review comments --- orchagent/fdborch.cpp | 80 ++++++++++++++++++++++++----------------- orchagent/port.h | 3 +- orchagent/portsorch.cpp | 1 - orchagent/portsorch.h | 3 +- 4 files changed, 49 insertions(+), 38 deletions(-) diff --git a/orchagent/fdborch.cpp b/orchagent/fdborch.cpp index 1679fa8cb1..63478e7dd6 100644 --- a/orchagent/fdborch.cpp +++ b/orchagent/fdborch.cpp @@ -199,6 +199,7 @@ void FdbOrch::update(sai_fdb_event_t type, } update.add = true; + update.entry.port_name = update.port.m_alias; update.type = "dynamic"; storeFdbEntryState(update); @@ -226,11 +227,11 @@ void FdbOrch::update(sai_fdb_event_t type, if (existing_entry->second.bridge_port_id != bridge_port_id) { - SWSS_LOG_NOTICE("FdbOrch AGE notification: Stale aging event received for mac-bv_id %s-0x%lx with bp=0x%lx existing bp=0x%lx", update.entry.mac.to_string().c_str(), entry->bv_id, bridge_port_id, existing_entry->second.bridge_port_id); + SWSS_LOG_INFO("FdbOrch AGE notification: Stale aging event received for mac-bv_id %s-0x%lx with bp=0x%lx existing bp=0x%lx", update.entry.mac.to_string().c_str(), entry->bv_id, bridge_port_id, existing_entry->second.bridge_port_id); // We need to get the port for bridge-port in existing fdb if (!m_portsOrch->getPortByBridgePortId(existing_entry->second.bridge_port_id, update.port)) { - SWSS_LOG_NOTICE("FdbOrch AGE notification: Failed to get port by bridge port ID 0x%lx", existing_entry->second.bridge_port_id); + SWSS_LOG_INFO("FdbOrch AGE notification: Failed to get port by bridge port ID 0x%lx", existing_entry->second.bridge_port_id); } // dont return, let it delete just to bring SONiC and SAI in sync // return; @@ -832,11 +833,24 @@ void FdbOrch::updateVlanMember(const VlanMemberUpdate& update) string port_name = update.member.m_alias; auto fdb_list = std::move(saved_fdb_entries[port_name]); saved_fdb_entries[port_name].clear(); - for (const auto& fdb: fdb_list) + if(!fdb_list.empty()) { - // try to insert an FDB entry. If the FDB entry is not ready to be inserted yet, - // it would be added back to the saved_fdb_entries structure by addFDBEntry() - (void)addFdbEntry(fdb.entry, port_name, fdb.fdbData); + for (const auto& fdb: fdb_list) + { + // try to insert an FDB entry. If the FDB entry is not ready to be inserted yet, + // it would be added back to the saved_fdb_entries structure by addFDBEntry() + if(fdb.vlanId == update.vlan.m_vlan_info.vlan_id) + { + FdbEntry entry; + entry.mac = fdb.mac; + entry.bv_id = update.vlan.m_vlan_info.vlan_oid; + (void)addFdbEntry(entry, port_name, fdb.fdbData); + } + else + { + saved_fdb_entries[port_name].push_back(fdb); + } + } } } @@ -847,7 +861,7 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, Port port; SWSS_LOG_ENTER(); - SWSS_LOG_NOTICE("mac=%s bv_id=0x%lx port_name=%s type=%s origin=%d", + SWSS_LOG_INFO("mac=%s bv_id=0x%lx port_name=%s type=%s origin=%d", entry.mac.to_string().c_str(), entry.bv_id, port_name.c_str(), fdbData.type.c_str(), fdbData.origin); @@ -886,7 +900,7 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, FdbOrigin oldOrigin = FDB_ORIGIN_INVALID ; bool macUpdate = false; auto it = m_entries.find(entry); - if(it != m_entries.end()) + if (it != m_entries.end()) { /* get existing port and type */ oldType = it->second.type; @@ -898,18 +912,18 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, 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)) { /* Duplicate Mac */ - SWSS_LOG_NOTICE("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 is duplicate", entry.mac.to_string().c_str(), vlan.m_alias.c_str(), port_name.c_str(), fdbData.type.c_str(), fdbData.origin); return true; } - else if(fdbData.origin != oldOrigin) + else if (fdbData.origin != oldOrigin) { /* Mac origin has changed */ - if((oldType == "static") && (oldOrigin == FDB_ORIGIN_PROVISIONED)) + if ((oldType == "static") && (oldOrigin == FDB_ORIGIN_PROVISIONED)) { /* old mac was static and provisioned, it can not be changed by Remote Mac */ SWSS_LOG_NOTICE("Already existing static MAC:%s in Vlan:%d. " @@ -920,20 +934,20 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, return true; } - else if((oldType == "static") && (oldOrigin == + else if ((oldType == "static") && (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED) && (fdbData.type == "dynamic")) { /* old mac was static and received from remote, it can not be changed by dynamic locally provisioned Mac */ - SWSS_LOG_NOTICE("Already existing static MAC:%s in Vlan:%d " + SWSS_LOG_INFO("Already existing static MAC:%s in Vlan:%d " "from Peer:%s. Now same is provisioned as dynamic; " "Provisioned dynamic mac is ignored", entry.mac.to_string().c_str(), vlan.m_vlan_info.vlan_id, it->second.remote_ip.c_str()); return true; } - else if(oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED) + else if (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED) { - if((oldType == "static") && (fdbData.type == "static")) + if ((oldType == "static") && (fdbData.type == "static")) { SWSS_LOG_WARN("You have just overwritten existing static MAC:%s " "in Vlan:%d from Peer:%s, " @@ -982,11 +996,11 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, attr.value.oid = port.m_bridge_port_id; attrs.push_back(attr); - if(fdbData.origin == FDB_ORIGIN_VXLAN_ADVERTIZED) + if (fdbData.origin == FDB_ORIGIN_VXLAN_ADVERTIZED) { IpAddress remote = IpAddress(fdbData.remote_ip); sai_ip_address_t ipaddr; - if(remote.isV4()) + if (remote.isV4()) { ipaddr.addr_family = SAI_IP_ADDR_FAMILY_IPV4; ipaddr.addr.ip4 = remote.getV4Addr(); @@ -1000,7 +1014,7 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, attr.value.ipaddr = ipaddr; attrs.push_back(attr); } - else if(macUpdate + else if (macUpdate && (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED) && (fdbData.origin != oldOrigin)) { @@ -1015,9 +1029,9 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, attrs.push_back(attr); } - if(macUpdate && (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED)) + if (macUpdate && (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED)) { - if((fdbData.origin != oldOrigin) + if ((fdbData.origin != oldOrigin) || ((oldType == "dynamic") && (oldType != fdbData.type))) { attr.id = SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE; @@ -1027,13 +1041,13 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, } - if(macUpdate) + if (macUpdate) { - SWSS_LOG_NOTICE("MAC-Update FDB %s in %s on from-%s:to-%s from-%s:to-%s origin-%d-to-%d", + SWSS_LOG_INFO("MAC-Update FDB %s in %s on from-%s:to-%s from-%s:to-%s origin-%d-to-%d", entry.mac.to_string().c_str(), vlan.m_alias.c_str(), oldPort.m_alias.c_str(), port_name.c_str(), oldType.c_str(), fdbData.type.c_str(), oldOrigin, fdbData.origin); - for(auto itr : attrs) + for (auto itr : attrs) { status = sai_fdb_api->set_fdb_entry_attribute(&fdb_entry, &itr); if (status != SAI_STATUS_SUCCESS) @@ -1046,7 +1060,7 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, } else { - SWSS_LOG_NOTICE("MAC-Create %s FDB %s in %s on %s", fdbData.type.c_str(), entry.mac.to_string().c_str(), vlan.m_alias.c_str(), port_name.c_str()); + SWSS_LOG_INFO("MAC-Create %s FDB %s in %s on %s", fdbData.type.c_str(), entry.mac.to_string().c_str(), vlan.m_alias.c_str(), port_name.c_str()); status = sai_fdb_api->create_fdb_entry(&fdb_entry, (uint32_t)attrs.size(), attrs.data()); if (status != SAI_STATUS_SUCCESS) @@ -1086,7 +1100,7 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, m_fdbStateTable.del(key); } - if(!macUpdate) + if (!macUpdate) { gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_FDB_ENTRY); } @@ -1109,7 +1123,7 @@ bool FdbOrch::removeFdbEntry(const FdbEntry& entry, FdbOrigin origin) SWSS_LOG_ENTER(); - SWSS_LOG_NOTICE("FdbOrch RemoveFDBEntry: mac=%s bv_id=0x%lx origin %d", entry.mac.to_string().c_str(), entry.bv_id, origin); + SWSS_LOG_INFO("FdbOrch RemoveFDBEntry: mac=%s bv_id=0x%lx origin %d", entry.mac.to_string().c_str(), entry.bv_id, origin); if (!m_portsOrch->getPort(entry.bv_id, vlan)) { @@ -1118,7 +1132,7 @@ bool FdbOrch::removeFdbEntry(const FdbEntry& entry, FdbOrigin origin) } auto it= m_entries.find(entry); - if(it == m_entries.end()) + if (it == m_entries.end()) { SWSS_LOG_INFO("FdbOrch RemoveFDBEntry: FDB entry isn't found. mac=%s bv_id=0x%lx", entry.mac.to_string().c_str(), entry.bv_id); @@ -1134,7 +1148,7 @@ bool FdbOrch::removeFdbEntry(const FdbEntry& entry, FdbOrigin origin) return false; } - if(fdbData.origin != origin) + if (fdbData.origin != origin) { /* When mac is moved from remote to local * BGP will delete the mac from vxlan_fdb_table @@ -1207,14 +1221,14 @@ void FdbOrch::deleteFdbEntryFromSavedFDB(const MacAddress &mac, for (auto& itr: saved_fdb_entries) { - if(portName.empty() || (portName == itr.first)) + if (portName.empty() || (portName == itr.first)) { auto iter = saved_fdb_entries[itr.first].begin(); while(iter != saved_fdb_entries[itr.first].end()) { if (*iter == entry) { - if(iter->fdbData.origin == origin) + if (iter->fdbData.origin == origin) { SWSS_LOG_NOTICE("FDB entry found in saved fdb. deleting..." "mac=%s vlan_id=0x%x origin:%d port:%s", @@ -1237,7 +1251,7 @@ void FdbOrch::deleteFdbEntryFromSavedFDB(const MacAddress &mac, iter++; } } - if(found) + if (found) break; } } @@ -1247,7 +1261,7 @@ void FdbOrch::notifyTunnelOrch(Port& port) { VxlanTunnelOrch* tunnel_orch = gDirectory.get(); - if(port.m_type != Port::TUNNEL) + if (port.m_type != Port::TUNNEL) return; tunnel_orch->deleteTunnelPort(port); diff --git a/orchagent/port.h b/orchagent/port.h index 0db155428f..fb0b8b6434 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -26,7 +26,6 @@ namespace swss { struct VlanMemberEntry { - std::string alias; sai_object_id_t vlan_member_id; sai_vlan_tagging_mode_t vlan_mode; }; @@ -110,7 +109,7 @@ class Port uint32_t m_nat_zone_id = 0; uint32_t m_vnid = VNID_NONE; uint32_t m_fdb_count = 0; - uint32_t m_up_member_count = 0; + uint32_t m_up_member_count = 0; /* * Following two bit vectors are used to lock diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index f4a8831627..ed371267cf 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -46,7 +46,6 @@ extern BufferOrch *gBufferOrch; extern FdbOrch *gFdbOrch; extern Directory gDirectory; - #define VLAN_PREFIX "Vlan" #define DEFAULT_VLAN_ID 1 #define MAX_VALID_VLAN_ID 4094 diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index d4184ca150..9c683b9cd4 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -92,7 +92,6 @@ class PortsOrch : public Orch, public Subject void decreasePortRefCount(const string &alias); bool getPortByBridgePortId(sai_object_id_t bridge_port_id, Port &port); void setPort(string alias, Port port); - void erasePort(string alias); void getCpuPort(Port &port); bool getVlanByVlanId(sai_vlan_id_t vlan_id, Port &vlan); @@ -172,7 +171,7 @@ class PortsOrch : public Orch, public Subject Port m_cpuPort; // TODO: Add Bridge/Vlan class sai_object_id_t m_default1QBridge; - sai_vlan_id_t m_defaultVlan; + sai_object_id_t m_defaultVlan; typedef enum { From f17b20e72b34899af0f9f8b6d27887f0d6e8b11d Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Sun, 27 Dec 2020 16:35:01 -0800 Subject: [PATCH 18/21] Update fdborch.cpp --- orchagent/fdborch.cpp | 41 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/orchagent/fdborch.cpp b/orchagent/fdborch.cpp index 63478e7dd6..9c8d81538d 100644 --- a/orchagent/fdborch.cpp +++ b/orchagent/fdborch.cpp @@ -201,6 +201,10 @@ void FdbOrch::update(sai_fdb_event_t type, update.add = true; update.entry.port_name = update.port.m_alias; update.type = "dynamic"; + update.port.m_fdb_count++; + m_portsOrch->setPort(update.port.m_alias, update.port); + vlan.m_fdb_count++; + m_portsOrch->setPort(vlan.m_alias, vlan); storeFdbEntryState(update); notify(SUBJECT_TYPE_FDB_CHANGE, &update); @@ -279,7 +283,16 @@ void FdbOrch::update(sai_fdb_event_t type, } update.add = false; - + if (!update.port.m_alias.empty()) + { + update.port.m_fdb_count--; + m_portsOrch->setPort(update.port.m_alias, update.port); + } + if (!vlan.m_alias.empty()) + { + vlan.m_fdb_count--; + m_portsOrch->setPort(vlan.m_alias, vlan); + } storeFdbEntryState(update); notify(SUBJECT_TYPE_FDB_CHANGE, &update); @@ -313,7 +326,13 @@ void FdbOrch::update(sai_fdb_event_t type, } update.add = true; - + if (!port_old.m_alias.empty()) + { + port_old.m_fdb_count--; + m_portsOrch->setPort(port_old.m_alias, port_old); + } + update.port.m_fdb_count++; + m_portsOrch->setPort(update.port.m_alias, update.port); storeFdbEntryState(update); notify(SUBJECT_TYPE_FDB_CHANGE, &update); @@ -1057,6 +1076,13 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, return false; } } + if (oldPort.m_bridge_port_id != port.m_bridge_port_id) + { + oldPort.m_fdb_count--; + m_portsOrch->setPort(oldPort.m_alias, oldPort); + port.m_fdb_count++; + m_portsOrch->setPort(port.m_alias, port); + } } else { @@ -1070,6 +1096,10 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, vlan.m_alias.c_str(), port_name.c_str(), status); return false; //FIXME: it should be based on status. Some could be retried, some not } + port.m_fdb_count++; + m_portsOrch->setPort(port.m_alias, port); + vlan.m_fdb_count++; + m_portsOrch->setPort(vlan.m_alias, vlan); } FdbData storeFdbData = fdbData; @@ -1185,6 +1215,10 @@ bool FdbOrch::removeFdbEntry(const FdbEntry& entry, FdbOrigin origin) SWSS_LOG_NOTICE("Removed mac=%s bv_id=0x%lx port:%s", entry.mac.to_string().c_str(), entry.bv_id, port.m_alias.c_str()); + port.m_fdb_count--; + m_portsOrch->setPort(port.m_alias, port); + vlan.m_fdb_count--; + m_portsOrch->setPort(vlan.m_alias, vlan); (void)m_entries.erase(entry); // Remove in StateDb @@ -1261,7 +1295,8 @@ void FdbOrch::notifyTunnelOrch(Port& port) { VxlanTunnelOrch* tunnel_orch = gDirectory.get(); - if (port.m_type != Port::TUNNEL) + if((port.m_type != Port::TUNNEL) || + (port.m_fdb_count != 0)) return; tunnel_orch->deleteTunnelPort(port); From 3ceb819e92ee6d8b466a4d842c0b79a4a4729a8f Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Tue, 29 Dec 2020 13:24:35 -0800 Subject: [PATCH 19/21] updated as per latest review comments updated as per latest review comments --- orchagent/fdborch.cpp | 8 ++++---- orchagent/portsorch.cpp | 1 - orchagent/portsorch.h | 4 +--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/orchagent/fdborch.cpp b/orchagent/fdborch.cpp index 9c8d81538d..0d06faddea 100644 --- a/orchagent/fdborch.cpp +++ b/orchagent/fdborch.cpp @@ -1185,7 +1185,7 @@ bool FdbOrch::removeFdbEntry(const FdbEntry& entry, FdbOrigin origin) * but we should not delete this mac here since now * mac in orchagent represents locally learnt */ - SWSS_LOG_NOTICE("FdbOrch RemoveFDBEntry: mac=%s fdb origin is different; found_origin:%d delete_origin:%d", + SWSS_LOG_INFO("FdbOrch RemoveFDBEntry: mac=%s fdb origin is different; found_origin:%d delete_origin:%d", entry.mac.to_string().c_str(), fdbData.origin, origin); /* We may still have the mac in saved-fdb probably due to unavailability @@ -1212,7 +1212,7 @@ bool FdbOrch::removeFdbEntry(const FdbEntry& entry, FdbOrigin origin) return true; //FIXME: it should be based on status. Some could be retried. some not } - SWSS_LOG_NOTICE("Removed mac=%s bv_id=0x%lx port:%s", + SWSS_LOG_INFO("Removed mac=%s bv_id=0x%lx port:%s", entry.mac.to_string().c_str(), entry.bv_id, port.m_alias.c_str()); port.m_fdb_count--; @@ -1264,7 +1264,7 @@ void FdbOrch::deleteFdbEntryFromSavedFDB(const MacAddress &mac, { if (iter->fdbData.origin == origin) { - SWSS_LOG_NOTICE("FDB entry found in saved fdb. deleting..." + SWSS_LOG_INFO("FDB entry found in saved fdb. deleting..." "mac=%s vlan_id=0x%x origin:%d port:%s", mac.to_string().c_str(), vlanId, origin, itr.first.c_str()); @@ -1275,7 +1275,7 @@ void FdbOrch::deleteFdbEntryFromSavedFDB(const MacAddress &mac, } else { - SWSS_LOG_NOTICE("FDB entry found in saved fdb, but Origin is " + SWSS_LOG_INFO("FDB entry found in saved fdb, but Origin is " "different mac=%s vlan_id=0x%x reqOrigin:%d " "foundOrigin:%d port:%s, IGNORED", mac.to_string().c_str(), vlanId, origin, diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index ed371267cf..0ed35ecb41 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1882,7 +1882,6 @@ bool PortsOrch::initPort(const string &alias, const int index, const set &l /* Add port to port list */ m_portList[alias] = p; m_portOidToIndex[id] = index; - m_port_ref_count[alias] = 0; /* Add port name map to counter table */ FieldValueTuple tuple(p.m_alias, sai_serialize_object_id(p.m_port_id)); diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 9c683b9cd4..dfd5ab875d 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -9,7 +9,6 @@ #include "observer.h" #include "macaddress.h" #include "producertable.h" -#include "notificationproducer.h" #include "flex_counter_manager.h" #include "gearboxutils.h" #include "saihelper.h" @@ -171,7 +170,7 @@ class PortsOrch : public Orch, public Subject Port m_cpuPort; // TODO: Add Bridge/Vlan class sai_object_id_t m_default1QBridge; - sai_object_id_t m_defaultVlan; + sai_object_id_t m_defaultVlan; typedef enum { @@ -200,7 +199,6 @@ class PortsOrch : public Orch, public Subject map, tuple> m_lanesAliasSpeedMap; map m_portList; unordered_map m_portOidToIndex; - map m_port_ref_count; unordered_set m_pendingPortSet; From df6b9e7598fb08523726428992c5bdea8e95c925 Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Tue, 29 Dec 2020 14:36:12 -0800 Subject: [PATCH 20/21] Update portsorch.cpp --- orchagent/portsorch.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 0ed35ecb41..5dd76f70b9 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1881,6 +1881,7 @@ bool PortsOrch::initPort(const string &alias, const int index, const set &l /* Add port to port list */ m_portList[alias] = p; + m_port_ref_count[alias] = 0; m_portOidToIndex[id] = index; /* Add port name map to counter table */ From 798d5fea6e07a774697bb04d127f54f4fc1ba876 Mon Sep 17 00:00:00 2001 From: anilkpan <47642449+anilkpan@users.noreply.github.com> Date: Thu, 31 Dec 2020 11:54:27 -0800 Subject: [PATCH 21/21] fix vs test --- tests/test_fdbsync.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_fdbsync.py b/tests/test_fdbsync.py index ee86c8616d..8d85e8458e 100644 --- a/tests/test_fdbsync.py +++ b/tests/test_fdbsync.py @@ -58,7 +58,7 @@ def set_admin_status(dvs, interface, status): local_intf = 'Ethernet8' local_intf2 = 'Ethernet12' tunnel_name_nvo = 'nvo' -app_fdb_name = '_VXLAN_FDB_TABLE:' +app_fdb_name = 'VXLAN_FDB_TABLE:' tunnel_remote_imet = '00:00:00:00:00:00' tunnel_remote_imet_type = 'permanent' app_imet_name = 'VXLAN_REMOTE_VNI_TABLE:'