Skip to content

Commit 7876662

Browse files
saifhhasanfacebook-github-bot
saifhhasan
authored and
facebook-github-bot
committed
Attr to distinguish between LFA vs 2nd-shortest path
Summary: Fib module forwards only smallest metric nexthops to FibAgent. This is inteded to not program LFA routes and keep them in backup. However in case of KSP2 we want both shortest and non-shortest paths to be programmed. Adding `useNonShortestRoute` attribute to nextHop to achieve the same. Nexthops that have this attribute set will always be programmed Reviewed By: jstrizich Differential Revision: D15728459 fbshipit-source-id: 46170e55ce8906fc3d73a44d3a646535f6ae319a
1 parent 150ad06 commit 7876662

6 files changed

Lines changed: 72 additions & 54 deletions

File tree

‎openr/common/Util.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ getBestNextHopsUnicast(std::vector<thrift::NextHopThrift> const& allNextHops) {
426426
// Find nextHops with the minimum cost
427427
std::vector<thrift::NextHopThrift> bestNextHops;
428428
for (auto const& nextHop : allNextHops) {
429-
if (nextHop.metric == minCost) {
429+
if (nextHop.metric == minCost or nextHop.useNonShortestRoute) {
430430
bestNextHops.emplace_back(nextHop);
431431
}
432432
}

‎openr/common/Util.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,12 +331,14 @@ createNextHop(
331331
thrift::BinaryAddress addr,
332332
const std::string& ifName = "",
333333
int32_t metric = 0,
334-
folly::Optional<thrift::MplsAction> maybeMplsAction = folly::none) {
334+
folly::Optional<thrift::MplsAction> maybeMplsAction = folly::none,
335+
bool useNonShortestRoute = false) {
335336
thrift::NextHopThrift nextHop;
336337
nextHop.address = addr;
337338
nextHop.address.ifName = ifName;
338339
nextHop.metric = metric;
339340
nextHop.mplsAction = maybeMplsAction;
341+
nextHop.useNonShortestRoute = useNonShortestRoute;
340342
return nextHop;
341343
}
342344

‎openr/common/tests/UtilTest.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,15 @@ TEST(UtilTest, getBestNextHopsUnicast) {
413413
getBestNextHopsUnicast({path1_2_1, path1_2_2, path1_2_3, path1_3_1});
414414
EXPECT_EQ(
415415
bestNextHops, std::vector<thrift::NextHopThrift>({path1_2_1, path1_3_1}));
416+
417+
auto path1_2_2_updated = path1_2_2;
418+
path1_2_2_updated.useNonShortestRoute = true;
419+
bestNextHops = getBestNextHopsUnicast(
420+
{path1_2_1, path1_2_2_updated, path1_2_3, path1_3_1});
421+
EXPECT_EQ(
422+
bestNextHops,
423+
std::vector<thrift::NextHopThrift>(
424+
{path1_2_1, path1_2_2_updated, path1_3_1}));
416425
}
417426

418427
TEST(UtilTest, getBestNextHopsMpls) {

‎openr/decision/Decision.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1215,7 +1215,8 @@ SpfSolver::SpfSolverImpl::createOpenRKsp2EdRoute(
12151215
: firstLink->getNhV6FromNode(myNodeName),
12161216
firstLink->getIfaceFromNode(myNodeName),
12171217
pathCost,
1218-
std::move(mplsAction)));
1218+
std::move(mplsAction),
1219+
true /* useNonShortestRoute */));
12191220
}
12201221

12211222
return std::move(route);

‎openr/decision/tests/DecisionTest.cpp‎

Lines changed: 54 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ const thrift::NextHopThrift labelPopNextHop{
109109
toBinaryAddress(folly::IPAddressV6("::")),
110110
0 /* weight */,
111111
labelPopAction,
112-
0 /* metric */};
112+
0 /* metric */,
113+
false /* useNonShortestRoute */};
113114

