Skip to content

Commit

Permalink
Fix up following review
Browse files Browse the repository at this point in the history
  • Loading branch information
@TACappleman
TACappleman committed Oct 8, 2021
1 parent a90c122 commit 78bece7
Show file tree
Hide file tree
Showing 12 changed files with 1,617 additions and 2,237 deletions.
19 changes: 12 additions & 7 deletions doc/swss-schema.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,20 +160,24 @@ and reflects the LAG ports into the redis under: `LAG_TABLE:<team0>:port`
key = ROUTE_TABLE:prefix
nexthop = *prefix, ;IP addresses separated “,” (empty indicates no gateway)
ifname = ifindex? PORT_TABLE.key ; zero or more separated by “,” (zero indicates no interface)
mpls_nh = STRING ; Comma-separated list of MPLS NH info.
blackhole = BIT ; Set to 1 if this route is a blackhole (or null0)
weight = weight_list ; List of weights.
nexthop_group = string ; index within the NEXTHOP_GROUP_TABLE, used instead of nexthop and intf fields

---------------------------------------------

###### LABEL_ROUTE_TABLE
; Defines schema for MPLS label route table attributes
key = LABEL_ROUTE_TABLE:mpls_label ; MPLS label
; field = value
nexthop = STRING ; Comma-separated list of nexthops.
ifname = STRING ; Comma-separated list of interfaces.
weight = STRING ; Comma-separated list of weights.
nexthop_group = string ; index within the NEXTHOP_GROUP_TABLE, used instead of nexthop and intf fields
; Defines schema for MPLS label route table attributes
key = LABEL_ROUTE_TABLE:mpls_label ; MPLS label
; field = value
nexthop = STRING ; Comma-separated list of nexthops.
ifname = STRING ; Comma-separated list of interfaces.
mpls_nh = STRING ; Comma-separated list of MPLS NH info.
mpls_pop = STRING ; Number of ingress MPLS labels to POP
weight = STRING ; Comma-separated list of weights.
blackhole = BIT ; Set to 1 if this route is a blackhole (or null0)
nexthop_group = string ; index within the NEXTHOP_GROUP_TABLE, used instead of nexthop and intf fields

---------------------------------------------
### NEXTHOP_GROUP_TABLE
Expand All @@ -182,6 +186,7 @@ nexthop_group = string ; index within the NEXTHOP_GROUP_TABLE, used instead of n
key = NEXTHOP_GROUP_TABLE:string ; arbitrary index for the next hop group
nexthop = *prefix, ;IP addresses separated “,” (empty indicates no gateway)
ifname = ifindex? PORT_TABLE.key ; zero or more separated by “,” (zero indicates no interface)
mpls_nh = STRING ; Comma-separated list of MPLS NH info.
weight = weight_list ; List of weights.

---------------------------------------------
Expand Down
48 changes: 14 additions & 34 deletions orchagent/mplsrouteorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,6 @@ void RouteOrch::doLabelTask(Consumer& consumer)

if (op == SET_COMMAND)
{
SWSS_LOG_DEBUG("Set operation");

string ips;
string aliases;
string mpls_nhs;
Expand Down Expand Up @@ -158,14 +156,6 @@ void RouteOrch::doLabelTask(Consumer& consumer)
nhg_index = fvValue(i);
}

SWSS_LOG_INFO("Label route %u has nexthop_group: %s, ips: %s, "
"MPLS NHs: %s, aliases: %s",
label,
nhg_index.c_str(),
ips.c_str(),
mpls_nhs.c_str(),
aliases.c_str());

