Skip to content

Commit 276ec46

Browse files
maxwindifffacebook-github-bot
maxwindiff
authored and
facebook-github-bot
committed
Add aging group to MoD config
Summary: Add aging group to MoD config so that we can have finer control over the number of MoD packets generated. Reviewed By: nivinl Differential Revision: D67998972 fbshipit-source-id: f42b5a2188adffb614348db99afeeb93fc309a54
1 parent 731fa29 commit 276ec46

9 files changed

+3712
-3530
lines changed

‎fboss/agent/ApplyThriftConfig.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -5529,7 +5529,7 @@ ThriftConfigApplier::createMirrorOnDropReport(
55295529
agingIntervalUsecs,
55305530
getLocalMacAddress().toString(),
55315531
utility::getFirstInterfaceMac(new_).toString(),
5532-
*config->eventIdToDropReasons());
5532+
*config->modEventToConfigMap());
55335533
}
55345534

55355535
std::shared_ptr<MirrorOnDropReport>

‎fboss/agent/hw/sai/switch/npu/bcm/SaiTamManager.cpp

+22-4
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ extern "C" {
2020

2121
namespace {
2222
using facebook::fboss::FbossError;
23+
using facebook::fboss::cfg::MirrorOnDropAgingGroup;
2324
using facebook::fboss::cfg::MirrorOnDropReasonAggregation;
2425

2526
// Must be less than BRCM_SAI_DNX_MOD_MAX_SUPP_AGE_TIME (776 on J3).
@@ -130,6 +131,21 @@ std::vector<sai_int32_t> getMirrorOnDropReasonsFromAggregation(
130131
return allReasons;
131132
}
132133

134+
sai_tam_event_aging_group_type_t getMirrorOnDropAgingGroupType(
135+
MirrorOnDropAgingGroup group) {
136+
switch (group) {
137+
case MirrorOnDropAgingGroup::PORT:
138+
return SAI_TAM_EVENT_AGING_GROUP_TYPE_PORT;
139+
case MirrorOnDropAgingGroup::PRIORITY_GROUP:
140+
return SAI_TAM_EVENT_AGING_GROUP_TYPE_PRIORITY_GROUP;
141+
case MirrorOnDropAgingGroup::VOQ:
142+
return SAI_TAM_EVENT_AGING_GROUP_TYPE_VOQ;
143+
case MirrorOnDropAgingGroup::GLOBAL:
144+
default:
145+
return SAI_TAM_EVENT_AGING_GROUP_TYPE_GLOBAL;
146+
}
147+
}
148+
133149
} // namespace
134150
#endif
135151

@@ -241,9 +257,9 @@ void SaiTamManager::addMirrorOnDropReport(
241257
std::vector<std::shared_ptr<SaiTamEventAgingGroup>> agingGroups;
242258
std::vector<std::shared_ptr<SaiTamEvent>> events;
243259
std::vector<sai_object_id_t> eventIds;
244-
for (const auto& [eventId, reasonAggs] :
245-
report->getEventIdToDropReasonAggregations()) {
246-
auto reasons = getMirrorOnDropReasonsFromAggregation(reasonAggs);
260+
for (const auto& [eventId, eventCfg] : report->getModEventToConfigMap()) {
261+
auto reasons = getMirrorOnDropReasonsFromAggregation(
262+
*eventCfg.dropReasonAggregations());
247263
if (reasons.empty()) {
248264
XLOG(WARN) << "No mirror on drop reasons specified for event " << eventId;
249265
continue;
@@ -254,7 +270,9 @@ void SaiTamManager::addMirrorOnDropReport(
254270
sai_uint16_t agingInterval =
255271
report->getAgingIntervalUsecs().value_or(kDefaultAgingIntervalUsecs);
256272
auto agingGroupTraits = SaiTamEventAgingGroupTraits::CreateAttributes{
257-
SAI_TAM_EVENT_AGING_GROUP_TYPE_VOQ, agingInterval};
273+
getMirrorOnDropAgingGroupType(eventCfg.agingGroup().value_or(
274+
cfg::MirrorOnDropAgingGroup::GLOBAL)),
275+
agingInterval};
258276
auto agingGroup =
259277
agingGroupStore.setObject(agingGroupTraits, agingGroupTraits);
260278
agingGroups.push_back(agingGroup);

‎fboss/agent/state/MirrorOnDropReport.cpp

+5-6
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@ MirrorOnDropReport::MirrorOnDropReport(
2323
std::optional<int32_t> agingIntervalUsecs,
2424
std::string switchMac,
2525
std::string firstInterfaceMac,
26-
std::map<int8_t, std::vector<cfg::MirrorOnDropReasonAggregation>>
27-
eventIdToDropReasons)
26+
std::map<int8_t, cfg::MirrorOnDropEventConfig> modEventToConfigMap)
2827
: ThriftStructNode<MirrorOnDropReport, state::MirrorOnDropReportFields>() {
2928
set<switch_state_tags::name>(name);
3029
set<switch_state_tags::mirrorPortId>(mirrorPortId);
@@ -40,7 +39,7 @@ MirrorOnDropReport::MirrorOnDropReport(
4039
}
4140
set<switch_state_tags::switchMac>(switchMac);
4241
set<switch_state_tags::firstInterfaceMac>(firstInterfaceMac);
43-
set<switch_state_tags::eventIdToDropReasons>(eventIdToDropReasons);
42+
set<switch_state_tags::modEventToConfigMap>(modEventToConfigMap);
4443
}
4544

4645
std::string MirrorOnDropReport::getID() const {
@@ -99,9 +98,9 @@ std::string MirrorOnDropReport::getFirstInterfaceMac() const {
9998
return get<switch_state_tags::firstInterfaceMac>()->cref();
10099
}
101100

102-
std::map<int8_t, std::vector<cfg::MirrorOnDropReasonAggregation>>
103-
MirrorOnDropReport::getEventIdToDropReasonAggregations() const {
104-
return get<switch_state_tags::eventIdToDropReasons>()->toThrift();
101+
std::map<int8_t, cfg::MirrorOnDropEventConfig>
102+
MirrorOnDropReport::getModEventToConfigMap() const {
103+
return get<switch_state_tags::modEventToConfigMap>()->toThrift();
105104
}
106105

107106
template struct ThriftStructNode<

‎fboss/agent/state/MirrorOnDropReport.h

+2-4
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,7 @@ class MirrorOnDropReport : public ThriftStructNode<
3434
std::optional<int32_t> agingIntervalUsecs,
3535
std::string switchMac,
3636
std::string firstInterfaceMac,
37-
std::map<int8_t, std::vector<cfg::MirrorOnDropReasonAggregation>>
38-
eventIdToDropReasons);
37+
std::map<int8_t, cfg::MirrorOnDropEventConfig> modEventToConfigMap);
3938

4039
std::string getID() const;
4140
PortID getMirrorPortId() const;
@@ -50,8 +49,7 @@ class MirrorOnDropReport : public ThriftStructNode<
5049
std::optional<uint32_t> getAgingIntervalUsecs() const;
5150
std::string getSwitchMac() const;
5251
std::string getFirstInterfaceMac() const;
53-
std::map<int8_t, std::vector<cfg::MirrorOnDropReasonAggregation>>
54-
getEventIdToDropReasonAggregations() const;
52+
std::map<int8_t, cfg::MirrorOnDropEventConfig> getModEventToConfigMap() const;
5553

5654
private:
5755
// Inherit the constructors required for clone()

‎fboss/agent/state/tests/MirrorOnDropReportTests.cpp

+40-9
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,17 @@ class MirrorOnDropReportTest : public ::testing::Test {
2424
cfg::SwitchConfig config_;
2525
};
2626

27+
cfg::MirrorOnDropEventConfig makeEventConfig(
28+
std::optional<cfg::MirrorOnDropAgingGroup> group,
29+
const std::vector<cfg::MirrorOnDropReasonAggregation>& reasonAggs) {
30+
cfg::MirrorOnDropEventConfig eventCfg;
31+
if (group.has_value()) {
32+
eventCfg.agingGroup() = group.value();
33+
}
34+
eventCfg.dropReasonAggregations() = reasonAggs;
35+
return eventCfg;
36+
}
37+
2738
cfg::MirrorOnDropReport makeReportCfg(const std::string& ip) {
2839
cfg::MirrorOnDropReport report;
2940
report.name() = "mod-1";
@@ -35,13 +46,17 @@ cfg::MirrorOnDropReport makeReportCfg(const std::string& ip) {
3546
report.truncateSize() = 128;
3647
report.dscp() = 0;
3748
report.agingIntervalUsecs() = 100;
38-
report.eventIdToDropReasons()[0] = {
39-
cfg::MirrorOnDropReasonAggregation::INGRESS_PACKET_PROCESSING_DISCARDS};
40-
report.eventIdToDropReasons()[1] = {
41-
cfg::MirrorOnDropReasonAggregation::INGRESS_SOURCE_CONGESTION_DISCARDS,
42-
cfg::MirrorOnDropReasonAggregation::
43-
INGRESS_DESTINATION_CONGESTION_DISCARDS,
44-
};
49+
report.modEventToConfigMap()[0] = makeEventConfig(
50+
std::nullopt,
51+
{cfg::MirrorOnDropReasonAggregation::INGRESS_PACKET_PROCESSING_DISCARDS});
52+
report.modEventToConfigMap()[1] = makeEventConfig(
53+
cfg::MirrorOnDropAgingGroup::PORT,
54+
{
55+
cfg::MirrorOnDropReasonAggregation::
56+
INGRESS_SOURCE_CONGESTION_DISCARDS,
57+
cfg::MirrorOnDropReasonAggregation::
58+
INGRESS_DESTINATION_CONGESTION_DISCARDS,
59+
});
4560
return report;
4661
}
4762

@@ -56,7 +71,15 @@ TEST_F(MirrorOnDropReportTest, CreateReportV4) {
5671
EXPECT_TRUE(report->getLocalSrcIp().isV4());
5772
EXPECT_FALSE(report->getSwitchMac().empty());
5873
EXPECT_FALSE(report->getFirstInterfaceMac().empty());
59-
EXPECT_TRUE(report->getEventIdToDropReasonAggregations().size() == 2);
74+
EXPECT_TRUE(report->getModEventToConfigMap().size() == 2);
75+
EXPECT_TRUE(
76+
report->getModEventToConfigMap()[0].agingGroup().value_or(
77+
cfg::MirrorOnDropAgingGroup::GLOBAL) ==
78+
cfg::MirrorOnDropAgingGroup::GLOBAL);
79+
EXPECT_TRUE(
80+
report->getModEventToConfigMap()[1].agingGroup().value_or(
81+
cfg::MirrorOnDropAgingGroup::GLOBAL) ==
82+
cfg::MirrorOnDropAgingGroup::PORT);
6083
}
6184

6285
TEST_F(MirrorOnDropReportTest, CreateReportV6) {
@@ -70,7 +93,15 @@ TEST_F(MirrorOnDropReportTest, CreateReportV6) {
7093
EXPECT_TRUE(report->getLocalSrcIp().isV6());
7194
EXPECT_FALSE(report->getSwitchMac().empty());
7295
EXPECT_FALSE(report->getFirstInterfaceMac().empty());
73-
EXPECT_TRUE(report->getEventIdToDropReasonAggregations().size() == 2);
96+
EXPECT_TRUE(report->getModEventToConfigMap().size() == 2);
97+
EXPECT_TRUE(
98+
report->getModEventToConfigMap()[0].agingGroup().value_or(
99+
cfg::MirrorOnDropAgingGroup::GLOBAL) ==
100+
cfg::MirrorOnDropAgingGroup::GLOBAL);
101+
EXPECT_TRUE(
102+
report->getModEventToConfigMap()[1].agingGroup().value_or(
103+
cfg::MirrorOnDropAgingGroup::GLOBAL) ==
104+
cfg::MirrorOnDropAgingGroup::PORT);
74105
}
75106

76107
} // namespace facebook::fboss

‎fboss/agent/switch_config.thrift

+30-3
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,29 @@ enum MirrorOnDropReasonAggregation {
385385
INGRESS_GLOBAL_CONGESTION_DISCARDS = 6,
386386
}
387387

388+
/**
389+
* Aging group of an event, which controls the granularity at which MOD packets
390+
* are sent:
391+
* - At most one packet per interval will be generated for GLOBAL events
392+
* - At most one packet per port pre interval will be generated for PORT events
393+
* - ...and so on
394+
*/
395+
enum MirrorOnDropAgingGroup {
396+
GLOBAL = 0,
397+
PORT = 1,
398+
PRIORITY_GROUP = 2,
399+
VOQ = 3,
400+
}
401+
402+
/**
403+
* Configuration for each event ID under a MirrorOnDropReport. An event can have its own
404+
* aging granularity setting and associated drop reasons.
405+
*/
406+
struct MirrorOnDropEventConfig {
407+
1: list<MirrorOnDropReasonAggregation> dropReasonAggregations;
408+
2: optional MirrorOnDropAgingGroup agingGroup; // defaults to 0 i.e. GLOBAL
409+
}
410+
388411
struct MirrorOnDropReport {
389412
1: string name;
390413
/*
@@ -402,10 +425,14 @@ struct MirrorOnDropReport {
402425
// Contents of the dropped packet will be truncated when mirroring.
403426
7: i16 truncateSize = 128;
404427
8: byte dscp = 0;
405-
// At most one mirrored packet will be sent per port/PG/VOQ within an interval. Granularity is not configurable as of now.
428+
// At most one mirrored packet will be sent per port/PG/VOQ within an interval.
406429
9: optional i32 agingIntervalUsecs;
407-
// Mapping from drop ID / event ID to groups of reasons.
408-
10: map<byte, list<MirrorOnDropReasonAggregation>> eventIdToDropReasons;
430+
10: map<
431+
byte,
432+
list<MirrorOnDropReasonAggregation>
433+
> eventIdToDropReasons_DEPRECATED (deprecated);
434+
// Configuration for each event ID.
435+
11: map<byte, MirrorOnDropEventConfig> modEventToConfigMap;
409436
}
410437

411438
/**

‎fboss/agent/switch_state.thrift

+2-1
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,8 @@ struct MirrorOnDropReportFields {
335335
13: map<
336336
byte,
337337
list<switch_config.MirrorOnDropReasonAggregation>
338-
> eventIdToDropReasons;
338+
> eventIdToDropReasons_DEPRECATED;
339+
14: map<byte, switch_config.MirrorOnDropEventConfig> modEventToConfigMap;
339340
}
340341

341342
struct ControlPlaneFields {

‎fboss/agent/test/agent_hw_tests/AgentMirrorOnDropTests.cpp

+44-19
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,23 @@ class AgentMirrorOnDropTest : public AgentHwTest {
116116
ecmpHelper.programRoutes(&routeUpdater, {port}, {route});
117117
}
118118

119+
cfg::MirrorOnDropEventConfig makeEventConfig(
120+
std::optional<cfg::MirrorOnDropAgingGroup> group,
121+
const std::vector<cfg::MirrorOnDropReasonAggregation>& reasonAggs) {
122+
cfg::MirrorOnDropEventConfig eventCfg;
123+
if (group.has_value()) {
124+
eventCfg.agingGroup() = group.value();
125+
}
126+
eventCfg.dropReasonAggregations() = reasonAggs;
127+
return eventCfg;
128+
}
129+
119130
void setupMirrorOnDrop(
120131
cfg::SwitchConfig* config,
121132
PortID portId,
122133
const folly::IPAddressV6& collectorIp,
123-
const std::map<int8_t, std::vector<cfg::MirrorOnDropReasonAggregation>>&
124-
eventIdToDropReasons) {
134+
const std::map<int8_t, cfg::MirrorOnDropEventConfig>&
135+
modEventToConfigMap) {
125136
cfg::MirrorOnDropReport report;
126137
report.name() = "mod-1";
127138
report.mirrorPortId() = portId;
@@ -132,7 +143,7 @@ class AgentMirrorOnDropTest : public AgentHwTest {
132143
report.truncateSize() = kTruncateSize;
133144
report.dscp() = 0;
134145
report.agingIntervalUsecs() = 100;
135-
report.eventIdToDropReasons() = eventIdToDropReasons;
146+
report.modEventToConfigMap() = modEventToConfigMap;
136147
config->mirrorOnDropReports()->push_back(report);
137148
}
138149

@@ -273,8 +284,10 @@ TEST_F(AgentMirrorOnDropTest, ConfigChangePostWarmboot) {
273284
mirrorPortId,
274285
kCollectorIp_,
275286
{{0,
276-
{cfg::MirrorOnDropReasonAggregation::
277-
INGRESS_PACKET_PROCESSING_DISCARDS}}});
287+
makeEventConfig(
288+
std::nullopt, // use default aging group
289+
{cfg::MirrorOnDropReasonAggregation::
290+
INGRESS_PACKET_PROCESSING_DISCARDS})}});
278291
applyNewConfig(config);
279292
};
280293

@@ -285,9 +298,11 @@ TEST_F(AgentMirrorOnDropTest, ConfigChangePostWarmboot) {
285298
&config,
286299
mirrorPortId,
287300
kCollectorIp_,
288-
{{1,
289-
{cfg::MirrorOnDropReasonAggregation::
290-
INGRESS_PACKET_PROCESSING_DISCARDS}}});
301+
{{0,
302+
makeEventConfig(
303+
cfg::MirrorOnDropAgingGroup::PORT,
304+
{cfg::MirrorOnDropReasonAggregation::
305+
INGRESS_PACKET_PROCESSING_DISCARDS})}});
291306
applyNewConfig(config);
292307
};
293308

@@ -310,8 +325,10 @@ TEST_F(AgentMirrorOnDropTest, PacketProcessingError) {
310325
mirrorPortId,
311326
kCollectorIp_,
312327
{{kEventId,
313-
{cfg::MirrorOnDropReasonAggregation::
314-
INGRESS_PACKET_PROCESSING_DISCARDS}}});
328+
makeEventConfig(
329+
std::nullopt, // use default aging group
330+
{cfg::MirrorOnDropReasonAggregation::
331+
INGRESS_PACKET_PROCESSING_DISCARDS})}});
315332
utility::addTrapPacketAcl(&config, kCollectorNextHopMac_);
316333
applyNewConfig(config);
317334

@@ -364,15 +381,23 @@ TEST_F(AgentMirrorOnDropTest, MultipleEventIDs) {
364381
mirrorPortId,
365382
kCollectorIp_,
366383
// Sandwich PP drops between other reasons and see if it still works
367-
{{0,
368-
{cfg::MirrorOnDropReasonAggregation::
369-
INGRESS_SOURCE_CONGESTION_DISCARDS}},
370-
{1,
371-
{cfg::MirrorOnDropReasonAggregation::
372-
INGRESS_PACKET_PROCESSING_DISCARDS}},
373-
{2,
374-
{cfg::MirrorOnDropReasonAggregation::
375-
INGRESS_DESTINATION_CONGESTION_DISCARDS}}});
384+
{
385+
{0,
386+
makeEventConfig(
387+
cfg::MirrorOnDropAgingGroup::GLOBAL,
388+
{cfg::MirrorOnDropReasonAggregation::
389+
INGRESS_SOURCE_CONGESTION_DISCARDS})},
390+
{1,
391+
makeEventConfig(
392+
cfg::MirrorOnDropAgingGroup::PORT,
393+
{cfg::MirrorOnDropReasonAggregation::
394+
INGRESS_PACKET_PROCESSING_DISCARDS})},
395+
{2,
396+
makeEventConfig(
397+
cfg::MirrorOnDropAgingGroup::VOQ,
398+
{cfg::MirrorOnDropReasonAggregation::
399+
INGRESS_DESTINATION_CONGESTION_DISCARDS})},
400+
});
376401
utility::addTrapPacketAcl(&config, kCollectorNextHopMac_);
377402
applyNewConfig(config);
378403

0 commit comments

Comments
 (0)