114115
// timeout to wait until decision debounce
115116
// (i.e. spf recalculation, route rebuild) finished
@@ -130,12 +131,14 @@ createNextHopFromAdj(
130131
thrift::Adjacency adj,
131132
bool isV4,
132133
int32_t metric,
133-
folly::Optional<thrift::MplsAction> mplsAction = folly::none) {
134+
folly::Optional<thrift::MplsAction> mplsAction = folly::none,
135+
bool useNonShortestRoute = false) {
134136
return createNextHop(
135137
isV4 ? adj.nextHopV4 : adj.nextHopV6,
136138
adj.ifName,
137139
metric,
138-
std::move(mplsAction));
140+
std::move(mplsAction),
141+
useNonShortestRoute);
139142
}
140143

141144
// Note: use unordered_set bcoz paths in a route can be in arbitrary order
@@ -1348,25 +1351,25 @@ TEST_P(SimpleRingTopologyFixture, Ksp2EdEcmp) {
13481351
// validate router 1
13491352
EXPECT_EQ(
13501353
routeMap[make_pair("1", toString(v4Enabled ? addr4V4 : addr4))],
1351-
NextHops({createNextHopFromAdj(adj12, v4Enabled, 20, push4),
1352-
createNextHopFromAdj(adj13, v4Enabled, 20, push4)}));
1354+
NextHops({createNextHopFromAdj(adj12, v4Enabled, 20, push4, true),
1355+
createNextHopFromAdj(adj13, v4Enabled, 20, push4, true)}));
13531356
EXPECT_EQ(
13541357
routeMap[make_pair("1", std::to_string(adjacencyDb4.nodeLabel))],
13551358
NextHops({createNextHopFromAdj(adj12, false, 20, labelSwapAction4),
13561359
createNextHopFromAdj(adj13, false, 20, labelSwapAction4)}));
13571360

13581361
EXPECT_EQ(
13591362
routeMap[make_pair("1", toString(v4Enabled ? addr3V4 : addr3))],
1360-
NextHops({createNextHopFromAdj(adj13, v4Enabled, 10),
1361-
createNextHopFromAdj(adj12, v4Enabled, 30, push34)}));
1363+
NextHops({createNextHopFromAdj(adj13, v4Enabled, 10, folly::none, true),
1364+
createNextHopFromAdj(adj12, v4Enabled, 30, push34, true)}));
13621365
EXPECT_EQ(
13631366
routeMap[make_pair("1", std::to_string(adjacencyDb3.nodeLabel))],
13641367
NextHops({createNextHopFromAdj(adj13, false, 10, labelPhpAction)}));
13651368

13661369
EXPECT_EQ(
13671370
routeMap[make_pair("1", toString(v4Enabled ? addr2V4 : addr2))],
1368-
NextHops({createNextHopFromAdj(adj12, v4Enabled, 10),
1369-
createNextHopFromAdj(adj13, v4Enabled, 30, push24)}));
1371+
NextHops({createNextHopFromAdj(adj12, v4Enabled, 10, folly::none, true),
1372+
createNextHopFromAdj(adj13, v4Enabled, 30, push24, true)}));
13701373
EXPECT_EQ(
13711374
routeMap[make_pair("1", std::to_string(adjacencyDb2.nodeLabel))],
13721375
NextHops({createNextHopFromAdj(adj12, false, 10, labelPhpAction)}));
@@ -1378,25 +1381,25 @@ TEST_P(SimpleRingTopologyFixture, Ksp2EdEcmp) {
13781381

13791382
EXPECT_EQ(
13801383
routeMap[make_pair("2", toString(v4Enabled ? addr4V4 : addr4))],
1381-
NextHops({createNextHopFromAdj(adj24, v4Enabled, 10),
1382-
createNextHopFromAdj(adj21, v4Enabled, 30, push43)}));
1384+
NextHops({createNextHopFromAdj(adj24, v4Enabled, 10, folly::none, true),
1385+
createNextHopFromAdj(adj21, v4Enabled, 30, push43, true)}));
13831386
EXPECT_EQ(
13841387
routeMap[make_pair("2", std::to_string(adjacencyDb4.nodeLabel))],
13851388
NextHops({createNextHopFromAdj(adj24, false, 10, labelPhpAction)}));
13861389

