diff --git a/orchagent/mirrororch.cpp b/orchagent/mirrororch.cpp index ac6857ff4a..fbb241d1b3 100644 --- a/orchagent/mirrororch.cpp +++ b/orchagent/mirrororch.cpp @@ -10,10 +10,12 @@ #include "swssnet.h" #include "converter.h" #include "mirrororch.h" +#include "tokenize.h" #define MIRROR_SESSION_STATUS "status" #define MIRROR_SESSION_STATUS_ACTIVE "active" #define MIRROR_SESSION_STATUS_INACTIVE "inactive" +#define MIRROR_SESSION_NEXT_HOP_IP "next_hop_ip" #define MIRROR_SESSION_SRC_IP "src_ip" #define MIRROR_SESSION_DST_IP "dst_ip" #define MIRROR_SESSION_GRE_TYPE "gre_type" @@ -23,7 +25,7 @@ #define MIRROR_SESSION_DST_MAC_ADDRESS "dst_mac" #define MIRROR_SESSION_MONITOR_PORT "monitor_port" #define MIRROR_SESSION_ROUTE_PREFIX "route_prefix" -#define MIRROR_SESSION_VLAN_HEADER_VALID "vlan_header_valid" +#define MIRROR_SESSION_VLAN_ID "vlan_id" #define MIRROR_SESSION_POLICER "policer" #define MIRROR_SESSION_DEFAULT_VLAN_PRI 0 @@ -58,6 +60,7 @@ MirrorEntry::MirrorEntry(const string& platform) : } nexthopInfo.prefix = IpPrefix("0.0.0.0/0"); + nexthopInfo.nexthop = IpAddress("0.0.0.0"); } MirrorOrch::MirrorOrch(TableConnector stateDbConnector, TableConnector confDbConnector, @@ -75,6 +78,74 @@ MirrorOrch::MirrorOrch(TableConnector stateDbConnector, TableConnector confDbCon m_fdbOrch->attach(this); } +bool MirrorOrch::bake() +{ + SWSS_LOG_ENTER(); + + // Freeze the route update during orchagent restoration + m_freeze = true; + + deque entries; + vector keys; + m_mirrorTable.getKeys(keys); + for (const auto &key : keys) + { + vector tuples; + m_mirrorTable.get(key, tuples); + + bool active = false; + string monitor_port; + string next_hop_ip; + + for (const auto &tuple : tuples) + { + if (fvField(tuple) == MIRROR_SESSION_STATUS) + { + active = fvValue(tuple) == MIRROR_SESSION_STATUS_ACTIVE; + } + else if (fvField(tuple) == MIRROR_SESSION_MONITOR_PORT) + { + monitor_port = fvValue(tuple); + } + else if (fvField(tuple) == MIRROR_SESSION_NEXT_HOP_IP) + { + next_hop_ip = fvValue(tuple); + } + } + + if (!active) + { + continue; + } + + SWSS_LOG_NOTICE("Found mirror session %s active before warm reboot", + key.c_str()); + + // Recover saved active session's monitor port + m_recoverySessionMap.emplace( + key, monitor_port + state_db_key_delimiter + next_hop_ip); + } + + return Orch::bake(); +} + +bool MirrorOrch::postBake() +{ + SWSS_LOG_ENTER(); + + SWSS_LOG_NOTICE("Start MirrorOrch post-baking"); + + // Unfreeze the route update + m_freeze = false; + + Orch::doTask(); + + // Clean up the recovery cache + m_recoverySessionMap.clear(); + + return Orch::postBake(); +} + void MirrorOrch::update(SubjectType type, void *cntx) { SWSS_LOG_ENTER(); @@ -320,10 +391,11 @@ void MirrorOrch::setSessionState(const string& name, const MirrorEntry& session, { SWSS_LOG_ENTER(); - SWSS_LOG_INFO("Setting mirroring sessions %s state\n", name.c_str()); + SWSS_LOG_INFO("Update mirroring sessions %s state", name.c_str()); vector fvVector; string value; + if (attr.empty() || attr == MIRROR_SESSION_STATUS) { value = session.status ? MIRROR_SESSION_STATUS_ACTIVE : MIRROR_SESSION_STATUS_INACTIVE; @@ -332,8 +404,9 @@ void MirrorOrch::setSessionState(const string& name, const MirrorEntry& session, if (attr.empty() || attr == MIRROR_SESSION_MONITOR_PORT) { - value = sai_serialize_object_id(session.neighborInfo.portId); - fvVector.emplace_back(MIRROR_SESSION_MONITOR_PORT, value); + Port port; + m_portsOrch->getPort(session.neighborInfo.portId, port); + fvVector.emplace_back(MIRROR_SESSION_MONITOR_PORT, port.m_alias); } if (attr.empty() || attr == MIRROR_SESSION_DST_MAC_ADDRESS) @@ -348,10 +421,16 @@ void MirrorOrch::setSessionState(const string& name, const MirrorEntry& session, fvVector.emplace_back(MIRROR_SESSION_ROUTE_PREFIX, value); } - if (attr.empty() || attr == MIRROR_SESSION_VLAN_HEADER_VALID) + if (attr.empty() || attr == MIRROR_SESSION_VLAN_ID) + { + value = to_string(session.neighborInfo.port.m_vlan_info.vlan_id); + fvVector.emplace_back(MIRROR_SESSION_VLAN_ID, value); + } + + if (attr.empty() || attr == MIRROR_SESSION_NEXT_HOP_IP) { - value = to_string(session.neighborInfo.port.m_type == Port::VLAN); - fvVector.emplace_back(MIRROR_SESSION_VLAN_HEADER_VALID, value); + value = session.nexthopInfo.nexthop.to_string(); + fvVector.emplace_back(MIRROR_SESSION_NEXT_HOP_IP, value); } m_mirrorTable.set(name, fvVector); @@ -396,32 +475,68 @@ bool MirrorOrch::getNeighborInfo(const string& name, MirrorEntry& session) return false; } - // Get the firt member of the LAG - Port member; - const auto& first_member_alias = *session.neighborInfo.port.m_members.begin(); - m_portsOrch->getPort(first_member_alias, member); + // Recover the LAG member monitor port picked before warm reboot + // to minimalize the data plane changes across warm reboot. + if (m_recoverySessionMap.find(name) != m_recoverySessionMap.end()) + { + string alias = tokenize(m_recoverySessionMap[name], + state_db_key_delimiter, 1)[0]; + Port member; + m_portsOrch->getPort(alias, member); + + SWSS_LOG_NOTICE("Recover mirror session %s with LAG member port %s", + name.c_str(), alias.c_str()); + session.neighborInfo.portId = member.m_port_id; + } + else + { + // Get the firt member of the LAG + Port member; + string first_member_alias = *session.neighborInfo.port.m_members.begin(); + m_portsOrch->getPort(first_member_alias, member); + + session.neighborInfo.portId = member.m_port_id; + } - session.neighborInfo.portId = member.m_port_id; return true; } case Port::VLAN: { SWSS_LOG_NOTICE("Get mirror session destination IP neighbor VLAN %d", session.neighborInfo.port.m_vlan_info.vlan_id); - Port member; - if (!m_fdbOrch->getPort(session.neighborInfo.mac, session.neighborInfo.port.m_vlan_info.vlan_id, member)) + + // Recover the VLAN member monitor port picked before warm reboot + // since the FDB entries are not yet learned on the hardware + if (m_recoverySessionMap.find(name) != m_recoverySessionMap.end()) { - SWSS_LOG_NOTICE("Waiting to get FDB entry MAC %s under VLAN %s", - session.neighborInfo.mac.to_string().c_str(), - session.neighborInfo.port.m_alias.c_str()); - return false; + string alias = tokenize(m_recoverySessionMap[name], + state_db_key_delimiter, 1)[0]; + Port member; + m_portsOrch->getPort(alias, member); + + SWSS_LOG_NOTICE("Recover mirror session %s with VLAN member port %s", + name.c_str(), alias.c_str()); + session.neighborInfo.portId = member.m_port_id; } else { - // Update monitor port - session.neighborInfo.portId = member.m_port_id; - return true; + Port member; + if (!m_fdbOrch->getPort(session.neighborInfo.mac, + session.neighborInfo.port.m_vlan_info.vlan_id, member)) + { + SWSS_LOG_NOTICE("Waiting to get FDB entry MAC %s under VLAN %s", + session.neighborInfo.mac.to_string().c_str(), + session.neighborInfo.port.m_alias.c_str()); + return false; + } + else + { + // Update monitor port + session.neighborInfo.portId = member.m_port_id; + } } + + return true; } default: { @@ -741,7 +856,7 @@ bool MirrorOrch::updateSessionType(const string& name, MirrorEntry& session) SWSS_LOG_NOTICE("Update mirror session %s VLAN to %s", name.c_str(), session.neighborInfo.port.m_alias.c_str()); - setSessionState(name, session, MIRROR_SESSION_VLAN_HEADER_VALID); + setSessionState(name, session, MIRROR_SESSION_VLAN_ID); return true; } @@ -782,7 +897,35 @@ void MirrorOrch::updateNextHop(const NextHopUpdate& update) if (update.nexthopGroup != IpAddresses()) { - session.nexthopInfo.nexthop = *update.nexthopGroup.getIpAddresses().begin(); + SWSS_LOG_NOTICE(" next hop IPs: %s", update.nexthopGroup.to_string().c_str()); + + // Recover the session based on the state database information + if (m_recoverySessionMap.find(name) != m_recoverySessionMap.end()) + { + IpAddress nexthop = IpAddress(tokenize(m_recoverySessionMap[name], + state_db_key_delimiter, 1)[1]); + + // Check if recovered next hop IP is within the update's next hop IPs + if (update.nexthopGroup.getIpAddresses().count(nexthop)) + { + SWSS_LOG_NOTICE("Recover mirror session %s with next hop %s", + name.c_str(), nexthop.to_string().c_str()); + session.nexthopInfo.nexthop = nexthop; + } + else + { + // Correct the next hop IP + SWSS_LOG_NOTICE("Correct mirror session %s next hop from %s to %s", + name.c_str(), session.nexthopInfo.nexthop.to_string().c_str(), + nexthop.to_string().c_str()); + session.nexthopInfo.nexthop = *update.nexthopGroup.getIpAddresses().begin(); + } + } + else + { + // Pick the first one from the next hop group + session.nexthopInfo.nexthop = *update.nexthopGroup.getIpAddresses().begin(); + } } else { @@ -968,6 +1111,11 @@ void MirrorOrch::doTask(Consumer& consumer) { SWSS_LOG_ENTER(); + if (m_freeze) + { + return; + } + if (!gPortsOrch->allPortsReady()) { return; @@ -991,7 +1139,7 @@ void MirrorOrch::doTask(Consumer& consumer) } else { - SWSS_LOG_ERROR("Unknown operation type %s\n", op.c_str()); + SWSS_LOG_ERROR("Unknown operation type %s", op.c_str()); } consumer.m_toSync.erase(it++); diff --git a/orchagent/mirrororch.h b/orchagent/mirrororch.h index 765b24cb5f..4a5801df6f 100644 --- a/orchagent/mirrororch.h +++ b/orchagent/mirrororch.h @@ -69,6 +69,8 @@ class MirrorOrch : public Orch, public Observer, public Subject MirrorOrch(TableConnector appDbConnector, TableConnector confDbConnector, PortsOrch *portOrch, RouteOrch *routeOrch, NeighOrch *neighOrch, FdbOrch *fdbOrch, PolicerOrch *policerOrch); + bool bake() override; + bool postBake() override; void update(SubjectType, void *); bool sessionExists(const string&); bool getSessionStatus(const string&, bool&); @@ -86,6 +88,10 @@ class MirrorOrch : public Orch, public Observer, public Subject Table m_mirrorTable; MirrorTable m_syncdMirrors; + // session_name -> VLAN | monitor_port_alias | next_hop_ip + map m_recoverySessionMap; + + bool m_freeze = false; void createEntry(const string&, const vector&); void deleteEntry(const string&); diff --git a/orchagent/orch.cpp b/orchagent/orch.cpp index 89f924dccc..a6dce7efbb 100644 --- a/orchagent/orch.cpp +++ b/orchagent/orch.cpp @@ -27,7 +27,7 @@ Orch::Orch(DBConnector *db, const string tableName, int pri) Orch::Orch(DBConnector *db, const vector &tableNames) { - for(auto it : tableNames) + for (auto it : tableNames) { addConsumer(db, it, default_orch_pri); } @@ -35,7 +35,7 @@ Orch::Orch(DBConnector *db, const vector &tableNames) Orch::Orch(DBConnector *db, const vector &tableNames_with_pri) { - for(const auto& it : tableNames_with_pri) + for (const auto& it : tableNames_with_pri) { addConsumer(db, it.first, it.second); } @@ -60,7 +60,7 @@ Orch::~Orch() vector Orch::getSelectables() { vector selectables; - for(auto& it : m_consumerMap) + for (auto& it : m_consumerMap) { selectables.push_back(it.second.get()); } @@ -240,7 +240,7 @@ bool Orch::bake() { SWSS_LOG_ENTER(); - for(auto &it : m_consumerMap) + for (auto &it : m_consumerMap) { string executorName = it.first; auto executor = it.second; @@ -257,6 +257,13 @@ bool Orch::bake() return true; } +bool Orch::postBake() +{ + SWSS_LOG_ENTER(); + + return true; +} + /* - Validates reference has proper format which is [table_name:object_name] - validates table_name exists @@ -365,7 +372,7 @@ ref_resolve_status Orch::resolveFieldRefValue( void Orch::doTask() { - for(auto &it : m_consumerMap) + for (auto &it : m_consumerMap) { it.second->drain(); } @@ -373,7 +380,7 @@ void Orch::doTask() void Orch::dumpPendingTasks(vector &ts) { - for(auto &it : m_consumerMap) + for (auto &it : m_consumerMap) { Consumer* consumer = dynamic_cast(it.second.get()); if (consumer == NULL) diff --git a/orchagent/orch.h b/orchagent/orch.h index 0298770f2f..8b2a130823 100644 --- a/orchagent/orch.h +++ b/orchagent/orch.h @@ -183,6 +183,8 @@ class Orch // Prepare for warm start if Redis contains valid input data // otherwise fallback to cold start virtual bool bake(); + // Clean up the state set in bake() + virtual bool postBake(); /* Iterate all consumers in m_consumerMap and run doTask(Consumer) */ virtual void doTask(); diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 2c4f2126b2..400d24f85a 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -479,14 +479,22 @@ bool OrchDaemon::warmRestoreAndSyncUp() * Fourth iteration: Drain remaining data that are out of order like LAG_MEMBER_TABLE and * VLAN_MEMBER_TABLE since they were checked before LAG_TABLE and VLAN_TABLE within gPortsOrch. */ + for (auto it = 0; it < 4; it++) { + SWSS_LOG_DEBUG("The current iteration is %d", it); + for (Orch *o : m_orchList) { o->doTask(); } } + for (Orch *o : m_orchList) + { + o->postBake(); + } + /* * At this point, all the pre-existing data should have been processed properly, and * orchagent should be in exact same state of pre-shutdown. diff --git a/orchagent/port.h b/orchagent/port.h index 34b72aa8c2..ad264a1db5 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -30,8 +30,8 @@ typedef std::map vlan_members_t; struct VlanInfo { - sai_object_id_t vlan_oid; - sai_vlan_id_t vlan_id; + sai_object_id_t vlan_oid = 0; + sai_vlan_id_t vlan_id = 0; }; class Port diff --git a/tests/test_mirror.py b/tests/test_mirror.py index c3da54d43d..80db250755 100644 --- a/tests/test_mirror.py +++ b/tests/test_mirror.py @@ -140,7 +140,7 @@ def test_MirrorAddRemove(self, dvs, testlog): # add route to mirror destination via 10.0.0.1 self.add_route(dvs, "2.2.2.2", "10.0.0.1") assert self.get_mirror_session_state(session)["status"] == "active" - assert self.get_mirror_session_state(session)["monitor_port"] == dvs.asicdb.portnamemap["Ethernet16"] + assert self.get_mirror_session_state(session)["monitor_port"] == "Ethernet16" assert self.get_mirror_session_state(session)["dst_mac"] == "02:04:06:08:10:12" assert self.get_mirror_session_state(session)["route_prefix"] == "2.2.2.2/32"