Skip to content

Commit 2b0a533

Browse files
TACapplemanTACappleman
committed
Fixes
1 parent 54234b4 commit 2b0a533

File tree

2 files changed

+101
-109
lines changed

2 files changed

+101
-109
lines changed

‎orchagent/mplsrouteorch.cpp

+100-108
Original file line numberDiff line numberDiff line change
@@ -474,97 +474,93 @@ bool RouteOrch::addLabelRoute(LabelRouteBulkContext& ctx, const NextHopGroupKey
474474

475475
auto it_route = m_syncdLabelRoutes.at(vrf_id).find(label);
476476

477-
if (ctx.nhg_index.empty())
477+
if (!ctx.nhg_index.empty())
478478
{
479-
if (nextHops.getSize() == 0)
479+
try
480480
{
481-
/* The route is pointing to a blackhole */
482-
blackhole = true;
481+
const NextHopGroup& nhg = gNhgOrch->getNhg(ctx.nhg_index);
482+
next_hop_id = nhg.getId();
483483
}
484-
/* The route is pointing to a next hop */
485-
else if (nextHops.getSize() == 1)
484+
catch(const std::out_of_range& e)
486485
{
487-
const NextHopKey& nexthop = *nextHops.getNextHops().begin();
488-
if (nexthop.isIntfNextHop())
486+
SWSS_LOG_WARN("Next hop group key %s does not exist", ctx.nhg_index.c_str());
487+
return false;
488+
}
489+
}
490+
else if (nextHops.getSize() == 0)
491+
{
492+
/* The route is pointing to a blackhole */
493+
blackhole = true;
494+
}
495+
/* The route is pointing to a next hop */
496+
else if (nextHops.getSize() == 1)
497+
{
498+
const NextHopKey& nexthop = *nextHops.getNextHops().begin();
499+
if (nexthop.isIntfNextHop())
500+
{
501+
next_hop_id = m_intfsOrch->getRouterIntfsId(nexthop.alias);
502+
/* rif is not created yet */
503+
if (next_hop_id == SAI_NULL_OBJECT_ID)
489504
{
490-
next_hop_id = m_intfsOrch->getRouterIntfsId(nexthop.alias);
491-
/* rif is not created yet */
492-
if (next_hop_id == SAI_NULL_OBJECT_ID)
493-
{
494-
SWSS_LOG_INFO("Failed to get next hop %s for %u",
495-
nextHops.to_string().c_str(), label);
496-
return false;
497-
}
505+
SWSS_LOG_INFO("Failed to get next hop %s for %u",
506+
nextHops.to_string().c_str(), label);
507+
return false;
508+
}
509+
}
510+
else
511+
{
512+
if (m_neighOrch->hasNextHop(nexthop))
513+
{
514+
next_hop_id = m_neighOrch->getNextHopId(nexthop);
515+
}
516+
/* See if there is an IP neighbor nexthop */
517+
else if (nexthop.isMplsNextHop() &&
518+
m_neighOrch->isNeighborResolved(nexthop))
519+
{
520+
m_neighOrch->addNextHop(nexthop);
521+
next_hop_id = m_neighOrch->getNextHopId(nexthop);
498522
}
499523
else
500524
{
501-
if (m_neighOrch->hasNextHop(nexthop))
502-
{
503-
next_hop_id = m_neighOrch->getNextHopId(nexthop);
504-
}
505-
/* See if there is an IP neighbor nexthop */
506-
else if (nexthop.isMplsNextHop() &&
507-
m_neighOrch->isNeighborResolved(nexthop))
508-
{
509-
m_neighOrch->addNextHop(nexthop);
510-
next_hop_id = m_neighOrch->getNextHopId(nexthop);
511-
}
512-
else
513-
{
514-
SWSS_LOG_INFO("Failed to get next hop %s for %u",
515-
nextHops.to_string().c_str(), label);
516-
return false;
517-
}
525+
SWSS_LOG_INFO("Failed to get next hop %s for %u",
526+
nextHops.to_string().c_str(), label);
527+
return false;
518528
}
519529
}
520-
/* The route is pointing to a next hop group */
521-
else
530+
}
531+
/* The route is pointing to a next hop group */
532+
else
533+
{
534+
/* Check if there is already an existing next hop group */
535+
if (!hasNextHopGroup(nextHops))
522536
{
523-
/* Check if there is already an existing next hop group */
524-
if (!hasNextHopGroup(nextHops))
537+
/* Try to create a new next hop group */
538+
if (!addNextHopGroup(nextHops))
525539
{
526-
/* Try to create a new next hop group */
527-
if (!addNextHopGroup(nextHops))
528-
{
529-
/* Failed to create the next hop group and check if a temporary route is needed */
540+
/* Failed to create the next hop group and check if a temporary route is needed */
530541

531-
/* If the current next hop is part of the next hop group to sync,
532-
* then return false and no need to add another temporary route. */
533-
if (it_route != m_syncdLabelRoutes.at(vrf_id).end() &&
534-
it_route->second.nhg_key.getSize() == 1)
542+
/* If the current next hop is part of the next hop group to sync,
543+
* then return false and no need to add another temporary route. */
544+
if (it_route != m_syncdLabelRoutes.at(vrf_id).end() &&
545+
it_route->second.nhg_key.getSize() == 1)
546+
{
547+
const NextHopKey& nexthop = *it_route->second.nhg_key.getNextHops().begin();
548+
if (nextHops.contains(nexthop))
535549
{
536-
const NextHopKey& nexthop = *it_route->second.nhg_key.getNextHops().begin();
537-
if (nextHops.contains(nexthop))
538-
{
539-
return false;
540-
}
550+
return false;
541551
}
542-
543-
/* Add a temporary route when a next hop group cannot be added,
544-
* and there is no temporary route right now or the current temporary
545-
* route is not pointing to a member of the next hop group to sync. */
546-
addTempLabelRoute(ctx, nextHops);
547-
/* Return false since the original route is not successfully added */
548-
return false;
549552
}
550-
}
551553

552-
next_hop_id = m_syncdNextHopGroups[nextHops].next_hop_group_id;
553-
}
554-
}
555-
else
556-
{
557-
SWSS_LOG_DEBUG("Next hop group is owned by NhgOrch with index %s", ctx.nhg_index.c_str());
558-
try
559-
{
560-
const NextHopGroup& nhg = gNhgOrch->getNhg(ctx.nhg_index);
561-
next_hop_id = nhg.getId();
562-
}
563-
catch(const std::out_of_range& e)
564-
{
565-
SWSS_LOG_WARN("Next hop group key %s does not exist", ctx.nhg_index.c_str());
566-
return false;
554+
/* Add a temporary route when a next hop group cannot be added,
555+
* and there is no temporary route right now or the current temporary
556+
* route is not pointing to a member of the next hop group to sync. */
557+
addTempLabelRoute(ctx, nextHops);
558+
/* Return false since the original route is not successfully added */
559+
return false;
560+
}
567561
}
562+
563+
next_hop_id = m_syncdNextHopGroups[nextHops].next_hop_group_id;
568564
}
569565

570566
/* Sync the inseg entry */
@@ -661,57 +657,53 @@ bool RouteOrch::addLabelRoutePost(const LabelRouteBulkContext& ctx, const NextHo
661657
sai_object_id_t next_hop_id;
662658

663659
/* Check that the next hop group is not owned by NhgOrch. */
664-
if (ctx.nhg_index.empty())
660+
if (!ctx.nhg_index.empty())
665661
{
666-
if (nextHops.getSize() == 0)
662+
if (!gNhgOrch->hasNhg(ctx.nhg_index))
667663
{
668-
/* The route is pointing to a blackhole */
669-
blackhole = true;
664+
SWSS_LOG_WARN("Failed to get next hop group with index %s", ctx.nhg_index.c_str());
665+
return false;
670666
}
671-
/* The route is pointing to a next hop */
672-
else if (nextHops.getSize() == 1)
667+
}
668+
else if (nextHops.getSize() == 0)
669+
{
670+
/* The route is pointing to a blackhole */
671+
blackhole = true;
672+
}
673+
/* The route is pointing to a next hop */
674+
else if (nextHops.getSize() == 1)
675+
{
676+
const NextHopKey& nexthop = *nextHops.getNextHops().begin();
677+
if (nexthop.isIntfNextHop())
673678
{
674-
const NextHopKey& nexthop = *nextHops.getNextHops().begin();
675-
if (nexthop.isIntfNextHop())
676-
{
677679

678-
next_hop_id = m_intfsOrch->getRouterIntfsId(nexthop.alias);
679-
/* rif is not created yet */
680-
if (next_hop_id == SAI_NULL_OBJECT_ID)
681-
{
682-
SWSS_LOG_INFO("Failed to get next hop %s for label %u",
683-
nextHops.to_string().c_str(), label);
684-
return false;
685-
}
686-
}
687-
else
680+
next_hop_id = m_intfsOrch->getRouterIntfsId(nexthop.alias);
681+
/* rif is not created yet */
682+
if (next_hop_id == SAI_NULL_OBJECT_ID)
688683
{
689-
if (!m_neighOrch->hasNextHop(nexthop))
690-
{
691-
SWSS_LOG_INFO("Failed to get next hop %s for label %u",
692-
nextHops.to_string().c_str(), label);
693-
return false;
694-
}
684+
SWSS_LOG_INFO("Failed to get next hop %s for label %u",
685+
nextHops.to_string().c_str(), label);
686+
return false;
695687
}
696688
}
697-
/* The route is pointing to a next hop group */
698-
else if (nextHops.getSize() > 1)
689+
else
699690
{
700-
if (!hasNextHopGroup(nextHops))
691+
if (!m_neighOrch->hasNextHop(nexthop))
701692
{
702-
// Previous added an temporary route
703-
auto& tmp_next_hop = ctx.tmp_next_hop;
704-
addLabelRoutePost(ctx, tmp_next_hop);
693+
SWSS_LOG_INFO("Failed to get next hop %s for label %u",
694+
nextHops.to_string().c_str(), label);
705695
return false;
706696
}
707697
}
708698
}
709-
else
699+
/* The route is pointing to a next hop group */
700+
else if (nextHops.getSize() > 1)
710701
{
711-
SWSS_LOG_DEBUG("NhgOrch owns the next hop group with index %s", ctx.nhg_index.c_str());
712-
if (!gNhgOrch->hasNhg(ctx.nhg_index))
702+
if (!hasNextHopGroup(nextHops))
713703
{
714-
SWSS_LOG_WARN("Failed to get next hop group with index %s", ctx.nhg_index.c_str());
704+
// Previous added an temporary route
705+
auto& tmp_next_hop = ctx.tmp_next_hop;
706+
addLabelRoutePost(ctx, tmp_next_hop);
715707
return false;
716708
}
717709
}

‎orchagent/routeorch.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ struct LabelRouteBulkContext
168168
class RouteOrch : public Orch, public Subject
169169
{
170170
public:
171-
RouteOrch(DBConnector *db, vector<table_name_with_pri_t> &tableNames, SwitchOrch *switchOrc, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch, FgNhgOrch *fgNhgOrch);
171+
RouteOrch(DBConnector *db, vector<table_name_with_pri_t> &tableNames, SwitchOrch *switchOrch, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch, FgNhgOrch *fgNhgOrch);
172172

173173
bool hasNextHopGroup(const NextHopGroupKey&) const;
174174
sai_object_id_t getNextHopGroupId(const NextHopGroupKey&);

0 commit comments

Comments
 (0)