13871390
EXPECT_EQ(
13881391
routeMap[make_pair("2", toString(v4Enabled ? addr3V4 : addr3))],
1389-
NextHops({createNextHopFromAdj(adj21, v4Enabled, 20, push3),
1390-
createNextHopFromAdj(adj24, v4Enabled, 20, push3)}));
1392+
NextHops({createNextHopFromAdj(adj21, v4Enabled, 20, push3, true),
1393+
createNextHopFromAdj(adj24, v4Enabled, 20, push3, true)}));
13911394
EXPECT_EQ(
13921395
routeMap[make_pair("2", std::to_string(adjacencyDb3.nodeLabel))],
13931396
NextHops({createNextHopFromAdj(adj21, false, 20, labelSwapAction3),
13941397
createNextHopFromAdj(adj24, false, 20, labelSwapAction3)}));
13951398

13961399
EXPECT_EQ(
13971400
routeMap[make_pair("2", toString(v4Enabled ? addr1V4 : addr1))],
1398-
NextHops({createNextHopFromAdj(adj21, v4Enabled, 10),
1399-
createNextHopFromAdj(adj24, v4Enabled, 30, push13)}));
1401+
NextHops({createNextHopFromAdj(adj21, v4Enabled, 10, folly::none, true),
1402+
createNextHopFromAdj(adj24, v4Enabled, 30, push13, true)}));
14001403
EXPECT_EQ(
14011404
routeMap[make_pair("2", std::to_string(adjacencyDb1.nodeLabel))],
14021405
NextHops({createNextHopFromAdj(adj21, false, 10, labelPhpAction)}));
@@ -1408,25 +1411,25 @@ TEST_P(SimpleRingTopologyFixture, Ksp2EdEcmp) {
14081411

14091412
EXPECT_EQ(
14101413
routeMap[make_pair("3", toString(v4Enabled ? addr4V4 : addr4))],
1411-
NextHops({createNextHopFromAdj(adj34, v4Enabled, 10),
1412-
createNextHopFromAdj(adj31, v4Enabled, 30, push42)}));
1414+
NextHops({createNextHopFromAdj(adj34, v4Enabled, 10, folly::none, true),
1415+
createNextHopFromAdj(adj31, v4Enabled, 30, push42, true)}));
14131416
EXPECT_EQ(
14141417
routeMap[make_pair("3", std::to_string(adjacencyDb4.nodeLabel))],
14151418
NextHops({createNextHopFromAdj(adj34, false, 10, labelPhpAction)}));
14161419

14171420
EXPECT_EQ(
14181421
routeMap[make_pair("3", toString(v4Enabled ? addr2V4 : addr2))],
1419-
NextHops({createNextHopFromAdj(adj31, v4Enabled, 20, push2),
1420-
createNextHopFromAdj(adj34, v4Enabled, 20, push2)}));
1422+
NextHops({createNextHopFromAdj(adj31, v4Enabled, 20, push2, true),
1423+
createNextHopFromAdj(adj34, v4Enabled, 20, push2, true)}));
14211424
EXPECT_EQ(
14221425
routeMap[make_pair("3", std::to_string(adjacencyDb2.nodeLabel))],
14231426
NextHops({createNextHopFromAdj(adj31, false, 20, labelSwapAction2),
14241427
createNextHopFromAdj(adj34, false, 20, labelSwapAction2)}));
14251428

14261429
EXPECT_EQ(
14271430
routeMap[make_pair("3", toString(v4Enabled ? addr1V4 : addr1))],
1428-
NextHops({createNextHopFromAdj(adj31, v4Enabled, 10),
1429-
createNextHopFromAdj(adj34, v4Enabled, 30, push12)}));
1431+
NextHops({createNextHopFromAdj(adj31, v4Enabled, 10, folly::none, true),
1432+
createNextHopFromAdj(adj34, v4Enabled, 30, push12, true)}));
14301433
EXPECT_EQ(
14311434
routeMap[make_pair("3", std::to_string(adjacencyDb1.nodeLabel))],
14321435
NextHops({createNextHopFromAdj(adj31, false, 10, labelPhpAction)}));
@@ -1438,24 +1441,24 @@ TEST_P(SimpleRingTopologyFixture, Ksp2EdEcmp) {
14381441

14391442
EXPECT_EQ(
14401443
routeMap[make_pair("4", toString(v4Enabled ? addr3V4 : addr3))],
1441-
NextHops({createNextHopFromAdj(adj43, v4Enabled, 10),
1442-
createNextHopFromAdj(adj42, v4Enabled, 30, push31)}));
1444+
NextHops({createNextHopFromAdj(adj43, v4Enabled, 10, folly::none, true),
1445+
createNextHopFromAdj(adj42, v4Enabled, 30, push31, true)}));
14431446
EXPECT_EQ(
14441447
routeMap[make_pair("4", std::to_string(adjacencyDb3.nodeLabel))],
14451448
NextHops({createNextHopFromAdj(adj43, false, 10, labelPhpAction)}));
14461449

