Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[nhg]: Add support for weight in nexthop group member. #1853

Merged
merged 3 commits into from
Aug 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion fpmsyncd/routesync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,6 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj)
{
onRouteMsg(nlmsg_type, obj, master_name);
}

}
else
{
Expand Down Expand Up @@ -711,6 +710,7 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf)
/* Get nexthop lists */
string nexthops = getNextHopGw(route_obj);
string ifnames = getNextHopIf(route_obj);
string weights = getNextHopWt(route_obj);

vector<string> alsv = tokenize(ifnames, ',');
for (auto alias : alsv)
Expand All @@ -733,6 +733,11 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf)

fvVector.push_back(nh);
fvVector.push_back(idx);
if (!weights.empty())
{
FieldValueTuple wt("weight", weights);
fvVector.push_back(wt);
}

if (!warmRestartInProgress)
{
Expand Down Expand Up @@ -973,3 +978,36 @@ string RouteSync::getNextHopIf(struct rtnl_route *route_obj)

return result;
}

/*
* Get next hop weights
* @arg route_obj route object
*
* Return concatenation of interface names: wt0 + "," + wt1 + .... + "," + wtN
*/
string RouteSync::getNextHopWt(struct rtnl_route *route_obj)
{
string result = "";

for (int i = 0; i < rtnl_route_get_nnexthops(route_obj); i++)
{
struct rtnl_nexthop *nexthop = rtnl_route_nexthop_n(route_obj, i);
/* Get the weight of next hop */
uint8_t weight = rtnl_route_nh_get_weight(nexthop);
if (weight)
{
result += to_string(weight + 1);
}
else
{
return "";
}

if (i + 1 < rtnl_route_get_nnexthops(route_obj))
{
result += string(",");
}
}

return result;
}
3 changes: 3 additions & 0 deletions fpmsyncd/routesync.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ class RouteSync : public NetMsg

/* Get next hop interfaces */
string getNextHopIf(struct rtnl_route *route_obj);

/* Get next hop weights*/
string getNextHopWt(struct rtnl_route *route_obj);
};

}
Expand Down
51 changes: 49 additions & 2 deletions orchagent/nexthopgroupkey.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,20 @@ class NextHopGroupKey
}
}

NextHopGroupKey(const std::string &nexthops, const std::string &weights)
{
m_overlay_nexthops = false;
std::vector<std::string> nhv = tokenize(nexthops, NHG_DELIMITER);
std::vector<std::string> wtv = tokenize(weights, NHG_DELIMITER);
bool set_weight = wtv.size() == nhv.size();
for (uint32_t i = 0; i < nhv.size(); i++)
{
NextHopKey nh(nhv[i]);
nh.weight = set_weight? (uint32_t)std::stoi(wtv[i]) : 0;
m_nexthops.insert(nh);
}
}

inline const std::set<NextHopKey> &getNextHops() const
{
return m_nexthops;
Expand All @@ -43,12 +57,45 @@ class NextHopGroupKey

inline bool operator<(const NextHopGroupKey &o) const
{
return m_nexthops < o.m_nexthops;
if (m_nexthops < o.m_nexthops)
{
return true;
}
else if (m_nexthops == o.m_nexthops)
{
auto it1 = m_nexthops.begin();
for (auto& it2 : o.m_nexthops)
{
if (it1->weight < it2.weight)
{
return true;
}
else if(it1->weight > it2.weight)
{
return false;
}
it1++;
}
}
return false;
}

inline bool operator==(const NextHopGroupKey &o) const
{
return m_nexthops == o.m_nexthops;
if (m_nexthops != o.m_nexthops)
{
return false;
}
auto it1 = m_nexthops.begin();
for (auto& it2 : o.m_nexthops)
{
if (it2.weight != it1->weight)
{
return false;
}
it1++;
}
return true;
}

inline bool operator!=(const NextHopGroupKey &o) const
Expand Down
9 changes: 6 additions & 3 deletions orchagent/nexthopkey.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,17 @@ struct NextHopKey
uint32_t vni; // Encap VNI overlay nexthop
MacAddress mac_address; // Overlay Nexthop MAC.
LabelStack label_stack; // MPLS label stack
uint32_t weight; // NH weight for NHGs

NextHopKey() = default;
NextHopKey() : weight(0) {}
NextHopKey(const std::string &str, const std::string &alias) :
alias(alias), vni(0), mac_address()
alias(alias), vni(0), mac_address(), weight(0)
{
std::string ip_str = parseMplsNextHop(str);
ip_address = ip_str;
}
NextHopKey(const IpAddress &ip, const std::string &alias) :
ip_address(ip), alias(alias), vni(0), mac_address() {}
ip_address(ip), alias(alias), vni(0), mac_address(), weight(0) {}
NextHopKey(const std::string &str) :
vni(0), mac_address()
{
Expand Down Expand Up @@ -57,6 +58,7 @@ struct NextHopKey
std::string err = "Error converting " + str + " to NextHop";
throw std::invalid_argument(err);
}
weight = 0;
}
NextHopKey(const std::string &str, bool overlay_nh)
{
Expand All @@ -76,6 +78,7 @@ struct NextHopKey
alias = keys[1];
vni = static_cast<uint32_t>(std::stoul(keys[2]));
mac_address = keys[3];
weight = 0;
}

