[nhg]: Add support for weight in nexthop group member. (sonic-net#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 <zcai@juniper.net>

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<string> 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;
+}

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);
 };
 
 }

orchagent/nexthopgroupkey.h

@@ -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;
@@ -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

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<uint32_t>(std::stoul(keys[2]));
  mac_address = keys[3];
+ weight = 0;
  }
 
  const std::string to_string() const

orchagent/routeorch.cpp

@@ -338,6 +338,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);
@@ -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<string> 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<sai_attribute_t> 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());

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