14471450
EXPECT_EQ(
14481451
routeMap[make_pair("4", toString(v4Enabled ? addr2V4 : addr2))],
1449-
NextHops({createNextHopFromAdj(adj42, v4Enabled, 10),
1450-
createNextHopFromAdj(adj43, v4Enabled, 30, push21)}));
1452+
NextHops({createNextHopFromAdj(adj42, v4Enabled, 10, folly::none, true),
1453+
createNextHopFromAdj(adj43, v4Enabled, 30, push21, true)}));
14511454
EXPECT_EQ(
14521455
routeMap[make_pair("4", std::to_string(adjacencyDb2.nodeLabel))],
14531456
NextHops({createNextHopFromAdj(adj42, false, 10, labelPhpAction)}));
14541457

14551458
EXPECT_EQ(
14561459
routeMap[make_pair("4", toString(v4Enabled ? addr1V4 : addr1))],
1457-
NextHops({createNextHopFromAdj(adj42, v4Enabled, 20, push1),
1458-
createNextHopFromAdj(adj43, v4Enabled, 20, push1)}));
1460+
NextHops({createNextHopFromAdj(adj42, v4Enabled, 20, push1, true),
1461+
createNextHopFromAdj(adj43, v4Enabled, 20, push1, true)}));
14591462
EXPECT_EQ(
14601463
routeMap[make_pair("4", std::to_string(adjacencyDb1.nodeLabel))],
14611464
NextHops({createNextHopFromAdj(adj42, false, 20, labelSwapAction1),
@@ -2243,63 +2246,63 @@ TEST_F(ParallelAdjRingTopologyFixture, Ksp2EdEcmp) {
22432246

22442247
EXPECT_EQ(
22452248
routeMap[make_pair("1", toString(addr4))],
2246-
NextHops({createNextHopFromAdj(adj12_1, false, 22, push4),
2247-
createNextHopFromAdj(adj13_1, false, 22, push4)}));
2249+
NextHops({createNextHopFromAdj(adj12_1, false, 22, push4, true),
2250+
createNextHopFromAdj(adj13_1, false, 22, push4, true)}));
22482251

22492252
EXPECT_EQ(
22502253
routeMap[make_pair("1", toString(addr3))],
2251-
NextHops({createNextHopFromAdj(adj13_1, false, 11),
2252-
createNextHopFromAdj(adj12_1, false, 33, push34)}));
2254+
NextHops({createNextHopFromAdj(adj13_1, false, 11, folly::none, true),
2255+
createNextHopFromAdj(adj12_1, false, 33, push34, true)}));
22532256
EXPECT_EQ(
22542257
routeMap[make_pair("1", toString(addr2))],
2255-
NextHops({createNextHopFromAdj(adj12_1, false, 11),
2256-
createNextHopFromAdj(adj12_3, false, 20)}));
2258+
NextHops({createNextHopFromAdj(adj12_1, false, 11, folly::none, true),
2259+
createNextHopFromAdj(adj12_3, false, 20, folly::none, true)}));
22572260

22582261
// validate router 2
22592262

22602263
EXPECT_EQ(
22612264
routeMap[make_pair("2", toString(addr4))],
2262-
NextHops({createNextHopFromAdj(adj24_1, false, 11),
2263-
createNextHopFromAdj(adj21_1, false, 33, push43)}));
2265+
NextHops({createNextHopFromAdj(adj24_1, false, 11, folly::none, true),
2266+
createNextHopFromAdj(adj21_1, false, 33, push43, true)}));
22642267