const std::string to_string() const
Expand Down
24 changes: 23 additions & 1 deletion orchagent/routeorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,9 @@ bool RouteOrch::validnexthopinNextHopGroup(const NextHopKey &nexthop, uint32_t&
vector<sai_attribute_t> nhgm_attrs;
sai_attribute_t nhgm_attr;

/* get updated nhkey with possible weight */
auto nhkey = nhopgroup->first.getNextHops().find(nexthop);

nhgm_attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID;
nhgm_attr.value.oid = nhopgroup->second.next_hop_group_id;
nhgm_attrs.push_back(nhgm_attr);
Expand All @@ -325,6 +328,13 @@ bool RouteOrch::validnexthopinNextHopGroup(const NextHopKey &nexthop, uint32_t&
nhgm_attr.value.oid = m_neighOrch->getNextHopId(nexthop);
nhgm_attrs.push_back(nhgm_attr);

if (nhkey->weight)
{
nhgm_attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT;
nhgm_attr.value.s32 = nhkey->weight;
nhgm_attrs.push_back(nhgm_attr);
}

status = sai_next_hop_group_api->create_next_hop_group_member(&nexthop_id, gSwitchId,
(uint32_t)nhgm_attrs.size(),
nhgm_attrs.data());
Expand Down Expand Up @@ -523,6 +533,7 @@ void RouteOrch::doTask(Consumer& consumer)
string mpls_nhs;
string vni_labels;
string remote_macs;
string weights;
bool& excp_intfs_flag = ctx.excp_intfs_flag;
bool overlay_nh = false;
bool blackhole = false;
Expand All @@ -548,6 +559,9 @@ void RouteOrch::doTask(Consumer& consumer)

if (fvField(i) == "blackhole")
blackhole = fvValue(i) == "true";

if (fvField(i) == "weight")
weights = fvValue(i);
}

vector<string> ipv = tokenize(ips, ',');
Expand Down Expand Up @@ -636,7 +650,7 @@ void RouteOrch::doTask(Consumer& consumer)
nhg_str += ipv[i] + NH_DELIMITER + alsv[i];
}

