From 0217b664f15257795bce229fcbc83ba40f0fe0d8 Mon Sep 17 00:00:00 2001 From: Zhenghui Cai-Juniper Networks Date: Wed, 11 Aug 2021 08:15:43 -0400 Subject: [PATCH] [nhg]: Add support for weight in nexthop group member. (#1853) * * Add support for weight in nexthop group member. 1. In fpmsyncd, parse weight field in nlmsg, set APP_DB if weight is set. 2. In routeorch, collect weight and pass weight attribute to next hop group memeber object. Signed-off-by: Zhenghui Cai --- fpmsyncd/routesync.cpp | 40 ++++++++++++- fpmsyncd/routesync.h | 3 + orchagent/nexthopgroupkey.h | 51 ++++++++++++++++- orchagent/nexthopkey.h | 9 ++- orchagent/routeorch.cpp | 24 +++++++- tests/test_nhg.py | 110 +++++++++++++++++++++++++++++++++++- 6 files changed, 229 insertions(+), 8 deletions(-) diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index 9910dca07e1f..00cb7cdd2b73 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -598,7 +598,6 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) { onRouteMsg(nlmsg_type, obj, master_name); } - } else { @@ -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 alsv = tokenize(ifnames, ','); for (auto alias : alsv) @@ -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) { @@ -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; +} diff --git a/fpmsyncd/routesync.h b/fpmsyncd/routesync.h index 71a20f9d6662..968072c428e5 100644 --- a/fpmsyncd/routesync.h +++ b/fpmsyncd/routesync.h @@ -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); }; } diff --git a/orchagent/nexthopgroupkey.h b/orchagent/nexthopgroupkey.h index 22e75de55148..caba479012d2 100644 --- a/orchagent/nexthopgroupkey.h +++ b/orchagent/nexthopgroupkey.h @@ -31,6 +31,20 @@ class NextHopGroupKey } } + NextHopGroupKey(const std::string &nexthops, const std::string &weights) + { + m_overlay_nexthops = false; + std::vector nhv = tokenize(nexthops, NHG_DELIMITER); + std::vector 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 &getNextHops() const { return m_nexthops; @@ -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 diff --git a/orchagent/nexthopkey.h b/orchagent/nexthopkey.h index 25461ce08b48..87c294415a26 100644 --- a/orchagent/nexthopkey.h +++ b/orchagent/nexthopkey.h @@ -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() { @@ -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) { @@ -76,6 +78,7 @@ struct NextHopKey alias = keys[1]; vni = static_cast(std::stoul(keys[2])); mac_address = keys[3]; + weight = 0; } const std::string to_string() const diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index 3cf490a5fc56..04907b3de911 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -338,6 +338,9 @@ bool RouteOrch::validnexthopinNextHopGroup(const NextHopKey &nexthop, uint32_t& vector 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); @@ -346,6 +349,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()); @@ -544,6 +554,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; @@ -569,6 +580,9 @@ void RouteOrch::doTask(Consumer& consumer) if (fvField(i) == "blackhole") blackhole = fvValue(i) == "true"; + + if (fvField(i) == "weight") + weights = fvValue(i); } vector ipv = tokenize(ips, ','); @@ -657,7 +671,7 @@ void RouteOrch::doTask(Consumer& consumer) nhg_str += ipv[i] + NH_DELIMITER + alsv[i]; } - nhg = NextHopGroupKey(nhg_str); + nhg = NextHopGroupKey(nhg_str, weights); } else @@ -1125,6 +1139,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 nhgm_attrs; @@ -1138,6 +1153,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()); diff --git a/tests/test_nhg.py b/tests/test_nhg.py index 485fc7661c1e..7a40b2a94212 100644 --- a/tests/test_nhg.py +++ b/tests/test_nhg.py @@ -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 @@ -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