Skip to content

Commit

Permalink
[mirrororch] Fix a bug where ERSPAN used LAG as the destination port …
Browse files Browse the repository at this point in the history 
…and linked down the first LAG member.

[portsorch] Add a function to return a linked-up member of an input LAG.

- What I did
When a destination port belongs to a LAG in an ERSPAN session,
detect the LAG member that is linked down and switch the destination port to a LAG member that is linked up.

- Why I did it
Current ERSPAN is unaware of the LAG member port linking up or down.

- How to verify it
The community tests and manual tests pass.

Signed-off-by: sdn@inspur.com
  • Loading branch information
@inspurSDN
inspurSDN authored and waynelai committed Dec 7, 2022
1 parent dca78d8 commit 6f1d24a
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 19 deletions.
124 changes: 105 additions & 19 deletions orchagent/mirrororch.cpp
100644 → 100755
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,12 @@ void MirrorOrch::update(SubjectType type, void *cntx)
updateVlanMember(*update);
break;
}
case SUBJECT_TYPE_PORT_OPER_STATE_CHANGE:
{
PortOperStateUpdate *update = static_cast<PortOperStateUpdate *>(cntx);
updateLagMemberOperChange(*update);
break;
}
default:
// Received update in which we are not interested
// Ignore it
Expand Down Expand Up @@ -590,11 +596,11 @@ void MirrorOrch::setSessionState(const string& name, const MirrorEntry& session,
SWSS_LOG_ERROR("Failed to get recirc port for mirror session %s", name.c_str());
return;
}
}
else
{
m_portsOrch->getPort(session.neighborInfo.portId, port);
}
}
else
{
m_portsOrch->getPort(session.neighborInfo.portId, port);
}
fvVector.emplace_back(MIRROR_SESSION_MONITOR_PORT, port.m_alias);
}