22652268
EXPECT_EQ(
22662269
routeMap[make_pair("2", toString(addr3))],
2267-
NextHops({createNextHopFromAdj(adj21_1, false, 22, push3),
2268-
createNextHopFromAdj(adj24_1, false, 22, push3)}));
2270+
NextHops({createNextHopFromAdj(adj21_1, false, 22, push3, true),
2271+
createNextHopFromAdj(adj24_1, false, 22, push3, true)}));
22692272
EXPECT_EQ(
22702273
routeMap[make_pair("2", toString(addr1))],
2271-
NextHops({createNextHopFromAdj(adj21_1, false, 11),
2272-
createNextHopFromAdj(adj21_3, false, 20)}));
2274+
NextHops({createNextHopFromAdj(adj21_1, false, 11, folly::none, true),
2275+
createNextHopFromAdj(adj21_3, false, 20, folly::none, true)}));
22732276

22742277
// validate router 3
22752278

22762279
EXPECT_EQ(
22772280
routeMap[make_pair("3", toString(addr4))],
2278-
NextHops({createNextHopFromAdj(adj34_1, false, 11),
2279-
createNextHopFromAdj(adj34_3, false, 20)}));
2281+
NextHops({createNextHopFromAdj(adj34_1, false, 11, folly::none, true),
2282+
createNextHopFromAdj(adj34_3, false, 20, folly::none, true)}));
22802283
EXPECT_EQ(
22812284
routeMap[make_pair("3", toString(addr2))],
2282-
NextHops({createNextHopFromAdj(adj31_1, false, 22, push2),
2283-
createNextHopFromAdj(adj34_1, false, 22, push2)}));
2285+
NextHops({createNextHopFromAdj(adj31_1, false, 22, push2, true),
2286+
createNextHopFromAdj(adj34_1, false, 22, push2, true)}));
22842287
EXPECT_EQ(
22852288
routeMap[make_pair("3", toString(addr1))],
2286-
NextHops({createNextHopFromAdj(adj31_1, false, 11),
2287-
createNextHopFromAdj(adj34_1, false, 33, push12)}));
2289+
NextHops({createNextHopFromAdj(adj31_1, false, 11, folly::none, true),
2290+
createNextHopFromAdj(adj34_1, false, 33, push12, true)}));
22882291

22892292
// validate router 4
22902293

22912294
EXPECT_EQ(
22922295
routeMap[make_pair("4", toString(addr3))],
2293-
NextHops({createNextHopFromAdj(adj43_1, false, 11),
2294-
createNextHopFromAdj(adj43_3, false, 20)}));
2296+
NextHops({createNextHopFromAdj(adj43_1, false, 11, folly::none, true),
2297+
createNextHopFromAdj(adj43_3, false, 20, folly::none, true)}));
22952298
EXPECT_EQ(
22962299
routeMap[make_pair("4", toString(addr2))],
2297-
NextHops({createNextHopFromAdj(adj42_1, false, 11),
2298-
createNextHopFromAdj(adj43_1, false, 33, push21)}));
2300+
NextHops({createNextHopFromAdj(adj42_1, false, 11, folly::none, true),
2301+
createNextHopFromAdj(adj43_1, false, 33, push21, true)}));
22992302
EXPECT_EQ(
23002303
routeMap[make_pair("4", toString(addr1))],
2301-
NextHops({createNextHopFromAdj(adj42_1, false, 22, push1),
2302-
createNextHopFromAdj(adj43_1, false, 22, push1)}));
2304+
NextHops({createNextHopFromAdj(adj42_1, false, 22, push1, true),
2305+
createNextHopFromAdj(adj43_1, false, 22, push1, true)}));
23032306
}
23042307

23052308
/**

‎openr/if/Network.thrift‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ struct NextHopThrift {
6363

6464
// Metric (aka cost) associated with this nexthop
6565
51: i32 metric = 0
66+
67+
// Use non-shortest route (usually false but enabled for KSP2_ED_ECMP)
68+
52: bool useNonShortestRoute = 0
6669
}
6770

6871
struct MplsRoute {

0 commit comments

Comments
 (0)