Skip to content

Commit b426b2b

Browse files
stephenxsstephenxs
authored
Support checking capabilities of the mirror (sonic-net#3934)
What I did Support checking the capabilities of ingress/egress mirror before setting it to SAI. Switch orchagent fetches the mirror capabilities from SAI during initialization and exposes to field PORT_INGRESS_MIRROR_CAPABLE and PORT_EGRESS_MIRROR_CAPABLE in STATE_DB table SWITCH_CAPABILITY. Mirror orchagent check whether the direction is supported before calling the corresponding SAI API. Why I did it This is to avoid SAI returning error on the platforms that the mirror is not supported on a direction. It will collect SAI SDK dump on receiving a SAI error message, which is unnecessary.
1 parent 25647cd commit b426b2b

File tree

10 files changed

+161
-5
lines changed

10 files changed

+161
-5
lines changed

‎orchagent/mirrororch.cpp‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,14 @@ MirrorEntry::MirrorEntry(const string& platform) :
7777
}
7878

7979
MirrorOrch::MirrorOrch(TableConnector stateDbConnector, TableConnector confDbConnector,
80-
PortsOrch *portOrch, RouteOrch *routeOrch, NeighOrch *neighOrch, FdbOrch *fdbOrch, PolicerOrch *policerOrch) :
80+
PortsOrch *portOrch, RouteOrch *routeOrch, NeighOrch *neighOrch, FdbOrch *fdbOrch, PolicerOrch *policerOrch, SwitchOrch *switchOrch) :
8181
Orch(confDbConnector.first, confDbConnector.second),
8282
m_portsOrch(portOrch),
8383
m_routeOrch(routeOrch),
8484
m_neighOrch(neighOrch),
8585
m_fdbOrch(fdbOrch),
8686
m_policerOrch(policerOrch),
87+
m_switchOrch(switchOrch),
8788
m_mirrorTable(stateDbConnector.first, stateDbConnector.second)
8889
{
8990
sai_status_t status;
@@ -812,6 +813,18 @@ bool MirrorOrch::setUnsetPortMirror(Port port,
812813
bool set,
813814
sai_object_id_t sessionId)
814815
{
816+
// Check if the mirror direction is supported by the ASIC
817+
if (ingress && !m_switchOrch->isPortIngressMirrorSupported())
818+
{
819+
SWSS_LOG_ERROR("Port ingress mirror is not supported by the ASIC");
820+
return false;
821+
}
822+
if (!ingress && !m_switchOrch->isPortEgressMirrorSupported())
823+
{
824+
SWSS_LOG_ERROR("Port egress mirror is not supported by the ASIC");
825+
return false;
826+
}
827+
815828
sai_status_t status;
816829
sai_attribute_t port_attr;
817830
port_attr.id = ingress ? SAI_PORT_ATTR_INGRESS_MIRROR_SESSION:

‎orchagent/mirrororch.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ class MirrorOrch : public Orch, public Observer, public Subject
7777
{
7878
public:
7979
MirrorOrch(TableConnector appDbConnector, TableConnector confDbConnector,
80-
PortsOrch *portOrch, RouteOrch *routeOrch, NeighOrch *neighOrch, FdbOrch *fdbOrch, PolicerOrch *policerOrch);
80+
PortsOrch *portOrch, RouteOrch *routeOrch, NeighOrch *neighOrch, FdbOrch *fdbOrch, PolicerOrch *policerOrch, SwitchOrch *switchOrch);
8181

8282
bool bake() override;
8383
void update(SubjectType, void *);
@@ -95,6 +95,7 @@ class MirrorOrch : public Orch, public Observer, public Subject
9595
NeighOrch *m_neighOrch;
9696
FdbOrch *m_fdbOrch;
9797
PolicerOrch *m_policerOrch;
98+
SwitchOrch *m_switchOrch;
9899
// Maximum number of traffic classes starting at 0, thus queue can be 0 - m_maxNumTC-1
99100
uint8_t m_maxNumTC;
100101

‎orchagent/orchdaemon.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ bool OrchDaemon::init()
377377

378378
TableConnector stateDbMirrorSession(m_stateDb, STATE_MIRROR_SESSION_TABLE_NAME);
379379
TableConnector confDbMirrorSession(m_configDb, CFG_MIRROR_SESSION_TABLE_NAME);
380-
gMirrorOrch = new MirrorOrch(stateDbMirrorSession, confDbMirrorSession, gPortsOrch, gRouteOrch, gNeighOrch, gFdbOrch, gPolicerOrch);
380+
gMirrorOrch = new MirrorOrch(stateDbMirrorSession, confDbMirrorSession, gPortsOrch, gRouteOrch, gNeighOrch, gFdbOrch, gPolicerOrch, gSwitchOrch);
381381

382382
TableConnector confDbAclTable(m_configDb, CFG_ACL_TABLE_TABLE_NAME);
383383
TableConnector confDbAclTableType(m_configDb, CFG_ACL_TABLE_TYPE_TABLE_NAME);

‎orchagent/switchorch.cpp‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ SwitchOrch::SwitchOrch(DBConnector *db, vector<TableConnector>& connectors, Tabl
164164
initSensorsTable();
165165
querySwitchTpidCapability();
166166
querySwitchPortEgressSampleCapability();
167+
querySwitchPortMirrorCapability();
167168
querySwitchHashDefaults();
168169
setSwitchIcmpOffloadCapability();
169170

@@ -1859,6 +1860,63 @@ void SwitchOrch::querySwitchPortEgressSampleCapability()
18591860
set_switch_capability(fvVector);
18601861
}
18611862

1863+
void SwitchOrch::querySwitchPortMirrorCapability()
1864+
{
1865+
vector<FieldValueTuple> fvVector;
1866+
sai_status_t status = SAI_STATUS_SUCCESS;
1867+
sai_attr_capability_t capability;
1868+
1869+
// Check if SAI is capable of handling Port ingress mirror session
1870+
status = sai_query_attribute_capability(gSwitchId, SAI_OBJECT_TYPE_PORT,
1871+
SAI_PORT_ATTR_INGRESS_MIRROR_SESSION, &capability);
1872+
if (status != SAI_STATUS_SUCCESS)
1873+
{
1874+
SWSS_LOG_WARN("Could not query port ingress mirror capability %d", status);
1875+
fvVector.emplace_back(SWITCH_CAPABILITY_TABLE_PORT_INGRESS_MIRROR_CAPABLE, "true");
1876+
m_portIngressMirrorSupported = true;
1877+
}
1878+
else
1879+
{
1880+
if (capability.set_implemented)
1881+
{
1882+
fvVector.emplace_back(SWITCH_CAPABILITY_TABLE_PORT_INGRESS_MIRROR_CAPABLE, "true");
1883+
m_portIngressMirrorSupported = true;
1884+
}
1885+
else
1886+
{
1887+
fvVector.emplace_back(SWITCH_CAPABILITY_TABLE_PORT_INGRESS_MIRROR_CAPABLE, "false");
1888+
m_portIngressMirrorSupported = false;
1889+
}
1890+
SWSS_LOG_NOTICE("port ingress mirror capability %d", capability.set_implemented);
1891+
}
1892+
1893+
// Check if SAI is capable of handling Port egress mirror session
1894+
status = sai_query_attribute_capability(gSwitchId, SAI_OBJECT_TYPE_PORT,
1895+
SAI_PORT_ATTR_EGRESS_MIRROR_SESSION, &capability);
1896+
if (status != SAI_STATUS_SUCCESS)
1897+
{
1898+
SWSS_LOG_WARN("Could not query port egress mirror capability %d", status);
1899+
fvVector.emplace_back(SWITCH_CAPABILITY_TABLE_PORT_EGRESS_MIRROR_CAPABLE, "true");
1900+
m_portEgressMirrorSupported = true;
1901+
}
1902+
else
1903+
{
1904+
if (capability.set_implemented)
1905+
{
1906+
fvVector.emplace_back(SWITCH_CAPABILITY_TABLE_PORT_EGRESS_MIRROR_CAPABLE, "true");
1907+
m_portEgressMirrorSupported = true;
1908+
}
1909+
else
1910+
{
1911+
fvVector.emplace_back(SWITCH_CAPABILITY_TABLE_PORT_EGRESS_MIRROR_CAPABLE, "false");
1912+
m_portEgressMirrorSupported = false;
1913+
}
1914+
SWSS_LOG_NOTICE("port egress mirror capability %d", capability.set_implemented);
1915+
}
1916+
1917+
set_switch_capability(fvVector);
1918+
}
1919+
18621920
void SwitchOrch::querySwitchTpidCapability()
18631921
{
18641922
SWSS_LOG_ENTER();

‎orchagent/switchorch.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
#define SWITCH_CAPABILITY_TABLE_REG_FATAL_ASIC_SDK_HEALTH_CATEGORY "REG_FATAL_ASIC_SDK_HEALTH_CATEGORY"
2727
#define SWITCH_CAPABILITY_TABLE_REG_WARNING_ASIC_SDK_HEALTH_CATEGORY "REG_WARNING_ASIC_SDK_HEALTH_CATEGORY"
2828
#define SWITCH_CAPABILITY_TABLE_REG_NOTICE_ASIC_SDK_HEALTH_CATEGORY "REG_NOTICE_ASIC_SDK_HEALTH_CATEGORY"
29+
#define SWITCH_CAPABILITY_TABLE_PORT_INGRESS_MIRROR_CAPABLE "PORT_INGRESS_MIRROR_CAPABLE"
30+
#define SWITCH_CAPABILITY_TABLE_PORT_EGRESS_MIRROR_CAPABLE "PORT_EGRESS_MIRROR_CAPABLE"
2931

3032
#define SWITCH_STAT_COUNTER_FLEX_COUNTER_GROUP "SWITCH_STAT_COUNTER"
3133

@@ -78,6 +80,10 @@ class SwitchOrch : public Orch
7880
// Statistics
7981
void generateSwitchCounterIdList();
8082

83+
// Mirror capability interface for MirrorOrch
84+
bool isPortIngressMirrorSupported() const { return m_portIngressMirrorSupported; }
85+
bool isPortEgressMirrorSupported() const { return m_portEgressMirrorSupported; }
86+
8187
private:
8288
void doTask(Consumer &consumer);
8389
void doTask(swss::SelectableTimer &timer);
@@ -89,6 +95,7 @@ class SwitchOrch : public Orch
8995
void initSensorsTable();
9096
void querySwitchTpidCapability();
9197
void querySwitchPortEgressSampleCapability();
98+
void querySwitchPortMirrorCapability();
9299

93100
// Statistics
94101
void generateSwitchCounterNameMap() const;
@@ -148,6 +155,10 @@ class SwitchOrch : public Orch
148155
bool m_orderedEcmpEnable = false;
149156
bool m_PfcDlrInitEnable = false;
150157

158+
// Port mirror capabilities
159+
bool m_portIngressMirrorSupported = false;
160+
bool m_portEgressMirrorSupported = false;
161+
151162
// ASIC SDK health event
152163
std::shared_ptr<swss::DBConnector> m_stateDbForNotification = nullptr;
153164
std::shared_ptr<swss::Table> m_asicSdkHealthEventTable = nullptr;

‎tests/mock_tests/Makefile.am‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ tests_SOURCES = aclorch_ut.cpp \
7979
mock_dash_orch_test.cpp \
8080
zmq_orch_ut.cpp \
8181
mock_saihelper.cpp \
82+
mirrororch_ut.cpp \
8283
$(top_srcdir)/warmrestart/warmRestartHelper.cpp \
8384
$(top_srcdir)/lib/gearboxutils.cpp \
8485
$(top_srcdir)/lib/subintf.cpp \

‎tests/mock_tests/aclorch_ut.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ namespace aclorch_test
449449

450450
ASSERT_EQ(gMirrorOrch, nullptr);
451451
gMirrorOrch = new MirrorOrch(stateDbMirrorSession, confDbMirrorSession,
452-
gPortsOrch, gRouteOrch, gNeighOrch, gFdbOrch, policer_orch);
452+
gPortsOrch, gRouteOrch, gNeighOrch, gFdbOrch, policer_orch, gSwitchOrch);
453453

454454
auto consumer = unique_ptr<Consumer>(new Consumer(
455455
new swss::ConsumerStateTable(m_app_db.get(), APP_PORT_TABLE_NAME, 1, 1), gPortsOrch, APP_PORT_TABLE_NAME));

‎tests/mock_tests/mirrororch_ut.cpp‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Make selected privates visible for unit testing
2+
#define private public
3+
#include "directory.h"
4+
#undef private
5+
6+
#define protected public
7+
#include "orch.h"
8+
#undef protected
9+
10+
#define private public
11+
#include "switchorch.h"
12+
#undef private
13+
14+
#include "portsorch.h"
15+
#define private public
16+
#include "mirrororch.h"
17+
#undef private
18+
#include "mock_orch_test.h"
19+
20+
namespace mirrororch_test
21+
{
22+
using namespace mock_orch_test;
23+
24+
class MirrorOrchTest : public MockOrchTest
25+
{
26+
};
27+
28+
TEST_F(MirrorOrchTest, RejectsIngressWhenUnsupported)
29+
{
30+
// Ensure environment initialized by MockOrchTest
31+
ASSERT_NE(gSwitchOrch, nullptr);
32+
ASSERT_NE(gMirrorOrch, nullptr);
33+
34+
// Force ingress unsupported and egress supported
35+
gSwitchOrch->m_portIngressMirrorSupported = false;
36+
gSwitchOrch->m_portEgressMirrorSupported = true;
37+
38+
Port dummyPort; // Unused due to early return
39+
auto ret = gMirrorOrch->setUnsetPortMirror(dummyPort, /*ingress*/ true, /*set*/ true, /*sessionId*/ SAI_NULL_OBJECT_ID);
40+
ASSERT_FALSE(ret);
41+
}
42+
43+
TEST_F(MirrorOrchTest, RejectsEgressWhenUnsupported)
44+
{
45+
ASSERT_NE(gSwitchOrch, nullptr);
46+
ASSERT_NE(gMirrorOrch, nullptr);
47+
48+
// Force egress unsupported and ingress supported
49+
gSwitchOrch->m_portIngressMirrorSupported = true;
50+
gSwitchOrch->m_portEgressMirrorSupported = false;
51+
52+
Port dummyPort; // Unused due to early return
53+
auto ret = gMirrorOrch->setUnsetPortMirror(dummyPort, /*ingress*/ false, /*set*/ true, /*sessionId*/ SAI_NULL_OBJECT_ID);
54+
ASSERT_FALSE(ret);
55+
}
56+
}
57+

‎tests/mock_tests/mock_orch_test.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ void MockOrchTest::SetUp()
234234

235235
TableConnector stateDbMirrorSession(m_state_db.get(), STATE_MIRROR_SESSION_TABLE_NAME);
236236
TableConnector confDbMirrorSession(m_config_db.get(), CFG_MIRROR_SESSION_TABLE_NAME);
237-
gMirrorOrch = new MirrorOrch(stateDbMirrorSession, confDbMirrorSession, gPortsOrch, gRouteOrch, gNeighOrch, gFdbOrch, gPolicerOrch);
237+
gMirrorOrch = new MirrorOrch(stateDbMirrorSession, confDbMirrorSession, gPortsOrch, gRouteOrch, gNeighOrch, gFdbOrch, gPolicerOrch, gSwitchOrch);
238238
gDirectory.set(gMirrorOrch);
239239
ut_orch_list.push_back((Orch **)&gMirrorOrch);
240240
global_orch_list.insert((Orch **)&gMirrorOrch);

‎tests/mock_tests/switchorch_ut.cpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "mock_orchagent_main.h"
99
#include "mock_table.h"
1010
#include "mock_response_publisher.h"
11+
#include "switchorch.h"
1112

1213
extern void on_switch_asic_sdk_health_event(sai_object_id_t switch_id,
1314
sai_switch_asic_sdk_health_severity_t severity,
@@ -287,6 +288,13 @@ namespace switchorch_test
287288
ASSERT_EQ(value, "true");
288289
gSwitchOrch->m_switchTable.hget("switch", SWITCH_CAPABILITY_TABLE_REG_NOTICE_ASIC_SDK_HEALTH_CATEGORY, value);
289290
ASSERT_EQ(value, "true");
291+
292+
// Test that mirror capabilities are also queried and stored
293+
// The actual values depend on the SAI implementation, but we can verify the entries exist
294+
bool ingress_exists = gSwitchOrch->m_switchTable.hget("switch", SWITCH_CAPABILITY_TABLE_PORT_INGRESS_MIRROR_CAPABLE, value);
295+
ASSERT_TRUE(ingress_exists);
296+
bool egress_exists = gSwitchOrch->m_switchTable.hget("switch", SWITCH_CAPABILITY_TABLE_PORT_EGRESS_MIRROR_CAPABLE, value);
297+
ASSERT_TRUE(egress_exists);
290298
}
291299

292300
TEST_F(SwitchOrchTest, SwitchOrchTestCheckCapabilityUnsupported)
@@ -307,6 +315,13 @@ namespace switchorch_test
307315
gSwitchOrch->m_switchTable.hget("switch", SWITCH_CAPABILITY_TABLE_REG_NOTICE_ASIC_SDK_HEALTH_CATEGORY, value);
308316
ASSERT_EQ(value, "false");
309317

318+
// Test that mirror capabilities are also queried and stored
319+
// The actual values depend on the SAI implementation, but we can verify the entries exist
320+
bool ingress_exists = gSwitchOrch->m_switchTable.hget("switch", SWITCH_CAPABILITY_TABLE_PORT_INGRESS_MIRROR_CAPABLE, value);
321+
ASSERT_TRUE(ingress_exists);
322+
bool egress_exists = gSwitchOrch->m_switchTable.hget("switch", SWITCH_CAPABILITY_TABLE_PORT_EGRESS_MIRROR_CAPABLE, value);
323+
ASSERT_TRUE(egress_exists);
324+
310325
// case: unsupported severity. To satisfy coverage.
311326
vector<string> ts;
312327
std::deque<KeyOpFieldsValuesTuple> entries;

0 commit comments

Comments
 (0)