Expand Down Expand Up @@ -692,11 +698,13 @@ bool MirrorOrch::getNeighborInfo(const string& name, MirrorEntry& session)
}
else
{
// Get the first member of the LAG
// Get the first linked up member of the LAG
Port member;
string first_member_alias = *session.neighborInfo.port.m_members.begin();
m_portsOrch->getPort(first_member_alias, member);

if (!m_portsOrch->getUpLagMember(session.neighborInfo.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;
}

Expand Down Expand Up @@ -1438,6 +1446,7 @@ void MirrorOrch::updateLagMember(const LagMemberUpdate& update)
{
SWSS_LOG_ENTER();

Port update_lag = update.lag;
for (auto it = m_syncdMirrors.begin(); it != m_syncdMirrors.end(); it++)
{
const auto& name = it->first;
Expand All @@ -1450,7 +1459,7 @@ void MirrorOrch::updateLagMember(const LagMemberUpdate& update)
// if the above condition matches then set/unset mirror configuration to new member port.
if (session.status &&
!session.src_port.empty() &&
session.src_port.find(update.lag.m_alias.c_str()) != std::string::npos &&
session.src_port.find(update_lag.m_alias.c_str()) != std::string::npos &&
!checkPortExistsInSrcPortList(update.member.m_alias, session.src_port))
{
if (session.direction == MIRROR_RX_DIRECTION || session.direction == MIRROR_BOTH_DIRECTION)
Expand All @@ -1467,7 +1476,7 @@ void MirrorOrch::updateLagMember(const LagMemberUpdate& update)
// 1) the neighbor is LAG
// 2) the neighbor LAG matches the update LAG
if (session.neighborInfo.port.m_type != Port::LAG ||
session.neighborInfo.port != update.lag)
session.neighborInfo.port != update_lag)
{
continue;
}
Expand All @@ -1479,19 +1488,21 @@ void MirrorOrch::updateLagMember(const LagMemberUpdate& update)
// session is already activated, no further action is needed.
if (!session.status)
{
assert(!update.lag.m_members.empty());
const string& member_name = *update.lag.m_members.begin();
assert(!update_lag.m_members.empty());
Port member;
m_portsOrch->getPort(member_name, member);

if (!m_portsOrch->getUpLagMember(update_lag, member))
{
const string& member_name = *update_lag.m_members.begin();
m_portsOrch->getPort(member_name, member);
}
session.neighborInfo.portId = member.m_port_id;
activateSession(name, session);
}
}
else
{
// If LAG is empty, deactivate session
if (update.lag.m_members.empty())
if (update_lag.m_members.empty())
{
if (session.status)
{
Expand All @@ -1502,10 +1513,12 @@ void MirrorOrch::updateLagMember(const LagMemberUpdate& update)
// Switch to a new member of the LAG
else
{
const string& member_name = *update.lag.m_members.begin();
Port member;
m_portsOrch->getPort(member_name, member);

if (!m_portsOrch->getUpLagMember(update_lag, member))
{
const string& member_name = *update_lag.m_members.begin();
m_portsOrch->getPort(member_name, member);
}
session.neighborInfo.portId = member.m_port_id;
// The destination MAC remains the same
updateSessionDstPort(name, session);
Expand Down Expand Up @@ -1546,6 +1559,79 @@ void MirrorOrch::updateVlanMember(const VlanMemberUpdate& update)
}
}

/* Handle the case when the member of the LAG is up or down.
This function is called when SUBJECT_TYPE_PORT_OPER_STATE_CHANGE is received. */
void MirrorOrch::updateLagMemberOperChange(const PortOperStateUpdate& update)
{
SWSS_LOG_ENTER();

/* Check the following two conditions:
1) The update port is PHY port.
2) The update port belongs to a LAG. */
Port update_lag;
if (update.port.m_type != Port::PHY ||
!m_portsOrch->getPort(update.port.m_lag_id, update_lag))
{
return;
}

for (auto it = m_syncdMirrors.begin(); it != m_syncdMirrors.end(); it++)
{
const auto& name = it->first;
auto& session = it->second;

/* Check the following two conditions:
1) the neighbor is LAG.
2) the neighbor LAG matches the update LAG. */
if (session.neighborInfo.port.m_type != Port::LAG ||
session.neighborInfo.port != update_lag)
{
continue;
}

Port cur_dst_port;
if (!m_portsOrch->getPort(session.neighborInfo.portId, cur_dst_port))
{
SWSS_LOG_ERROR("Failed to locate Port/LAG %s", session.dst_port.c_str());
continue;
}

if (update.operStatus == SAI_PORT_OPER_STATUS_UP)
{
/* Activate mirror session if it was deactivated due to the reason
that previously there was no active member in the LAG. If the mirror
session is already activated, no further action is needed. */
if (!session.status)
{
session.neighborInfo.portId = update.port.m_port_id;
activateSession(name, session);
}
}
else
{
// shutdown a LAG member
if (session.status)
{
// If the shoutdown port is the current dst port, try to switch to a new member of the LAG.
if (update.port == cur_dst_port)
{
Port upPort;
if (m_portsOrch->getUpLagMember(update_lag, upPort, update.port.m_alias))
{
session.neighborInfo.portId = upPort.m_port_id;
updateSessionDstPort(name, session);
}
else
{
session.neighborInfo.portId = SAI_NULL_OBJECT_ID;
deactivateSession(name, session);
}
}
}
}
}
}

void MirrorOrch::doTask(Consumer& consumer)
{
SWSS_LOG_ENTER();
Expand Down
1 change: 1 addition & 0 deletions orchagent/mirrororch.h
100644 → 100755
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ class MirrorOrch : public Orch, public Observer, public Subject
void updateFdb(const FdbUpdate&);
void updateLagMember(const LagMemberUpdate&);
void updateVlanMember(const VlanMemberUpdate&);
void updateLagMemberOperChange(const PortOperStateUpdate&);

bool checkPortExistsInSrcPortList(const string& port, const string& srcPortList);
bool validateSrcPortList(const string& srcPort);
Expand Down
19 changes: 19 additions & 0 deletions orchagent/portsorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5785,6 +5785,25 @@ void PortsOrch::getLagMember(Port &lag, vector<Port> &portv)
}
}

/* Get the first linked up member of the LAG,
and notice that the return value from getPort() is still stale if triggered by SUBJECT_TYPE_PORT_OPER_STATE_CHANGE.
To avoid getting stale information, pass a port_alias as a filter.
*/
bool PortsOrch::getUpLagMember(Port &lag, Port &upPort, const string &aliasExclude)
{
vector<Port> portv;
getLagMember(lag, portv);
for (const auto p : portv)
{
if (p.m_oper_status == SAI_PORT_OPER_STATUS_UP && p.m_alias != aliasExclude)
{
upPort = p;
return true;
}
}
return false;
}

bool PortsOrch::addLagMember(Port &lag, Port &port, bool enableForwarding)
{
SWSS_LOG_ENTER();
Expand Down
1 change: 1 addition & 0 deletions orchagent/portsorch.h
100644 → 100755
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ class PortsOrch : public Orch, public Subject
bool removeSubPort(const string &alias);
bool updateL3VniStatus(uint16_t vlan_id, bool status);
void getLagMember(Port &lag, vector<Port> &portv);
bool getUpLagMember(Port &lag, Port &upPort, const string &aliasExclude = "");
void updateChildPortsMtu(const Port &p, const uint32_t mtu);

bool addTunnel(string tunnel,sai_object_id_t, bool learning=true);
Expand Down

0 comments on commit 6f1d24a

Please sign in to comment.