nhg = NextHopGroupKey(nhg_str);
nhg = NextHopGroupKey(nhg_str, weights);

}
else
Expand Down Expand Up @@ -1104,6 +1118,7 @@ bool RouteOrch::addNextHopGroup(const NextHopGroupKey &nexthops)
for (size_t i = 0; i < npid_count; i++)
{
auto nhid = next_hop_ids[i];
auto weight = nhopgroup_members_set[nhid].weight;

// Create a next hop group member
vector<sai_attribute_t> nhgm_attrs;
Expand All @@ -1117,6 +1132,13 @@ bool RouteOrch::addNextHopGroup(const NextHopGroupKey &nexthops)
nhgm_attr.value.oid = nhid;
nhgm_attrs.push_back(nhgm_attr);

if (weight)
{
nhgm_attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT;
nhgm_attr.value.s32 = weight;
nhgm_attrs.push_back(nhgm_attr);
}

gNextHopGroupMemberBulker.create_entry(&nhgm_ids[i],
(uint32_t)nhgm_attrs.size(),
nhgm_attrs.data());
Expand Down
110 changes: 109 additions & 1 deletion tests/test_nhg.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ def test_route_nhg(self, dvs, dvs_route, testlog):

dvs_route.check_asicdb_deleted_route_entries([rtprefix])

fvs = swsscommon.FieldValuePairs([("nexthop","10.0.0.1,10.0.0.3,10.0.0.5"), ("ifname", "Ethernet0,Ethernet4,Ethernet8")])
# nexthop group without weight
fvs = swsscommon.FieldValuePairs([("nexthop","10.0.0.1,10.0.0.3,10.0.0.5"),
("ifname", "Ethernet0,Ethernet4,Ethernet8")])
ps.set(rtprefix, fvs)

# check if route was propagated to ASIC DB
Expand All @@ -68,6 +70,112 @@ def test_route_nhg(self, dvs, dvs_route, testlog):

assert fvs["SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID"] == nhgid

# verify weight attributes not in asic db
assert fvs.get("SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT") is None

# Remove route 2.2.2.0/24
ps._del(rtprefix)

# Wait for route 2.2.2.0/24 to be removed
dvs_route.check_asicdb_deleted_route_entries([rtprefix])

# Negative test with nexthops with incomplete weight info
fvs = swsscommon.FieldValuePairs([("nexthop","10.0.0.1,10.0.0.3,10.0.0.5"),
("ifname", "Ethernet0,Ethernet4,Ethernet8"),
("weight", "10,30")])
ps.set(rtprefix, fvs)

# check if route was propagated to ASIC DB
rtkeys = dvs_route.check_asicdb_route_entries([rtprefix])

# assert the route points to next hop group
fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY", rtkeys[0])

nhgid = fvs["SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID"]

fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP", nhgid)

assert bool(fvs)

keys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER")

assert len(keys) == 3

for k in keys:
fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER", k)

assert fvs["SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID"] == nhgid

# verify weight attributes not in asic db
assert fvs.get("SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT") is None

# Remove route 2.2.2.0/24
ps._del(rtprefix)

# Wait for route 2.2.2.0/24 to be removed
dvs_route.check_asicdb_deleted_route_entries([rtprefix])

fvs = swsscommon.FieldValuePairs([("nexthop","10.0.0.1,10.0.0.3,10.0.0.5"),
("ifname", "Ethernet0,Ethernet4,Ethernet8"),
("weight", "10,30,50")])
ps.set(rtprefix, fvs)

# check if route was propagated to ASIC DB
rtkeys = dvs_route.check_asicdb_route_entries([rtprefix])

# assert the route points to next hop group
fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY", rtkeys[0])

nhgid = fvs["SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID"]

fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP", nhgid)

assert bool(fvs)

keys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER")

assert len(keys) == 3

for k in keys:
fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER", k)

assert fvs["SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID"] == nhgid

# verify weight attributes in asic db
nhid = fvs["SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID"]
weight = fvs["SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT"]

fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP", nhid)
nhip = fvs["SAI_NEXT_HOP_ATTR_IP"].split('.')
expected_weight = int(nhip[3]) * 10

assert int(weight) == expected_weight

rtprefix2 = "3.3.3.0/24"

fvs = swsscommon.FieldValuePairs([("nexthop","10.0.0.1,10.0.0.3,10.0.0.5"),
("ifname", "Ethernet0,Ethernet4,Ethernet8"),
("weight", "20,30,40")])
ps.set(rtprefix2, fvs)

# wait for route to be programmed
time.sleep(1)

keys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP")

assert len(keys) == 2

keys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER")

assert len(keys) == 6

# Remove route 3.3.3.0/24
ps._del(rtprefix2)

# Wait for route 3.3.3.0/24 to be removed
dvs_route.check_asicdb_deleted_route_entries([rtprefix2])


# bring links down one-by-one
for i in [0, 1, 2]:
dvs.servers[i].runcmd("ip link set down dev eth0") == 0
Expand Down