/*
* A route should not fill both nexthop_group and ips /
* aliases.
Expand Down Expand Up @@ -265,12 +255,11 @@ void RouteOrch::doLabelTask(Consumer& consumer)
{
const NextHopGroup& nh_group = gNhgOrch->getNhg(nhg_index);
ctx.nhg = nh_group.getKey();
ctx.is_temp = nh_group.isTemp();
ctx.using_temp_nhg = nh_group.isTemp();
}
catch (const std::out_of_range& e)
{
SWSS_LOG_ERROR("Next hop group %s does not exist",
nhg_index.c_str());
SWSS_LOG_ERROR("Next hop group %s does not exist", nhg_index.c_str());
++it;
continue;
}
Expand Down Expand Up @@ -303,7 +292,7 @@ void RouteOrch::doLabelTask(Consumer& consumer)
else if (m_syncdLabelRoutes.find(vrf_id) == m_syncdLabelRoutes.end() ||
m_syncdLabelRoutes.at(vrf_id).find(label) == m_syncdLabelRoutes.at(vrf_id).end() ||
m_syncdLabelRoutes.at(vrf_id).at(label) != RouteNhg(nhg, nhg_index) ||
ctx.is_temp)
ctx.using_temp_nhg)
{
if (addLabelRoute(ctx, nhg))
it = consumer.m_toSync.erase(it);
Expand All @@ -328,7 +317,6 @@ void RouteOrch::doLabelTask(Consumer& consumer)
else if (op == DEL_COMMAND)
{
/* Cannot locate the route or remove succeed */
SWSS_LOG_DEBUG("Delete operation");
if (removeLabelRoute(ctx))
it = consumer.m_toSync.erase(it);
else
Expand Down Expand Up @@ -398,7 +386,7 @@ void RouteOrch::doLabelTask(Consumer& consumer)
else if (m_syncdLabelRoutes.find(vrf_id) == m_syncdLabelRoutes.end() ||
m_syncdLabelRoutes.at(vrf_id).find(label) == m_syncdLabelRoutes.at(vrf_id).end() ||
m_syncdLabelRoutes.at(vrf_id).at(label) != RouteNhg(nhg, ctx.nhg_index) ||
ctx.is_temp)
ctx.using_temp_nhg)
{
if (addLabelRoutePost(ctx, nhg))
it_prev = consumer.m_toSync.erase(it_prev);
Expand Down Expand Up @@ -443,10 +431,9 @@ void RouteOrch::addTempLabelRoute(LabelRouteBulkContext& ctx, const NextHopGroup
* a labeled one, which are created by RouteOrch or NhgOrch if the IP
* next hop exists.
*/
if (!m_neighOrch->hasNextHop(it->ipKey()))
if (!m_neighOrch->isNeighborResolved(*it))
{
SWSS_LOG_INFO("Failed to get next hop %s for %u",
(*it).to_string().c_str(), label);
SWSS_LOG_INFO("Failed to get next hop %s for %u", (*it).to_string().c_str(), label);
it = next_hop_set.erase(it);
}
else
Expand Down Expand Up @@ -475,10 +462,6 @@ bool RouteOrch::addLabelRoute(LabelRouteBulkContext& ctx, const NextHopGroupKey
sai_object_id_t& vrf_id = ctx.vrf_id;
Label& label = ctx.label;

SWSS_LOG_NOTICE("Adding route for label %u with next hop(s) %s",
label,
nextHops.to_string().c_str());

/* next_hop_id indicates the next hop id or next hop group id of this route */
sai_object_id_t next_hop_id;
bool blackhole = false;
Expand Down Expand Up @@ -521,7 +504,7 @@ bool RouteOrch::addLabelRoute(LabelRouteBulkContext& ctx, const NextHopGroupKey
}
/* See if there is an IP neighbor nexthop */
else if (nexthop.isMplsNextHop() &&
m_neighOrch->hasNextHop(NextHopKey(nexthop.ip_address, nexthop.alias)))
m_neighOrch->isNeighborResolved(nexthop))
{
m_neighOrch->addNextHop(nexthop);
next_hop_id = m_neighOrch->getNextHopId(nexthop);
Expand All @@ -547,7 +530,8 @@ bool RouteOrch::addLabelRoute(LabelRouteBulkContext& ctx, const NextHopGroupKey

/* If the current next hop is part of the next hop group to sync,
* then return false and no need to add another temporary route. */
if (it_route != m_syncdLabelRoutes.at(vrf_id).end() && it_route->second.nhg_key.getSize() == 1)
if (it_route != m_syncdLabelRoutes.at(vrf_id).end() &&
it_route->second.nhg_key.getSize() == 1)
{
const NextHopKey& nexthop = *it_route->second.nhg_key.getNextHops().begin();
if (nextHops.contains(nexthop))
Expand All @@ -570,17 +554,15 @@ bool RouteOrch::addLabelRoute(LabelRouteBulkContext& ctx, const NextHopGroupKey
}
else
{
SWSS_LOG_DEBUG("Next hop group is owned by NhgOrch with index %s",
ctx.nhg_index.c_str());
SWSS_LOG_DEBUG("Next hop group is owned by NhgOrch with index %s", ctx.nhg_index.c_str());
try
{
const NextHopGroup& nhg = gNhgOrch->getNhg(ctx.nhg_index);
next_hop_id = nhg.getId();
}
catch(const std::out_of_range& e)
{
SWSS_LOG_WARN("Next hop group key %s does not exist",
ctx.nhg_index.c_str());
SWSS_LOG_WARN("Next hop group key %s does not exist", ctx.nhg_index.c_str());
return false;
}
}
Expand Down Expand Up @@ -726,12 +708,10 @@ bool RouteOrch::addLabelRoutePost(const LabelRouteBulkContext& ctx, const NextHo
}
else
{
SWSS_LOG_DEBUG("NhgOrch owns the next hop group with index %s",
ctx.nhg_index.c_str());
SWSS_LOG_DEBUG("NhgOrch owns the next hop group with index %s", ctx.nhg_index.c_str());
if (!gNhgOrch->hasNhg(ctx.nhg_index))
{
SWSS_LOG_WARN("Failed to get next hop group with index %s",
ctx.nhg_index.c_str());
SWSS_LOG_WARN("Failed to get next hop group with index %s", ctx.nhg_index.c_str());
return false;
}
}
Expand Down Expand Up @@ -934,7 +914,7 @@ bool RouteOrch::removeLabelRoutePost(const LabelRouteBulkContext& ctx)
*/
else if (it_route->second.nhg_key.getSize() == 1)
{
NextHopKey nexthop(it_route->second.nhg_key.to_string());
const NextHopKey& nexthop = *it_route->second.nhg_key.getNextHops().begin();
if (nexthop.isMplsNextHop() &&
(m_neighOrch->getNextHopRefCount(nexthop) == 0))
{
Expand Down
16 changes: 13 additions & 3 deletions orchagent/neighorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ NeighOrch::NeighOrch(DBConnector *appDb, string tableName, IntfsOrch *intfsOrch,
SWSS_LOG_ENTER();

m_fdbOrch->attach(this);

if(gMySwitchType == "voq")
{
//Add subscriber to process VOQ system neigh
Expand Down Expand Up @@ -171,6 +171,16 @@ bool NeighOrch::hasNextHop(const NextHopKey &nexthop)
return m_syncdNextHops.find(nexthop) != m_syncdNextHops.end();
}

// Check if the underlying neighbor is resolved for a given next hop key.
bool NeighOrch::isNeighborResolved(const NextHopKey &nexthop)
{
// Extract the IP address and interface from the next hop key, and check if the next hop
// for just that pair exists.
NextHopKey base_nexthop = NextHopKey(nexthop.ip_address, nexthop.alias);

return hasNextHop(base_nexthop);
}

bool NeighOrch::addNextHop(const NextHopKey &nh)
{
SWSS_LOG_ENTER();
Expand Down Expand Up @@ -1028,7 +1038,7 @@ bool NeighOrch::removeNeighbor(const NeighborEntry &neighborEntry, bool disable)

NeighborUpdate update = { neighborEntry, MacAddress(), false };
notify(SUBJECT_TYPE_NEIGH_CHANGE, static_cast<void *>(&update));

if(gMySwitchType == "voq")
{
//Sync the neighbor to delete from the CHASSIS_APP_DB
Expand Down Expand Up @@ -1302,7 +1312,7 @@ void NeighOrch::doVoqSystemNeighTask(Consumer &consumer)
// the owner asic's mac is not readily avaiable here, the owner asic mac is derived from
// the switch id and lower 5 bytes of asic mac which is assumed to be same for all asics
// in the VS system.
// Therefore to make VOQ chassis systems work in VS platform based setups like the setups
// Therefore to make VOQ chassis systems work in VS platform based setups like the setups
// using KVMs, it is required that all asics have same base mac in the format given below
// <lower 5 bytes of mac same for all asics>:<6th byte = switch_id>

Expand Down
1 change: 1 addition & 0 deletions orchagent/neighorch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class NeighOrch : public Orch, public Subject, public Observer
~NeighOrch();

bool hasNextHop(const NextHopKey&);
bool isNeighborResolved(const NextHopKey&);
bool addNextHop(const NextHopKey&);
bool removeMplsNextHop(const NextHopKey&);

Expand Down
6 changes: 0 additions & 6 deletions orchagent/nexthopkey.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,6 @@ struct NextHopKey
}
return str;
}

// Method to get the underlying IP/interface pair for the next hop.
NextHopKey ipKey() const
{
return NextHopKey(ip_address, alias);
}
};

#endif /* SWSS_NEXTHOPKEY_H */
Loading

0 comments on commit 78bece7

Please sign in to comment.