From d3e3671486e5fde12805f6405c8d7513fc1c601a Mon Sep 17 00:00:00 2001 From: Kanji Nakano Date: Mon, 2 Oct 2023 18:42:39 +0000 Subject: [PATCH 01/15] nexthop group support Signed-off-by: Kanji Nakano --- fpmsyncd/fpmlink.cpp | 7 + fpmsyncd/routesync.cpp | 587 ++++++++++++++++++++++++++++++++++++----- fpmsyncd/routesync.h | 39 +++ 3 files changed, 565 insertions(+), 68 deletions(-) diff --git a/fpmsyncd/fpmlink.cpp b/fpmsyncd/fpmlink.cpp index 1ed888d292..7cbef6607a 100644 --- a/fpmsyncd/fpmlink.cpp +++ b/fpmsyncd/fpmlink.cpp @@ -281,6 +281,13 @@ void FpmLink::processFpmMessage(fpm_msg_hdr_t* hdr) /* EVPN Type5 Add route processing */ processRawMsg(nl_hdr); } +#ifdef HAVE_NEXTHOP_GROUP + else if(nl_hdr->nlmsg_type == RTM_NEWNEXTHOP || nl_hdr->nlmsg_type == RTM_DELNEXTHOP) + { + /* rtnl api dont support RTM_NEWNEXTHOP/RTM_DELNEXTHOP yet. Processing as raw message*/ + processRawMsg(nl_hdr); + } +#endif else { NetDispatcher::getInstance().onNetlinkMessage(msg); diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index cea63dc42f..1e2625fe2e 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -14,6 +14,10 @@ #include #include +#ifdef HAVE_NEXTHOP_GROUP +#include +#endif + using namespace std; using namespace swss; @@ -106,6 +110,8 @@ enum { ROUTE_ENCAP_SRV6_ENCAP_SRC_ADDR = 2, }; +#define MULTIPATH_NUM 256 //Same value used for FRR in SONiC + /* Returns name of the protocol passed number represents */ static string getProtocolString(int proto) { @@ -138,6 +144,9 @@ static decltype(auto) makeNlAddr(const T& ip) RouteSync::RouteSync(RedisPipeline *pipeline) : m_routeTable(pipeline, APP_ROUTE_TABLE_NAME, true), +#ifdef HAVE_NEXTHOP_GROUP + m_nexthop_groupTable(pipeline, APP_NEXTHOP_GROUP_TABLE_NAME, true), +#endif m_label_routeTable(pipeline, APP_LABEL_ROUTE_TABLE_NAME, true), m_vnet_routeTable(pipeline, APP_VNET_RT_TABLE_NAME, true), m_vnet_tunnelTable(pipeline, APP_VNET_RT_TUNNEL_TABLE_NAME, true), @@ -1444,11 +1453,26 @@ void RouteSync::onMsgRaw(struct nlmsghdr *h) if ((h->nlmsg_type != RTM_NEWROUTE) && (h->nlmsg_type != RTM_DELROUTE) && (h->nlmsg_type != RTM_NEWSRV6LOCALSID) - && (h->nlmsg_type != RTM_DELSRV6LOCALSID)) + && (h->nlmsg_type != RTM_DELSRV6LOCALSID) +#ifdef HAVE_NEXTHOP_GROUP + && (h->nlmsg_type != RTM_NEWNEXTHOP) + && (h->nlmsg_type != RTM_DELNEXTHOP) +#endif + ) return; + +#ifdef HAVE_NEXTHOP_GROUP + if(h->nlmsg_type == RTM_NEWNEXTHOP || h->nlmsg_type == RTM_DELNEXTHOP) + { + len = (int)(h->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg))); + } + else +#endif + { + len = (int)(h->nlmsg_len - NLMSG_LENGTH(sizeof(struct ndmsg))); + } /* Length validity. */ - len = (int)(h->nlmsg_len - NLMSG_LENGTH(sizeof(struct ndmsg))); if (len < 0) { SWSS_LOG_ERROR("%s: Message received from netlink is of a broken size %d %zu", @@ -1457,6 +1481,14 @@ void RouteSync::onMsgRaw(struct nlmsghdr *h) return; } +#ifdef HAVE_NEXTHOP_GROUP + if(h->nlmsg_type == RTM_NEWNEXTHOP || h->nlmsg_type == RTM_DELNEXTHOP) + { + onNextHopMsg(h, len); + return; + } +#endif + if ((h->nlmsg_type == RTM_NEWSRV6LOCALSID) || (h->nlmsg_type == RTM_DELSRV6LOCALSID)) { @@ -1481,8 +1513,6 @@ void RouteSync::onMsgRaw(struct nlmsghdr *h) onEvpnRouteMsg(h, len); break; } - - return; } void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj) @@ -1588,6 +1618,18 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) if (!warmRestartInProgress) { m_routeTable.del(destipprefix); +#ifdef HAVE_NEXTHOP_GROUP + const auto it = m_nh_routes.find(string(destipprefix)); + if(it == m_nh_routes.end()) + { + SWSS_LOG_INFO("Route not found: %s", destipprefix); + return; + } + + const NextHopGroupRoute& nh_route = it->second; + deleteNextHopGroup(nh_route.id); + m_nh_routes.erase(it); +#endif return; } else @@ -1637,88 +1679,178 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) return; } - struct nl_list_head *nhs = rtnl_route_get_nexthops(route_obj); - if (!nhs) - { - SWSS_LOG_INFO("Nexthop list is empty for %s", destipprefix); - return; - } - - /* Get nexthop lists */ + vector fvVector; string gw_list; string intf_list; string mpls_list; - getNextHopList(route_obj, gw_list, mpls_list, intf_list); - string weights = getNextHopWt(route_obj); + string nhg_id_key; - vector alsv = tokenize(intf_list, NHG_DELIMITER); - for (auto alias : alsv) +#ifdef HAVE_NEXTHOP_GROUP + uint32_t old_nhg_id = 0; + bool delete_route = false; + uint32_t nhg_id = rtnl_route_get_nh_id(route_obj); + if(nhg_id) { - /* - * An FRR behavior change from 7.2 to 7.5 makes FRR update default route to eth0 in interface - * up/down events. Skipping routes to eth0 or docker0 to avoid such behavior - */ - if (alias == "eth0" || alias == "docker0") - { - SWSS_LOG_DEBUG("Skip routes to eth0 or docker0: %s %s %s", - destipprefix, gw_list.c_str(), intf_list.c_str()); - // If intf_list has only this interface, that means all of the next hops of this route - // have been removed and the next hop on the eth0/docker0 has become the only next hop. - // In this case since we do not want the route with next hop on eth0/docker0, we return. - // But still we need to clear the route from the APPL_DB. Otherwise the APPL_DB and data - // path will be left with stale route entry - if(alsv.size() == 1) + bool use_nhg = false; + const auto itg = m_nh_groups.find(nhg_id); + if(itg == m_nh_groups.end()) + { + SWSS_LOG_DEBUG("NextHop group id %d not found. Dropping the route %s", nhg_id, destipprefix); + return; + } + NextHopGroup& nhg = itg->second; + if(hasIntfNextHop(nhg)) + { + string nexthops, ifnames, weights; + + getNextHopGroupFields(nhg, nexthops, ifnames, weights, rtnl_route_get_family(route_obj)); + FieldValueTuple gw("nexthop", nexthops.c_str()); + FieldValueTuple intf("ifname", ifnames.c_str()); + fvVector.push_back(gw); + fvVector.push_back(intf); + if(!weights.empty()) { - if (!warmRestartInProgress) - { - SWSS_LOG_NOTICE("RouteTable del msg for route with only one nh on eth0/docker0: %s %s %s %s", - destipprefix, gw_list.c_str(), intf_list.c_str(), mpls_list.c_str()); + FieldValueTuple wg("weight", weights.c_str()); + fvVector.push_back(wg); + } + use_nhg = false; + SWSS_LOG_DEBUG("NextHop group id %d has interface nexthop address. Filling the route table %s with nexthop and ifname", nhg_id, destipprefix); + } + else + { + nhg_id_key = getNextHopGroupKeyAsString(nhg_id); + FieldValueTuple nhg("nexthop_group", nhg_id_key.c_str()); + fvVector.push_back(nhg); + updateNextHopGroup(nhg_id); + use_nhg = false; + } - m_routeTable.del(destipprefix); - } - else + auto proto_num = rtnl_route_get_protocol(route_obj); + auto proto_str = getProtocolString(proto_num); + FieldValueTuple proto("protocol", proto_str); + fvVector.push_back(proto); + + const auto itr = m_nh_routes.find(destipprefix); + if(itr != m_nh_routes.end()) + { + NextHopGroupRoute &route = itr->second; + if(use_nhg != route.use_nhg) + { + delete_route = true; + } + old_nhg_id = route.id; + route.id = nhg_id; + route.use_nhg = use_nhg; + } + else + { + m_nh_routes.insert({string(destipprefix), NextHopGroupRoute{nhg_id, use_nhg}}); + } + } + else +#endif + { + struct nl_list_head *nhs = rtnl_route_get_nexthops(route_obj); + if (!nhs) + { + SWSS_LOG_INFO("Nexthop list is empty for %s", destipprefix); + return; + } + + /* Get nexthop lists */ + + getNextHopList(route_obj, gw_list, mpls_list, intf_list); + string weights = getNextHopWt(route_obj); + + vector alsv = tokenize(intf_list, NHG_DELIMITER); + for (auto alias : alsv) + { + /* + * An FRR behavior change from 7.2 to 7.5 makes FRR update default route to eth0 in interface + * up/down events. Skipping routes to eth0 or docker0 to avoid such behavior + */ + if (alias == "eth0" || alias == "docker0") + { + SWSS_LOG_DEBUG("Skip routes to eth0 or docker0: %s %s %s", + destipprefix, gw_list.c_str(), intf_list.c_str()); + // If intf_list has only this interface, that means all of the next hops of this route + // have been removed and the next hop on the eth0/docker0 has become the only next hop. + // In this case since we do not want the route with next hop on eth0/docker0, we return. + // But still we need to clear the route from the APPL_DB. Otherwise the APPL_DB and data + // path will be left with stale route entry + if(alsv.size() == 1) { - SWSS_LOG_NOTICE("Warm-Restart mode: Receiving delete msg for route with only nh on eth0/docker0: %s %s %s %s", - destipprefix, gw_list.c_str(), intf_list.c_str(), mpls_list.c_str()); - - vector fvVector; - const KeyOpFieldsValuesTuple kfv = std::make_tuple(destipprefix, - DEL_COMMAND, - fvVector); - m_warmStartHelper.insertRefreshMap(kfv); + if (!warmRestartInProgress) + { + SWSS_LOG_NOTICE("RouteTable del msg for route with only one nh on eth0/docker0: %s %s %s %s", + destipprefix, gw_list.c_str(), intf_list.c_str(), mpls_list.c_str()); + + m_routeTable.del(destipprefix); + } + else + { + SWSS_LOG_NOTICE("Warm-Restart mode: Receiving delete msg for route with only nh on eth0/docker0: %s %s %s %s", + destipprefix, gw_list.c_str(), intf_list.c_str(), mpls_list.c_str()); + + vector fvVector; + const KeyOpFieldsValuesTuple kfv = std::make_tuple(destipprefix, + DEL_COMMAND, + fvVector); + m_warmStartHelper.insertRefreshMap(kfv); + } } + return; } - return; } - } - auto proto_num = rtnl_route_get_protocol(route_obj); - auto proto_str = getProtocolString(proto_num); + auto proto_num = rtnl_route_get_protocol(route_obj); + auto proto_str = getProtocolString(proto_num); - vector fvVector; - FieldValueTuple proto("protocol", proto_str); - FieldValueTuple gw("nexthop", gw_list); - FieldValueTuple intf("ifname", intf_list); - fvVector.push_back(proto); - fvVector.push_back(gw); - fvVector.push_back(intf); - if (!mpls_list.empty()) - { - FieldValueTuple mpls_nh("mpls_nh", mpls_list); - fvVector.push_back(mpls_nh); - } - if (!weights.empty()) - { - FieldValueTuple wt("weight", weights); - fvVector.push_back(wt); + FieldValueTuple proto("protocol", proto_str); + FieldValueTuple gw("nexthop", gw_list); + FieldValueTuple intf("ifname", intf_list); + + fvVector.push_back(proto); + fvVector.push_back(gw); + fvVector.push_back(intf); + if (!mpls_list.empty()) + { + FieldValueTuple mpls_nh("mpls_nh", mpls_list); + fvVector.push_back(mpls_nh); + } + if (!weights.empty()) + { + FieldValueTuple wt("weight", weights); + fvVector.push_back(wt); + } } if (!warmRestartInProgress) { - m_routeTable.set(destipprefix, fvVector); - SWSS_LOG_DEBUG("RouteTable set msg: %s %s %s %s", destipprefix, +#ifdef HAVE_NEXTHOP_GROUP + if(nhg_id) + { + if(delete_route) + { + SWSS_LOG_DEBUG("Deleting route: %s. The nhg changed", destipprefix); + m_routeTable.del(destipprefix); + } + m_routeTable.set(destipprefix, fvVector); + SWSS_LOG_INFO("RouteTable set msg: %s group-id %s", destipprefix, nhg_id_key.c_str()); + if(old_nhg_id) + { + //Remove the old nhg and update + deleteNextHopGroup(old_nhg_id); + } + } + else +#endif + { + m_routeTable.set(destipprefix, fvVector); + SWSS_LOG_INFO("RouteTable set msg: %s %s %s %s", destipprefix, gw_list.c_str(), intf_list.c_str(), mpls_list.c_str()); + } } /* @@ -1737,7 +1869,152 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) } } -/* +/* + * Handle Nexthop msg + * @arg nlmsghdr Netlink message + */ +#ifdef HAVE_NEXTHOP_GROUP +void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) +{ + int nlmsg_type = h->nlmsg_type; + uint32_t id = 0; + unsigned char addr_family; + int32_t ifindex = -1, grp_count = 0; + string ifname; + struct nhmsg *nhm = NULL; + struct rtattr *tb[NHA_MAX + 1] = {}; + struct nexthop_grp grp[MULTIPATH_NUM]; + struct in_addr ipv4 = {0}; + struct in6_addr ipv6 = {0}; + char gateway[INET6_ADDRSTRLEN] = {0}; + char ifname_unknown[IFNAMSIZ] = "unknown"; + + SWSS_LOG_INFO("type %d len %d", nlmsg_type, len); + if ((nlmsg_type != RTM_NEWNEXTHOP) + && (nlmsg_type != RTM_DELNEXTHOP)) + { + return; + } + + nhm = (struct nhmsg *)NLMSG_DATA(h); + + netlink_parse_rtattr(tb, NHA_MAX, ((struct rtattr *)(((char *)(nhm)) + NLMSG_ALIGN(sizeof(struct nhmsg)))), len); + + if (!tb[NHA_ID]) { + SWSS_LOG_ERROR( + "Nexthop group without an ID received from the zebra"); + return; + } + + sendOffloadReply(h); + + /* We use the ID key'd nhg table for kernel updates */ + id = *((uint32_t *)RTA_DATA(tb[NHA_ID])); + + addr_family = nhm->nh_family; + + if (nlmsg_type == RTM_NEWNEXTHOP) + { + if(tb[NHA_GROUP]) + { + SWSS_LOG_INFO("New nexthop group message!"); + struct nexthop_grp *nha_grp = (struct nexthop_grp *)RTA_DATA(tb[NHA_GROUP]); + grp_count = (int)(RTA_PAYLOAD(tb[NHA_GROUP]) / sizeof(*nha_grp)); + + if(grp_count > MULTIPATH_NUM) + grp_count = MULTIPATH_NUM; + + for (int i = 0; i < grp_count; i++) { + grp[i].id = nha_grp[i].id; + /* + The minimum weight value is 1, but kernel store it as zero (https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/ip/iproute.c?h=v5.19.0#n1028). + Adding one to weight to write the right value to the database. + */ + grp[i].weight = nha_grp[i].weight + 1; + } + } + else + { + if (tb[NHA_GATEWAY]) + { + if (addr_family == AF_INET) + { + memcpy(&ipv4, (void *)RTA_DATA(tb[NHA_GATEWAY]), 4); + inet_ntop(AF_INET, &ipv4, gateway, INET_ADDRSTRLEN); + } + else if (addr_family == AF_INET6) + { + memcpy(&ipv6, (void *)RTA_DATA(tb[NHA_GATEWAY]), 16); + inet_ntop(AF_INET6, &ipv6, gateway, INET6_ADDRSTRLEN); + } + else + { + SWSS_LOG_ERROR( + "Unexpected nexthop address family"); + return; + } + } + + if(tb[NHA_OIF]) + { + ifindex = *((int32_t *)RTA_DATA(tb[NHA_OIF])); + char if_name[IFNAMSIZ] = {0}; + if (!getIfName(ifindex, if_name, IFNAMSIZ)) + { + strcpy(if_name, ifname_unknown); + } + ifname = string(if_name); + if (ifname == "eth0" || ifname == "docker0") + { + SWSS_LOG_DEBUG("Skip routes to inteface: %s id[%d]", ifname.c_str(), id); + return; + } + } + } + if(grp_count) + { + vector> group; + for(int i = 0; i < grp_count; i++) + { + group.push_back(std::make_pair(grp[i].id, grp[i].weight)); + } + auto it = m_nh_groups.find(id); + if(it != m_nh_groups.end()) + { + NextHopGroup &nhg = it->second; + nhg.group = group; + if(nhg.refcnt > 0) + { + updateNextHopGroupDb(nhg); + } + } + else + { + m_nh_groups.insert({id, NextHopGroup(id, group)}); + } + } + else + { + SWSS_LOG_INFO("Received: id[%d], if[%d/%s] address[%s]", id, ifindex, ifname.c_str(), gateway); + m_nh_groups.insert({id, NextHopGroup(id, string(gateway), ifname)}); + } + } + else if (nlmsg_type == RTM_DELNEXTHOP) + { + SWSS_LOG_INFO("NextHopGroup del event: %d", id); + string key = getNextHopGroupKeyAsString(id); + m_nexthop_groupTable.del(key.c_str()); //Force delete even if is not installed + if(m_nh_groups.find(id) != m_nh_groups.end()) + { + m_nh_groups.erase(id); + } + } + + return; +} +#endif + +/* * Handle label route * @arg nlmsg_type Netlink message type * @arg obj Netlink object @@ -2404,3 +2681,177 @@ void RouteSync::onWarmStartEnd(DBConnector& applStateDb) SWSS_LOG_NOTICE("Warm-Restart reconciliation processed."); } } + +/* + * Get nexthop group key as string + * @arg id next hop group id + * + * Return nexthop group key + */ +#ifdef HAVE_NEXTHOP_GROUP +const string RouteSync::getNextHopGroupKeyAsString(uint32_t id) const +{ + return string("ID") + to_string(id); +} + +/* + * update the nexthop group entry + * @arg nh_id nexthop group id + * + */ +void RouteSync::updateNextHopGroup(uint32_t nh_id) +{ + auto git = m_nh_groups.find(nh_id); + if(git == m_nh_groups.end()) + { + SWSS_LOG_INFO("Nexthop not found: %d", nh_id); + return; + } + + NextHopGroup& nhg = git->second; + + if(nhg.refcnt > 0) + { + //Nexthop group already installed + nhg.refcnt++; + return; + } + nhg.refcnt++; + updateNextHopGroupDb(nhg); +} + +/* + * delete the nexthop group entry + * @arg nh_id nexthop group id + * + */ +void RouteSync::deleteNextHopGroup(uint32_t nh_id) +{ + auto git = m_nh_groups.find(nh_id); + if(git == m_nh_groups.end()) + { + SWSS_LOG_INFO("Nexthop not found: %d", nh_id); + return; + } + + NextHopGroup& nhg = git->second; + + if(nhg.refcnt > 0 && --nhg.refcnt == 0) + { + string key = getNextHopGroupKeyAsString(nh_id); + m_nexthop_groupTable.del(key.c_str()); + SWSS_LOG_INFO("NextHopGroup table del: key [%s]", key.c_str()); + } + +} + +/* + * update the nexthop group table in database + * @arg nhg the nexthop group + * + */ +void RouteSync::updateNextHopGroupDb(const NextHopGroup& nhg) +{ + vector fvVector; + string nexthops; + string ifnames; + string weights; + string key = getNextHopGroupKeyAsString(nhg.id); + getNextHopGroupFields(nhg, nexthops, ifnames, weights); + + FieldValueTuple nh("nexthop", nexthops.c_str()); + FieldValueTuple ifname("ifname", ifnames.c_str()); + fvVector.push_back(nh); + fvVector.push_back(ifname); + if(!weights.empty()) + { + FieldValueTuple wg("weight", weights.c_str()); + fvVector.push_back(wg); + } + SWSS_LOG_INFO("NextHopGroup table set: key [%s] nexthop[%s] ifname[%s] weight[%s]", key.c_str(), nexthops.c_str(), ifnames.c_str(), weights.c_str()); + + //TODO: Take care of warm reboot + m_nexthop_groupTable.set(key.c_str(), fvVector); +} + +/* + * check if there are some interface route in the nexthop group + * @arg nhg the nexthop group + * + */ +bool RouteSync::hasIntfNextHop(const NextHopGroup& nhg) +{ + if(nhg.group.size() == 0 && nhg.nexthop.empty()) + { + return true; + } + else if(nhg.group.size() > 0) + { + for(const auto nh : nhg.group) + { + uint32_t id = nh.first; + auto itr = m_nh_groups.find(id); + if(itr == m_nh_groups.end()) + { + SWSS_LOG_INFO("NextHop group is incomplete: %d", nhg.id); + return false; + } + + NextHopGroup& nhgr = itr->second; + if(nhgr.nexthop.empty()) + { + return true; + } + } + } + return false; +} + +/* + * generate the database fields. + * @arg nhg the nexthop group + * + */ +void RouteSync::getNextHopGroupFields(const NextHopGroup& nhg, string& nexthops, string& ifnames, string& weights, uint8_t af /*= AF_INET*/) +{ + if(nhg.group.size() == 0) + { + if(!nhg.nexthop.empty()) + { + nexthops = nhg.nexthop; + } + else + { + nexthops = af == AF_INET ? "0.0.0.0" : "::"; + } + ifnames = nhg.intf; + } + else + { + int i = 0; + for(const auto nh : nhg.group) + { + uint32_t id = nh.first; + auto itr = m_nh_groups.find(id); + if(itr == m_nh_groups.end()) + { + SWSS_LOG_INFO("NextHop group is incomplete: %d", nhg.id); + return; + } + + NextHopGroup& nhgr = itr->second; + string weight = to_string(nh.second); + if(i) + { + nexthops += NHG_DELIMITER; + ifnames += NHG_DELIMITER; + weights += NHG_DELIMITER; + } + nexthops += nhgr.nexthop.empty() ? (af == AF_INET ? "0.0.0.0" : "::") : nhgr.nexthop; + ifnames += nhgr.intf; + weights += weight; + ++i; + } + } +} +#endif diff --git a/fpmsyncd/routesync.h b/fpmsyncd/routesync.h index fe67f6acfc..ee03af006d 100644 --- a/fpmsyncd/routesync.h +++ b/fpmsyncd/routesync.h @@ -9,9 +9,14 @@ #include "warmRestartHelper.h" #include #include +#include #include +#if (LINUX_VERSION_CODE > KERNEL_VERSION(5,3,0)) +#define HAVE_NEXTHOP_GROUP +#endif + // Add RTM_F_OFFLOAD define if it is not there. // Debian buster does not provide one but it is neccessary for compilation. #ifndef RTM_F_OFFLOAD @@ -26,6 +31,23 @@ extern void netlink_parse_rtattr(struct rtattr **tb, int max, struct rtattr *rta namespace swss { +#ifdef HAVE_NEXTHOP_GROUP +struct NextHopGroup { + uint32_t id; + vector> group; + string nexthop; + string intf; + uint32_t refcnt; + NextHopGroup(uint32_t id, const string& nexthop, const string& interface) : refcnt(0), id(id), nexthop(nexthop), intf(interface) {}; + NextHopGroup(uint32_t id, const vector>& group) : refcnt(0), id(id), group(group) {}; +}; + +struct NextHopGroupRoute { + uint32_t id; + bool use_nhg; +}; +#endif + /* Path to protocol name database provided by iproute2 */ constexpr auto DefaultRtProtoPath = "/etc/iproute2/rt_protos"; @@ -81,6 +103,12 @@ class RouteSync : public NetMsg ProducerStateTable m_srv6SidListTable; struct nl_cache *m_link_cache; struct nl_sock *m_nl_sock; +#ifdef HAVE_NEXTHOP_GROUP + /* nexthop group table */ + ProducerStateTable m_nexthop_groupTable; + map m_nh_groups; + map m_nh_routes; +#endif bool m_isSuppressionEnabled{false}; FpmInterface* m_fpmInterface {nullptr}; @@ -169,6 +197,17 @@ class RouteSync : public NetMsg uint16_t getEncapType(struct nlmsghdr *h); const char *mySidAction2Str(uint32_t action); +#ifdef HAVE_NEXTHOP_GROUP + /* Handle Nexthop message */ + void onNextHopMsg(struct nlmsghdr *h, int len); + /* Get next hop group key */ + const string getNextHopGroupKeyAsString(uint32_t id) const; + void updateNextHopGroup(uint32_t nh_id); + void deleteNextHopGroup(uint32_t nh_id); + void updateNextHopGroupDb(const NextHopGroup& nhg); + bool hasIntfNextHop(const NextHopGroup& nhg); + void getNextHopGroupFields(const NextHopGroup& nhg, string& nexthops, string& ifnames, string& weights, uint8_t af = AF_INET); +#endif }; } From 4874bc5f00ce2f6f0c57e016d3c7dc4e6a9b1196 Mon Sep 17 00:00:00 2001 From: Kanji Nakano Date: Thu, 9 Nov 2023 19:59:57 +0000 Subject: [PATCH 02/15] update to not cache in fpmsyncd Signed-off-by: Kanji Nakano --- fpmsyncd/routesync.cpp | 116 +++++------------------------------------ fpmsyncd/routesync.h | 13 ++--- 2 files changed, 17 insertions(+), 112 deletions(-) diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index 1e2625fe2e..7175d383eb 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -1618,18 +1618,6 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) if (!warmRestartInProgress) { m_routeTable.del(destipprefix); -#ifdef HAVE_NEXTHOP_GROUP - const auto it = m_nh_routes.find(string(destipprefix)); - if(it == m_nh_routes.end()) - { - SWSS_LOG_INFO("Route not found: %s", destipprefix); - return; - } - - const NextHopGroupRoute& nh_route = it->second; - deleteNextHopGroup(nh_route.id); - m_nh_routes.erase(it); -#endif return; } else @@ -1683,15 +1671,12 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) string gw_list; string intf_list; string mpls_list; - string nhg_id_key; #ifdef HAVE_NEXTHOP_GROUP - uint32_t old_nhg_id = 0; - bool delete_route = false; + string nhg_id_key; uint32_t nhg_id = rtnl_route_get_nh_id(route_obj); if(nhg_id) { - bool use_nhg = false; const auto itg = m_nh_groups.find(nhg_id); if(itg == m_nh_groups.end()) { @@ -1699,8 +1684,9 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) return; } NextHopGroup& nhg = itg->second; - if(hasIntfNextHop(nhg)) + if(nhg.group.size() == 0) { + //Using route-table only for single next-hop string nexthops, ifnames, weights; getNextHopGroupFields(nhg, nexthops, ifnames, weights, rtnl_route_get_family(route_obj)); @@ -1708,13 +1694,7 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) FieldValueTuple intf("ifname", ifnames.c_str()); fvVector.push_back(gw); fvVector.push_back(intf); - if(!weights.empty()) - { - FieldValueTuple wg("weight", weights.c_str()); - fvVector.push_back(wg); - } - use_nhg = false; - SWSS_LOG_DEBUG("NextHop group id %d has interface nexthop address. Filling the route table %s with nexthop and ifname", nhg_id, destipprefix); + SWSS_LOG_DEBUG("NextHop group id %d is a single nexthop address. Filling the route table %s with nexthop and ifname", nhg_id, destipprefix); } else { @@ -1722,7 +1702,6 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) FieldValueTuple nhg("nexthop_group", nhg_id_key.c_str()); fvVector.push_back(nhg); updateNextHopGroup(nhg_id); - use_nhg = false; } auto proto_num = rtnl_route_get_protocol(route_obj); @@ -1730,22 +1709,6 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) FieldValueTuple proto("protocol", proto_str); fvVector.push_back(proto); - const auto itr = m_nh_routes.find(destipprefix); - if(itr != m_nh_routes.end()) - { - NextHopGroupRoute &route = itr->second; - if(use_nhg != route.use_nhg) - { - delete_route = true; - } - old_nhg_id = route.id; - route.id = nhg_id; - route.use_nhg = use_nhg; - } - else - { - m_nh_routes.insert({string(destipprefix), NextHopGroupRoute{nhg_id, use_nhg}}); - } } else #endif @@ -1831,18 +1794,8 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) #ifdef HAVE_NEXTHOP_GROUP if(nhg_id) { - if(delete_route) - { - SWSS_LOG_DEBUG("Deleting route: %s. The nhg changed", destipprefix); - m_routeTable.del(destipprefix); - } m_routeTable.set(destipprefix, fvVector); - SWSS_LOG_INFO("RouteTable set msg: %s group-id %s", destipprefix, nhg_id_key.c_str()); - if(old_nhg_id) - { - //Remove the old nhg and update - deleteNextHopGroup(old_nhg_id); - } + SWSS_LOG_INFO("RouteTable set msg: %s %d ", destipprefix, nhg_id); } else #endif @@ -1906,8 +1859,6 @@ void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) return; } - sendOffloadReply(h); - /* We use the ID key'd nhg table for kernel updates */ id = *((uint32_t *)RTA_DATA(tb[NHA_ID])); @@ -1983,7 +1934,7 @@ void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) { NextHopGroup &nhg = it->second; nhg.group = group; - if(nhg.refcnt > 0) + if(nhg.installed) { updateNextHopGroupDb(nhg); } @@ -1995,19 +1946,14 @@ void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) } else { - SWSS_LOG_INFO("Received: id[%d], if[%d/%s] address[%s]", id, ifindex, ifname.c_str(), gateway); + SWSS_LOG_DEBUG("Received: id[%d], if[%d/%s] address[%s]", id, ifindex, ifname.c_str(), gateway); m_nh_groups.insert({id, NextHopGroup(id, string(gateway), ifname)}); } } else if (nlmsg_type == RTM_DELNEXTHOP) { - SWSS_LOG_INFO("NextHopGroup del event: %d", id); - string key = getNextHopGroupKeyAsString(id); - m_nexthop_groupTable.del(key.c_str()); //Force delete even if is not installed - if(m_nh_groups.find(id) != m_nh_groups.end()) - { - m_nh_groups.erase(id); - } + SWSS_LOG_DEBUG("NextHopGroup del event: %d", id); + deleteNextHopGroup(id); } return; @@ -2710,13 +2656,12 @@ void RouteSync::updateNextHopGroup(uint32_t nh_id) NextHopGroup& nhg = git->second; - if(nhg.refcnt > 0) + if(nhg.installed) { //Nexthop group already installed - nhg.refcnt++; return; } - nhg.refcnt++; + nhg.installed = true; updateNextHopGroupDb(nhg); } @@ -2736,13 +2681,13 @@ void RouteSync::deleteNextHopGroup(uint32_t nh_id) NextHopGroup& nhg = git->second; - if(nhg.refcnt > 0 && --nhg.refcnt == 0) + if(nhg.installed) { string key = getNextHopGroupKeyAsString(nh_id); m_nexthop_groupTable.del(key.c_str()); - SWSS_LOG_INFO("NextHopGroup table del: key [%s]", key.c_str()); + SWSS_LOG_DEBUG("NextHopGroup table del: key [%s]", key.c_str()); } - + m_nh_groups.erase(git); } /* @@ -2774,39 +2719,6 @@ void RouteSync::updateNextHopGroupDb(const NextHopGroup& nhg) m_nexthop_groupTable.set(key.c_str(), fvVector); } -/* - * check if there are some interface route in the nexthop group - * @arg nhg the nexthop group - * - */ -bool RouteSync::hasIntfNextHop(const NextHopGroup& nhg) -{ - if(nhg.group.size() == 0 && nhg.nexthop.empty()) - { - return true; - } - else if(nhg.group.size() > 0) - { - for(const auto nh : nhg.group) - { - uint32_t id = nh.first; - auto itr = m_nh_groups.find(id); - if(itr == m_nh_groups.end()) - { - SWSS_LOG_INFO("NextHop group is incomplete: %d", nhg.id); - return false; - } - - NextHopGroup& nhgr = itr->second; - if(nhgr.nexthop.empty()) - { - return true; - } - } - } - return false; -} - /* * generate the database fields. * @arg nhg the nexthop group diff --git a/fpmsyncd/routesync.h b/fpmsyncd/routesync.h index ee03af006d..ab517d2fe9 100644 --- a/fpmsyncd/routesync.h +++ b/fpmsyncd/routesync.h @@ -37,14 +37,9 @@ struct NextHopGroup { vector> group; string nexthop; string intf; - uint32_t refcnt; - NextHopGroup(uint32_t id, const string& nexthop, const string& interface) : refcnt(0), id(id), nexthop(nexthop), intf(interface) {}; - NextHopGroup(uint32_t id, const vector>& group) : refcnt(0), id(id), group(group) {}; -}; - -struct NextHopGroupRoute { - uint32_t id; - bool use_nhg; + bool installed; + NextHopGroup(uint32_t id, const string& nexthop, const string& interface) : installed(false), id(id), nexthop(nexthop), intf(interface) {}; + NextHopGroup(uint32_t id, const vector>& group) : installed(false), id(id), group(group) {}; }; #endif @@ -107,7 +102,6 @@ class RouteSync : public NetMsg /* nexthop group table */ ProducerStateTable m_nexthop_groupTable; map m_nh_groups; - map m_nh_routes; #endif bool m_isSuppressionEnabled{false}; @@ -205,7 +199,6 @@ class RouteSync : public NetMsg void updateNextHopGroup(uint32_t nh_id); void deleteNextHopGroup(uint32_t nh_id); void updateNextHopGroupDb(const NextHopGroup& nhg); - bool hasIntfNextHop(const NextHopGroup& nhg); void getNextHopGroupFields(const NextHopGroup& nhg, string& nexthops, string& ifnames, string& weights, uint8_t af = AF_INET); #endif }; From 7a2f1281c699bed7b0d4802c9091ca6438b8756f Mon Sep 17 00:00:00 2001 From: Kanji Nakano Date: Mon, 30 Sep 2024 20:00:58 +0900 Subject: [PATCH 03/15] update nexthop group support Signed-off-by: Kanji Nakano --- fpmsyncd/fpmlink.cpp | 4 +- fpmsyncd/routesync.cpp | 110 +++++++++++++++++------------------------ fpmsyncd/routesync.h | 13 +---- 3 files changed, 48 insertions(+), 79 deletions(-) diff --git a/fpmsyncd/fpmlink.cpp b/fpmsyncd/fpmlink.cpp index 7cbef6607a..d90ea8aed6 100644 --- a/fpmsyncd/fpmlink.cpp +++ b/fpmsyncd/fpmlink.cpp @@ -281,13 +281,11 @@ void FpmLink::processFpmMessage(fpm_msg_hdr_t* hdr) /* EVPN Type5 Add route processing */ processRawMsg(nl_hdr); } -#ifdef HAVE_NEXTHOP_GROUP else if(nl_hdr->nlmsg_type == RTM_NEWNEXTHOP || nl_hdr->nlmsg_type == RTM_DELNEXTHOP) { /* rtnl api dont support RTM_NEWNEXTHOP/RTM_DELNEXTHOP yet. Processing as raw message*/ processRawMsg(nl_hdr); } -#endif else { NetDispatcher::getInstance().onNetlinkMessage(msg); @@ -327,4 +325,4 @@ bool FpmLink::send(nlmsghdr* nl_hdr) } return true; -} +} \ No newline at end of file diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index 7175d383eb..f65daf2b17 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -13,10 +13,7 @@ #include "converter.h" #include #include - -#ifdef HAVE_NEXTHOP_GROUP #include -#endif using namespace std; using namespace swss; @@ -110,7 +107,7 @@ enum { ROUTE_ENCAP_SRV6_ENCAP_SRC_ADDR = 2, }; -#define MULTIPATH_NUM 256 //Same value used for FRR in SONiC +#define MAX_MULTIPATH_NUM 256 //Same value used for FRR in SONiC /* Returns name of the protocol passed number represents */ static string getProtocolString(int proto) @@ -144,9 +141,7 @@ static decltype(auto) makeNlAddr(const T& ip) RouteSync::RouteSync(RedisPipeline *pipeline) : m_routeTable(pipeline, APP_ROUTE_TABLE_NAME, true), -#ifdef HAVE_NEXTHOP_GROUP m_nexthop_groupTable(pipeline, APP_NEXTHOP_GROUP_TABLE_NAME, true), -#endif m_label_routeTable(pipeline, APP_LABEL_ROUTE_TABLE_NAME, true), m_vnet_routeTable(pipeline, APP_VNET_RT_TABLE_NAME, true), m_vnet_tunnelTable(pipeline, APP_VNET_RT_TUNNEL_TABLE_NAME, true), @@ -1454,21 +1449,17 @@ void RouteSync::onMsgRaw(struct nlmsghdr *h) && (h->nlmsg_type != RTM_DELROUTE) && (h->nlmsg_type != RTM_NEWSRV6LOCALSID) && (h->nlmsg_type != RTM_DELSRV6LOCALSID) -#ifdef HAVE_NEXTHOP_GROUP && (h->nlmsg_type != RTM_NEWNEXTHOP) && (h->nlmsg_type != RTM_DELNEXTHOP) -#endif ) return; -#ifdef HAVE_NEXTHOP_GROUP if(h->nlmsg_type == RTM_NEWNEXTHOP || h->nlmsg_type == RTM_DELNEXTHOP) { len = (int)(h->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg))); } else -#endif { len = (int)(h->nlmsg_len - NLMSG_LENGTH(sizeof(struct ndmsg))); } @@ -1481,13 +1472,11 @@ void RouteSync::onMsgRaw(struct nlmsghdr *h) return; } -#ifdef HAVE_NEXTHOP_GROUP if(h->nlmsg_type == RTM_NEWNEXTHOP || h->nlmsg_type == RTM_DELNEXTHOP) { onNextHopMsg(h, len); return; } -#endif if ((h->nlmsg_type == RTM_NEWSRV6LOCALSID) || (h->nlmsg_type == RTM_DELSRV6LOCALSID)) @@ -1672,7 +1661,6 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) string intf_list; string mpls_list; -#ifdef HAVE_NEXTHOP_GROUP string nhg_id_key; uint32_t nhg_id = rtnl_route_get_nh_id(route_obj); if(nhg_id) @@ -1711,7 +1699,6 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) } else -#endif { struct nl_list_head *nhs = rtnl_route_get_nexthops(route_obj); if (!nhs) @@ -1726,45 +1713,51 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) string weights = getNextHopWt(route_obj); vector alsv = tokenize(intf_list, NHG_DELIMITER); - for (auto alias : alsv) + + if (alsv.size() == 1) { - /* - * An FRR behavior change from 7.2 to 7.5 makes FRR update default route to eth0 in interface - * up/down events. Skipping routes to eth0 or docker0 to avoid such behavior - */ - if (alias == "eth0" || alias == "docker0") + if (alsv[0] == "eth0" || alsv[0] == "docker0") { SWSS_LOG_DEBUG("Skip routes to eth0 or docker0: %s %s %s", - destipprefix, gw_list.c_str(), intf_list.c_str()); - // If intf_list has only this interface, that means all of the next hops of this route - // have been removed and the next hop on the eth0/docker0 has become the only next hop. - // In this case since we do not want the route with next hop on eth0/docker0, we return. - // But still we need to clear the route from the APPL_DB. Otherwise the APPL_DB and data - // path will be left with stale route entry - if(alsv.size() == 1) + destipprefix, gw_list.c_str(), intf_list.c_str()); + + if (!warmRestartInProgress) { - if (!warmRestartInProgress) - { - SWSS_LOG_NOTICE("RouteTable del msg for route with only one nh on eth0/docker0: %s %s %s %s", - destipprefix, gw_list.c_str(), intf_list.c_str(), mpls_list.c_str()); + SWSS_LOG_NOTICE("RouteTable del msg for route with only one nh on eth0/docker0: %s %s %s %s", + destipprefix, gw_list.c_str(), intf_list.c_str(), mpls_list.c_str()); - m_routeTable.del(destipprefix); - } - else - { - SWSS_LOG_NOTICE("Warm-Restart mode: Receiving delete msg for route with only nh on eth0/docker0: %s %s %s %s", - destipprefix, gw_list.c_str(), intf_list.c_str(), mpls_list.c_str()); - - vector fvVector; - const KeyOpFieldsValuesTuple kfv = std::make_tuple(destipprefix, - DEL_COMMAND, - fvVector); - m_warmStartHelper.insertRefreshMap(kfv); - } + m_routeTable.del(destipprefix); + } + else + { + SWSS_LOG_NOTICE("Warm-Restart mode: Receiving delete msg for route with only nh on eth0/docker0: %s %s %s %s", + destipprefix, gw_list.c_str(), intf_list.c_str(), mpls_list.c_str()); + + vector fvVector; + const KeyOpFieldsValuesTuple kfv = std::make_tuple(destipprefix, + DEL_COMMAND, + fvVector); + m_warmStartHelper.insertRefreshMap(kfv); } return; } } + else + { + for (auto alias : alsv) + { + /* + * A change in FRR behavior from version 7.2 to 7.5 causes the default route to be updated to eth0 + * during interface up/down events. This skips routes to eth0 or docker0 to avoid such behavior. + */ + if (alias == "eth0" || alias == "docker0") + { + SWSS_LOG_DEBUG("Skip routes to eth0 or docker0: %s %s %s", + destipprefix, gw_list.c_str(), intf_list.c_str()); + continue; + } + } + } auto proto_num = rtnl_route_get_protocol(route_obj); auto proto_str = getProtocolString(proto_num); @@ -1791,14 +1784,12 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) if (!warmRestartInProgress) { -#ifdef HAVE_NEXTHOP_GROUP if(nhg_id) { m_routeTable.set(destipprefix, fvVector); SWSS_LOG_INFO("RouteTable set msg: %s %d ", destipprefix, nhg_id); } else -#endif { m_routeTable.set(destipprefix, fvVector); SWSS_LOG_INFO("RouteTable set msg: %s %s %s %s", destipprefix, @@ -1826,7 +1817,6 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) * Handle Nexthop msg * @arg nlmsghdr Netlink message */ -#ifdef HAVE_NEXTHOP_GROUP void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) { int nlmsg_type = h->nlmsg_type; @@ -1836,19 +1826,12 @@ void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) string ifname; struct nhmsg *nhm = NULL; struct rtattr *tb[NHA_MAX + 1] = {}; - struct nexthop_grp grp[MULTIPATH_NUM]; + struct nexthop_grp grp[MAX_MULTIPATH_NUM]; struct in_addr ipv4 = {0}; struct in6_addr ipv6 = {0}; char gateway[INET6_ADDRSTRLEN] = {0}; char ifname_unknown[IFNAMSIZ] = "unknown"; - SWSS_LOG_INFO("type %d len %d", nlmsg_type, len); - if ((nlmsg_type != RTM_NEWNEXTHOP) - && (nlmsg_type != RTM_DELNEXTHOP)) - { - return; - } - nhm = (struct nhmsg *)NLMSG_DATA(h); netlink_parse_rtattr(tb, NHA_MAX, ((struct rtattr *)(((char *)(nhm)) + NLMSG_ALIGN(sizeof(struct nhmsg)))), len); @@ -1872,8 +1855,8 @@ void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) struct nexthop_grp *nha_grp = (struct nexthop_grp *)RTA_DATA(tb[NHA_GROUP]); grp_count = (int)(RTA_PAYLOAD(tb[NHA_GROUP]) / sizeof(*nha_grp)); - if(grp_count > MULTIPATH_NUM) - grp_count = MULTIPATH_NUM; + if(grp_count > MAX_MULTIPATH_NUM) + grp_count = MAX_MULTIPATH_NUM; for (int i = 0; i < grp_count; i++) { grp[i].id = nha_grp[i].id; @@ -1958,7 +1941,6 @@ void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) return; } -#endif /* * Handle label route @@ -2634,7 +2616,6 @@ void RouteSync::onWarmStartEnd(DBConnector& applStateDb) * * Return nexthop group key */ -#ifdef HAVE_NEXTHOP_GROUP const string RouteSync::getNextHopGroupKeyAsString(uint32_t id) const { return string("ID") + to_string(id); @@ -2650,7 +2631,7 @@ void RouteSync::updateNextHopGroup(uint32_t nh_id) auto git = m_nh_groups.find(nh_id); if(git == m_nh_groups.end()) { - SWSS_LOG_INFO("Nexthop not found: %d", nh_id); + SWSS_LOG_ERROR("Nexthop not found: %d", nh_id); return; } @@ -2675,7 +2656,7 @@ void RouteSync::deleteNextHopGroup(uint32_t nh_id) auto git = m_nh_groups.find(nh_id); if(git == m_nh_groups.end()) { - SWSS_LOG_INFO("Nexthop not found: %d", nh_id); + SWSS_LOG_ERROR("Nexthop not found: %d", nh_id); return; } @@ -2741,13 +2722,13 @@ void RouteSync::getNextHopGroupFields(const NextHopGroup& nhg, string& nexthops, else { int i = 0; - for(const auto nh : nhg.group) + for(const auto& nh : nhg.group) { uint32_t id = nh.first; auto itr = m_nh_groups.find(id); if(itr == m_nh_groups.end()) { - SWSS_LOG_INFO("NextHop group is incomplete: %d", nhg.id); + SWSS_LOG_ERROR("NextHop group is incomplete: %d", nhg.id); return; } @@ -2765,5 +2746,4 @@ void RouteSync::getNextHopGroupFields(const NextHopGroup& nhg, string& nexthops, ++i; } } -} -#endif +} \ No newline at end of file diff --git a/fpmsyncd/routesync.h b/fpmsyncd/routesync.h index ab517d2fe9..312bca9fbb 100644 --- a/fpmsyncd/routesync.h +++ b/fpmsyncd/routesync.h @@ -13,10 +13,6 @@ #include -#if (LINUX_VERSION_CODE > KERNEL_VERSION(5,3,0)) -#define HAVE_NEXTHOP_GROUP -#endif - // Add RTM_F_OFFLOAD define if it is not there. // Debian buster does not provide one but it is neccessary for compilation. #ifndef RTM_F_OFFLOAD @@ -31,7 +27,6 @@ extern void netlink_parse_rtattr(struct rtattr **tb, int max, struct rtattr *rta namespace swss { -#ifdef HAVE_NEXTHOP_GROUP struct NextHopGroup { uint32_t id; vector> group; @@ -41,7 +36,6 @@ struct NextHopGroup { NextHopGroup(uint32_t id, const string& nexthop, const string& interface) : installed(false), id(id), nexthop(nexthop), intf(interface) {}; NextHopGroup(uint32_t id, const vector>& group) : installed(false), id(id), group(group) {}; }; -#endif /* Path to protocol name database provided by iproute2 */ constexpr auto DefaultRtProtoPath = "/etc/iproute2/rt_protos"; @@ -98,11 +92,9 @@ class RouteSync : public NetMsg ProducerStateTable m_srv6SidListTable; struct nl_cache *m_link_cache; struct nl_sock *m_nl_sock; -#ifdef HAVE_NEXTHOP_GROUP /* nexthop group table */ ProducerStateTable m_nexthop_groupTable; map m_nh_groups; -#endif bool m_isSuppressionEnabled{false}; FpmInterface* m_fpmInterface {nullptr}; @@ -191,7 +183,7 @@ class RouteSync : public NetMsg uint16_t getEncapType(struct nlmsghdr *h); const char *mySidAction2Str(uint32_t action); -#ifdef HAVE_NEXTHOP_GROUP + /* Handle Nexthop message */ void onNextHopMsg(struct nlmsghdr *h, int len); /* Get next hop group key */ @@ -200,9 +192,8 @@ class RouteSync : public NetMsg void deleteNextHopGroup(uint32_t nh_id); void updateNextHopGroupDb(const NextHopGroup& nhg); void getNextHopGroupFields(const NextHopGroup& nhg, string& nexthops, string& ifnames, string& weights, uint8_t af = AF_INET); -#endif }; } -#endif +#endif \ No newline at end of file From 2b464e08db878dfd175bd1c433724921839a9eb9 Mon Sep 17 00:00:00 2001 From: Kanji Nakano Date: Thu, 10 Oct 2024 05:22:55 +0900 Subject: [PATCH 04/15] update nexthop group support Signed-off-by: Kanji Nakano --- fpmsyncd/routesync.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index f65daf2b17..8b5435ad18 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -1454,7 +1454,6 @@ void RouteSync::onMsgRaw(struct nlmsghdr *h) ) return; - if(h->nlmsg_type == RTM_NEWNEXTHOP || h->nlmsg_type == RTM_DELNEXTHOP) { len = (int)(h->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg))); @@ -1834,8 +1833,12 @@ void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) nhm = (struct nhmsg *)NLMSG_DATA(h); - netlink_parse_rtattr(tb, NHA_MAX, ((struct rtattr *)(((char *)(nhm)) + NLMSG_ALIGN(sizeof(struct nhmsg)))), len); + struct rtattr* rta; + char* nhm_ptr = (char *)(nhm) + NLMSG_ALIGN(sizeof(struct nhmsg)); + memcpy(&rta, nhm_ptr, sizeof(struct rtattr)); + netlink_parse_rtattr(tb, NHA_MAX, rta, len); + if (!tb[NHA_ID]) { SWSS_LOG_ERROR( "Nexthop group without an ID received from the zebra"); From 1cc00d1071498ce35f1d3bc943b96ec1c45932fa Mon Sep 17 00:00:00 2001 From: Yijiao Qin Date: Tue, 3 Dec 2024 18:24:51 -0800 Subject: [PATCH 05/15] fix --- fpmsyncd/routesync.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index 8b5435ad18..6b6247ebfe 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -35,6 +35,11 @@ using namespace swss; ((struct rtattr *)(((char *)(r)) + NLMSG_ALIGN(sizeof(struct ndmsg)))) #endif +#ifndef NHA__RTA +#define NHA_RTA(r) \ + ((struct rtattr *)(((char *)(r)) + NLMSG_ALIGN(sizeof(struct nhmsg)))) +#endif + #define VXLAN_VNI 0 #define VXLAN_RMAC 1 #define NH_ENCAP_VXLAN 100 @@ -1833,12 +1838,13 @@ void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) nhm = (struct nhmsg *)NLMSG_DATA(h); - struct rtattr* rta; - char* nhm_ptr = (char *)(nhm) + NLMSG_ALIGN(sizeof(struct nhmsg)); + #pragma GCC diagnostic push + #pragma GCC diagnostic ignored "-Wcast-align" + struct rtattr* rta = NHA_RTA(nhm); + #pragma GCC diagnostic pop - memcpy(&rta, nhm_ptr, sizeof(struct rtattr)); netlink_parse_rtattr(tb, NHA_MAX, rta, len); - + if (!tb[NHA_ID]) { SWSS_LOG_ERROR( "Nexthop group without an ID received from the zebra"); From d35b41b044aa49f37c706b517b339997da69f101 Mon Sep 17 00:00:00 2001 From: Yijiao Qin Date: Wed, 20 Nov 2024 15:50:33 -0800 Subject: [PATCH 06/15] add testcases --- fpmsyncd/routesync.h | 2 +- tests/mock_tests/fpmsyncd/test_routesync.cpp | 151 ++++++++++++++++++- 2 files changed, 149 insertions(+), 4 deletions(-) diff --git a/fpmsyncd/routesync.h b/fpmsyncd/routesync.h index 312bca9fbb..f009d50110 100644 --- a/fpmsyncd/routesync.h +++ b/fpmsyncd/routesync.h @@ -137,7 +137,7 @@ class RouteSync : public NetMsg void onVnetRouteMsg(int nlmsg_type, struct nl_object *obj, string vnet); /* Get interface name based on interface index */ - bool getIfName(int if_index, char *if_name, size_t name_len); + virtual bool getIfName(int if_index, char *if_name, size_t name_len); /* Get interface if_index based on interface name */ rtnl_link* getLinkByName(const char *name); diff --git a/tests/mock_tests/fpmsyncd/test_routesync.cpp b/tests/mock_tests/fpmsyncd/test_routesync.cpp index b1c23aca85..8d2aaa3592 100644 --- a/tests/mock_tests/fpmsyncd/test_routesync.cpp +++ b/tests/mock_tests/fpmsyncd/test_routesync.cpp @@ -1,5 +1,5 @@ #include "redisutility.h" - +#include "ut_helpers_fpmsyncd.h" #include #include #include "mock_table.h" @@ -7,7 +7,17 @@ #include "fpmsyncd/routesync.h" #undef private +#include +#include +#include +#include +#include + +#include + using namespace swss; +using namespace testing; + #define MAX_PAYLOAD 1024 using ::testing::_; @@ -28,6 +38,7 @@ class MockRouteSync : public RouteSync rtattr *[], std::string&, std::string& , std::string&, std::string&), (override)); + MOCK_METHOD(bool, getIfName, (int, char *, size_t), (override)); }; class MockFpm : public FpmInterface { @@ -224,6 +235,7 @@ TEST_F(FpmSyncdResponseTest, testEvpn) return true; }); m_mockRouteSync.onMsgRaw(nlh); + vector keys; vector fieldValues; app_route_table.getKeys(keys); @@ -237,15 +249,148 @@ TEST_F(FpmSyncdResponseTest, testEvpn) TEST_F(FpmSyncdResponseTest, testSendOffloadReply) { - rt_build_ret = 1; rtnl_route* routeObject{}; - ASSERT_EQ(m_routeSync.sendOffloadReply(routeObject), false); rt_build_ret = 0; nlmsg_alloc_ret = false; ASSERT_EQ(m_routeSync.sendOffloadReply(routeObject), false); nlmsg_alloc_ret = true; +} + +struct nlmsghdr* createNewNextHopMsgHdr(int32_t ifindex, const char* gateway, uint32_t id) { + struct nlmsghdr *nlh = (struct nlmsghdr *)malloc(NLMSG_SPACE(MAX_PAYLOAD)); + memset(nlh, 0, NLMSG_SPACE(MAX_PAYLOAD)); + + // 设置基本header + nlh->nlmsg_type = RTM_NEWNEXTHOP; + nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_REPLACE; + nlh->nlmsg_len = NLMSG_LENGTH(sizeof(struct nhmsg)); + + printf("After header setup - nlmsg_len: %d\n", nlh->nlmsg_len); + + // 设置nhmsg + struct nhmsg *nhm = (struct nhmsg *)NLMSG_DATA(nlh); + nhm->nh_family = AF_INET; + + // 先添加 NHA_ID + struct rtattr *rta = (struct rtattr *)((char *)nlh + NLMSG_ALIGN(nlh->nlmsg_len)); + rta->rta_type = NHA_ID; + rta->rta_len = RTA_LENGTH(sizeof(uint32_t)); + *(uint32_t *)RTA_DATA(rta) = id; + nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + RTA_ALIGN(rta->rta_len); + + printf("After NHA_ID - nlmsg_len: %d\n", nlh->nlmsg_len); + + // 添加 NHA_GATEWAY + rta = (struct rtattr *)((char *)nlh + NLMSG_ALIGN(nlh->nlmsg_len)); + struct in_addr gw_addr; + inet_pton(AF_INET, gateway, &gw_addr); + rta->rta_type = NHA_GATEWAY; + rta->rta_len = RTA_LENGTH(sizeof(struct in_addr)); + memcpy(RTA_DATA(rta), &gw_addr, sizeof(struct in_addr)); + nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + RTA_ALIGN(rta->rta_len); + + printf("After NHA_GATEWAY - nlmsg_len: %d\n", nlh->nlmsg_len); + + // 添加 NHA_OIF + rta = (struct rtattr *)((char *)nlh + NLMSG_ALIGN(nlh->nlmsg_len)); + rta->rta_type = NHA_OIF; + rta->rta_len = RTA_LENGTH(sizeof(int32_t)); + *(int32_t *)RTA_DATA(rta) = ifindex; + nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + RTA_ALIGN(rta->rta_len); + + printf("After NHA_OIF - final nlmsg_len: %d\n", nlh->nlmsg_len); + + return nlh; +} + +void dump_nexthop_msg(struct nlmsghdr *nlh) { + printf("\nDumping nexthop message:\n"); + printf("nlmsg_len: %d\n", nlh->nlmsg_len); + printf("nlmsg_type: %d\n", nlh->nlmsg_type); + + struct nhmsg *nhm = (struct nhmsg *)NLMSG_DATA(nlh); + printf("nh_family: %d\n", nhm->nh_family); + + struct rtattr *rta = (struct rtattr *)((char *)nhm + NLMSG_ALIGN(sizeof(*nhm))); + int len = nlh->nlmsg_len - (int)NLMSG_LENGTH(sizeof(*nhm)); + + while (RTA_OK(rta, len)) { + printf("Attribute type: %d, len: %d\n", rta->rta_type, rta->rta_len); + if (rta->rta_type == NHA_OIF) { + printf(" OIF value: %d\n", *(int32_t *)RTA_DATA(rta)); + } + rta = RTA_NEXT(rta, len); + } +} +TEST_F(FpmSyncdResponseTest, TestNoNHAId) +{ + struct nlmsghdr *nlh = (struct nlmsghdr *)malloc(NLMSG_SPACE(MAX_PAYLOAD)); + memset(nlh, 0, NLMSG_SPACE(MAX_PAYLOAD)); + + nlh->nlmsg_type = RTM_NEWNEXTHOP; + nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_REPLACE; + nlh->nlmsg_len = NLMSG_LENGTH(sizeof(struct nhmsg)); + struct nhmsg *nhm = (struct nhmsg *)NLMSG_DATA(nlh); + nhm->nh_family = AF_INET; + + EXPECT_CALL(m_mockRouteSync, getIfName(_, _, _)) + .Times(0); + + m_mockRouteSync.onNextHopMsg(nlh, 0); + + free(nlh); +} + +TEST_F(FpmSyncdResponseTest, TestSingleNextHopAdd) +{ + uint32_t test_id = 10; + const char* test_gateway = "192.168.1.1"; + int32_t test_ifindex = 5; + struct nlmsghdr* nlh = createNewNextHopMsgHdr(test_ifindex, test_gateway, test_id); + int expected_length = (int)(nlh->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg))); + + EXPECT_CALL(m_mockRouteSync, getIfName(test_ifindex, _, _)) + .WillOnce(DoAll( + [](int32_t, char* ifname, size_t size) { + strncpy(ifname, "Ethernet1", size); + ifname[size-1] = '\0'; + }, + Return(true) + )); + + m_mockRouteSync.onNextHopMsg(nlh, expected_length); + + auto it = m_mockRouteSync.m_nh_groups.find(test_id); + ASSERT_NE(it, m_mockRouteSync.m_nh_groups.end()) << "Failed to add new nexthop"; + + free(nlh); +} + +TEST_F(FpmSyncdResponseTest, TestSkipSpecialInterfaces) +{ + uint32_t test_id = 11; + const char* test_gateway = "192.168.1.1"; + int32_t test_ifindex = 6; + + EXPECT_CALL(m_mockRouteSync, getIfName(test_ifindex, _, _)) + .WillOnce(DoAll( + [](int32_t ifidx, char* ifname, size_t size) { + strncpy(ifname, "eth0", size); + }, + Return(true) + )); + + struct nlmsghdr* nlh = createNewNextHopMsgHdr(test_ifindex, test_gateway, test_id); + int expected_length = (int)(nlh->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg))); + dump_nexthop_msg(nlh); + m_mockRouteSync.onNextHopMsg(nlh, expected_length); + + auto it = m_mockRouteSync.m_nh_groups.find(test_id); + EXPECT_EQ(it, m_mockRouteSync.m_nh_groups.end()) << "Should skip eth0 interface"; + + free(nlh); } From f97659bafc6287d9e7649f7e7b32c9f0e0e46a61 Mon Sep 17 00:00:00 2001 From: goomadao Date: Wed, 4 Dec 2024 16:30:56 +0800 Subject: [PATCH 07/15] increase MAX_MULTIPATH_NUM to 514 --- fpmsyncd/routesync.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index 6b6247ebfe..565e987ece 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -112,7 +112,7 @@ enum { ROUTE_ENCAP_SRV6_ENCAP_SRC_ADDR = 2, }; -#define MAX_MULTIPATH_NUM 256 //Same value used for FRR in SONiC +#define MAX_MULTIPATH_NUM 514 /* Returns name of the protocol passed number represents */ static string getProtocolString(int proto) From 7915cd65663ff1ca81a3c1d646340a5acf4e6489 Mon Sep 17 00:00:00 2001 From: Joy Date: Wed, 4 Dec 2024 22:59:00 +0000 Subject: [PATCH 08/15] add testcases for deleteNextHopGroup updateNextHopGroup getNextHopGroupFields --- tests/mock_tests/fpmsyncd/test_routesync.cpp | 171 +++++++++++++++++++ 1 file changed, 171 insertions(+) diff --git a/tests/mock_tests/fpmsyncd/test_routesync.cpp b/tests/mock_tests/fpmsyncd/test_routesync.cpp index 8d2aaa3592..f21ec3af40 100644 --- a/tests/mock_tests/fpmsyncd/test_routesync.cpp +++ b/tests/mock_tests/fpmsyncd/test_routesync.cpp @@ -394,3 +394,174 @@ TEST_F(FpmSyncdResponseTest, TestSkipSpecialInterfaces) free(nlh); } + +TEST_F(FpmSyncdResponseTest, TestNextHopGroupKeyString) +{ + EXPECT_EQ(m_mockRouteSync.getNextHopGroupKeyAsString(1), "ID1"); + EXPECT_EQ(m_mockRouteSync.getNextHopGroupKeyAsString(1234), "ID1234"); +} + +TEST_F(FpmSyncdResponseTest, TestGetNextHopGroupFields) +{ + // Test single next hop case + { + NextHopGroup nhg(1, "192.168.1.1", "Ethernet0"); + m_mockRouteSync.m_nh_groups.insert({1, nhg}); + + string nexthops, ifnames, weights; + m_mockRouteSync.getNextHopGroupFields(nhg, nexthops, ifnames, weights); + + EXPECT_EQ(nexthops, "192.168.1.1"); + EXPECT_EQ(ifnames, "Ethernet0"); + EXPECT_TRUE(weights.empty()); + } + + // Test multiple next hops with weights + { + // Create the component next hops first + NextHopGroup nhg1(1, "192.168.1.1", "Ethernet0"); + NextHopGroup nhg2(2, "192.168.1.2", "Ethernet1"); + m_mockRouteSync.m_nh_groups.insert({1, nhg1}); + m_mockRouteSync.m_nh_groups.insert({2, nhg2}); + + // Create the group with multiple next hops + vector> group_members; + group_members.push_back(make_pair(1, 1)); // id=1, weight=1 + group_members.push_back(make_pair(2, 2)); // id=2, weight=2 + + NextHopGroup nhg(3, group_members); + m_mockRouteSync.m_nh_groups.insert({3, nhg}); + + string nexthops, ifnames, weights; + m_mockRouteSync.getNextHopGroupFields(nhg, nexthops, ifnames, weights); + + EXPECT_EQ(nexthops, "192.168.1.1,192.168.1.2"); + EXPECT_EQ(ifnames, "Ethernet0,Ethernet1"); + EXPECT_EQ(weights, "1,2"); + } + + // Test IPv6 default case + { + NextHopGroup nhg(4, "", "Ethernet0"); + m_mockRouteSync.m_nh_groups.insert({4, nhg}); + + string nexthops, ifnames, weights; + m_mockRouteSync.getNextHopGroupFields(nhg, nexthops, ifnames, weights, AF_INET6); + + EXPECT_EQ(nexthops, "::"); + EXPECT_EQ(ifnames, "Ethernet0"); + EXPECT_TRUE(weights.empty()); + } + + // Both empty + { + NextHopGroup nhg(5, "", ""); + string nexthops, ifnames, weights; + m_mockRouteSync.getNextHopGroupFields(nhg, nexthops, ifnames, weights, AF_INET); + + EXPECT_EQ(nexthops, "0.0.0.0"); + EXPECT_TRUE(ifnames.empty()); + EXPECT_TRUE(weights.empty()); + } +} + +TEST_F(FpmSyncdResponseTest, TestUpdateNextHopGroupDb) +{ + Table nexthop_group_table(m_db.get(), APP_NEXTHOP_GROUP_TABLE_NAME); + + // Test single next hop group + { + NextHopGroup nhg(1, "192.168.1.1", "Ethernet0"); + m_mockRouteSync.updateNextHopGroupDb(nhg); + + vector fieldValues; + nexthop_group_table.get("ID1", fieldValues); + + EXPECT_EQ(fieldValues.size(), 2); + EXPECT_EQ(fvField(fieldValues[0]), "nexthop"); + EXPECT_EQ(fvValue(fieldValues[0]), "192.168.1.1"); + EXPECT_EQ(fvField(fieldValues[1]), "ifname"); + EXPECT_EQ(fvValue(fieldValues[1]), "Ethernet0"); + } + + // Test group with multiple next hops + { + vector> group_members; + group_members.push_back(make_pair(1, 1)); + group_members.push_back(make_pair(2, 2)); + + NextHopGroup nhg1(1, "192.168.1.1", "Ethernet0"); + NextHopGroup nhg2(2, "192.168.1.2", "Ethernet1"); + NextHopGroup group(3, group_members); + + m_mockRouteSync.m_nh_groups.insert({1, nhg1}); + m_mockRouteSync.m_nh_groups.insert({2, nhg2}); + m_mockRouteSync.m_nh_groups.insert({3, group}); + + m_mockRouteSync.updateNextHopGroup(3); + + auto it = m_mockRouteSync.m_nh_groups.find(3); + ASSERT_NE(it, m_mockRouteSync.m_nh_groups.end()); + EXPECT_TRUE(it->second.installed); + vector fieldValues; + nexthop_group_table.get("ID3", fieldValues); + EXPECT_EQ(fieldValues.size(), 3); + EXPECT_EQ(fvField(fieldValues[0]), "nexthop"); + EXPECT_EQ(fvValue(fieldValues[0]), "192.168.1.1,192.168.1.2"); + EXPECT_EQ(fvField(fieldValues[1]), "ifname"); + EXPECT_EQ(fvValue(fieldValues[1]), "Ethernet0,Ethernet1"); + EXPECT_EQ(fvField(fieldValues[2]), "weight"); + EXPECT_EQ(fvValue(fieldValues[2]), "1,2"); + } + + // Empty nexthop (default route case) + { + NextHopGroup nhg(4, "", "Ethernet0"); + m_mockRouteSync.updateNextHopGroupDb(nhg); + + vector fieldValues; + nexthop_group_table.get("ID4", fieldValues); + + EXPECT_EQ(fieldValues.size(), 2); + EXPECT_EQ(fvField(fieldValues[0]), "nexthop"); + EXPECT_EQ(fvValue(fieldValues[0]), "0.0.0.0"); + EXPECT_EQ(fvField(fieldValues[1]), "ifname"); + EXPECT_EQ(fvValue(fieldValues[1]), "Ethernet0"); + } + + // Empty interface name + { + NextHopGroup nhg(5, "192.168.1.1", ""); + m_mockRouteSync.updateNextHopGroupDb(nhg); + + vector fieldValues; + nexthop_group_table.get("ID5", fieldValues); + + EXPECT_EQ(fieldValues.size(), 2); + EXPECT_EQ(fvField(fieldValues[0]), "nexthop"); + EXPECT_EQ(fvValue(fieldValues[0]), "192.168.1.1"); + EXPECT_EQ(fvField(fieldValues[1]), "ifname"); + EXPECT_EQ(fvValue(fieldValues[1]), ""); + } +} + +TEST_F(FpmSyncdResponseTest, TestDeleteNextHopGroup) +{ + // Setup test groups + NextHopGroup nhg1(1, "192.168.1.1", "Ethernet0"); + NextHopGroup nhg2(2, "192.168.1.2", "Ethernet1"); + nhg1.installed = true; + nhg2.installed = true; + + m_mockRouteSync.m_nh_groups.insert({1, nhg1}); + m_mockRouteSync.m_nh_groups.insert({2, nhg2}); + + // Test deletion + m_mockRouteSync.deleteNextHopGroup(1); + EXPECT_EQ(m_mockRouteSync.m_nh_groups.find(1), m_mockRouteSync.m_nh_groups.end()); + EXPECT_NE(m_mockRouteSync.m_nh_groups.find(2), m_mockRouteSync.m_nh_groups.end()); + + // Test deleting non-existent group + m_mockRouteSync.deleteNextHopGroup(999); + EXPECT_EQ(m_mockRouteSync.m_nh_groups.find(999), m_mockRouteSync.m_nh_groups.end()); +} From 15b208655d99a9f4b1806988020b36528161c0cd Mon Sep 17 00:00:00 2001 From: Joy Date: Wed, 4 Dec 2024 23:46:57 +0000 Subject: [PATCH 09/15] add more testcases for onNextHopMsg --- tests/mock_tests/fpmsyncd/test_routesync.cpp | 132 +++++++++++++------ 1 file changed, 92 insertions(+), 40 deletions(-) diff --git a/tests/mock_tests/fpmsyncd/test_routesync.cpp b/tests/mock_tests/fpmsyncd/test_routesync.cpp index f21ec3af40..0c34bebfae 100644 --- a/tests/mock_tests/fpmsyncd/test_routesync.cpp +++ b/tests/mock_tests/fpmsyncd/test_routesync.cpp @@ -259,72 +259,55 @@ TEST_F(FpmSyncdResponseTest, testSendOffloadReply) nlmsg_alloc_ret = true; } -struct nlmsghdr* createNewNextHopMsgHdr(int32_t ifindex, const char* gateway, uint32_t id) { +struct nlmsghdr* createNewNextHopMsgHdr(int32_t ifindex, const char* gateway, uint32_t id, unsigned char nh_family=AF_INET) { struct nlmsghdr *nlh = (struct nlmsghdr *)malloc(NLMSG_SPACE(MAX_PAYLOAD)); memset(nlh, 0, NLMSG_SPACE(MAX_PAYLOAD)); - // 设置基本header + // Set header nlh->nlmsg_type = RTM_NEWNEXTHOP; nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_REPLACE; nlh->nlmsg_len = NLMSG_LENGTH(sizeof(struct nhmsg)); - - printf("After header setup - nlmsg_len: %d\n", nlh->nlmsg_len); - // 设置nhmsg + // Set nhmsg struct nhmsg *nhm = (struct nhmsg *)NLMSG_DATA(nlh); - nhm->nh_family = AF_INET; + nhm->nh_family = nh_family; - // 先添加 NHA_ID + // Add NHA_ID struct rtattr *rta = (struct rtattr *)((char *)nlh + NLMSG_ALIGN(nlh->nlmsg_len)); rta->rta_type = NHA_ID; rta->rta_len = RTA_LENGTH(sizeof(uint32_t)); *(uint32_t *)RTA_DATA(rta) = id; nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + RTA_ALIGN(rta->rta_len); - - printf("After NHA_ID - nlmsg_len: %d\n", nlh->nlmsg_len); - // 添加 NHA_GATEWAY + // Add NHA_GATEWAY rta = (struct rtattr *)((char *)nlh + NLMSG_ALIGN(nlh->nlmsg_len)); - struct in_addr gw_addr; - inet_pton(AF_INET, gateway, &gw_addr); rta->rta_type = NHA_GATEWAY; - rta->rta_len = RTA_LENGTH(sizeof(struct in_addr)); - memcpy(RTA_DATA(rta), &gw_addr, sizeof(struct in_addr)); + if (nh_family == AF_INET6) + { + struct in6_addr gw_addr6; + inet_pton(AF_INET6, gateway, &gw_addr6); + rta->rta_len = RTA_LENGTH(sizeof(struct in6_addr)); + memcpy(RTA_DATA(rta), &gw_addr6, sizeof(struct in6_addr)); + } + else + { + struct in_addr gw_addr; + inet_pton(AF_INET, gateway, &gw_addr); + rta->rta_len = RTA_LENGTH(sizeof(struct in_addr)); + memcpy(RTA_DATA(rta), &gw_addr, sizeof(struct in_addr)); + } nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + RTA_ALIGN(rta->rta_len); - - printf("After NHA_GATEWAY - nlmsg_len: %d\n", nlh->nlmsg_len); - // 添加 NHA_OIF + // Add NHA_OIF rta = (struct rtattr *)((char *)nlh + NLMSG_ALIGN(nlh->nlmsg_len)); rta->rta_type = NHA_OIF; rta->rta_len = RTA_LENGTH(sizeof(int32_t)); *(int32_t *)RTA_DATA(rta) = ifindex; nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + RTA_ALIGN(rta->rta_len); - - printf("After NHA_OIF - final nlmsg_len: %d\n", nlh->nlmsg_len); return nlh; } -void dump_nexthop_msg(struct nlmsghdr *nlh) { - printf("\nDumping nexthop message:\n"); - printf("nlmsg_len: %d\n", nlh->nlmsg_len); - printf("nlmsg_type: %d\n", nlh->nlmsg_type); - - struct nhmsg *nhm = (struct nhmsg *)NLMSG_DATA(nlh); - printf("nh_family: %d\n", nhm->nh_family); - - struct rtattr *rta = (struct rtattr *)((char *)nhm + NLMSG_ALIGN(sizeof(*nhm))); - int len = nlh->nlmsg_len - (int)NLMSG_LENGTH(sizeof(*nhm)); - - while (RTA_OK(rta, len)) { - printf("Attribute type: %d, len: %d\n", rta->rta_type, rta->rta_len); - if (rta->rta_type == NHA_OIF) { - printf(" OIF value: %d\n", *(int32_t *)RTA_DATA(rta)); - } - rta = RTA_NEXT(rta, len); - } -} TEST_F(FpmSyncdResponseTest, TestNoNHAId) { struct nlmsghdr *nlh = (struct nlmsghdr *)malloc(NLMSG_SPACE(MAX_PAYLOAD)); @@ -344,7 +327,7 @@ TEST_F(FpmSyncdResponseTest, TestNoNHAId) free(nlh); } -TEST_F(FpmSyncdResponseTest, TestSingleNextHopAdd) +TEST_F(FpmSyncdResponseTest, TestNextHopAdd) { uint32_t test_id = 10; const char* test_gateway = "192.168.1.1"; @@ -370,6 +353,75 @@ TEST_F(FpmSyncdResponseTest, TestSingleNextHopAdd) free(nlh); } +TEST_F(FpmSyncdResponseTest, TestIPv6NextHopAdd) +{ + uint32_t test_id = 20; + const char* test_gateway = "2001:db8::1"; + int32_t test_ifindex = 7; + + struct nlmsghdr* nlh = createNewNextHopMsgHdr(test_ifindex, test_gateway, test_id, AF_INET6); + int expected_length = (int)(nlh->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg))); + + EXPECT_CALL(m_mockRouteSync, getIfName(test_ifindex, _, _)) + .WillOnce(DoAll( + [](int32_t, char* ifname, size_t size) { + strncpy(ifname, "Ethernet2", size); + ifname[size-1] = '\0'; + }, + Return(true) + )); + + m_mockRouteSync.onNextHopMsg(nlh, expected_length); + + Table nexthop_group_table(m_db.get(), APP_NEXTHOP_GROUP_TABLE_NAME); + + vector fieldValues; + string key = "ID" + to_string(test_id); + nexthop_group_table.get(key, fieldValues); + + // onNextHopMsg only updates m_nh_groups unless the nhg is marked as installed + ASSERT_TRUE(fieldValues.empty()); + + // Update the nexthop group to mark it as installed and write to DB + m_mockRouteSync.updateNextHopGroup(test_id); + nexthop_group_table.get(key, fieldValues); + + string nexthop, ifname; + for (const auto& fv : fieldValues) { + if (fvField(fv) == "nexthop") { + nexthop = fvValue(fv); + } else if (fvField(fv) == "ifname") { + ifname = fvValue(fv); + } + } + + EXPECT_EQ(nexthop, test_gateway); + EXPECT_EQ(ifname, "Ethernet2"); + + free(nlh); +} + + +TEST_F(FpmSyncdResponseTest, TestGetIfNameFailure) +{ + uint32_t test_id = 22; + const char* test_gateway = "192.168.1.1"; + int32_t test_ifindex = 9; + + struct nlmsghdr* nlh = createNewNextHopMsgHdr(test_ifindex, test_gateway, test_id); + int expected_length = (int)(nlh->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg))); + + EXPECT_CALL(m_mockRouteSync, getIfName(test_ifindex, _, _)) + .WillOnce(Return(false)); + + m_mockRouteSync.onNextHopMsg(nlh, expected_length); + + auto it = m_mockRouteSync.m_nh_groups.find(test_id); + ASSERT_NE(it, m_mockRouteSync.m_nh_groups.end()); + EXPECT_EQ(it->second.intf, "unknown"); + + free(nlh); +} TEST_F(FpmSyncdResponseTest, TestSkipSpecialInterfaces) { uint32_t test_id = 11; @@ -386,7 +438,7 @@ TEST_F(FpmSyncdResponseTest, TestSkipSpecialInterfaces) struct nlmsghdr* nlh = createNewNextHopMsgHdr(test_ifindex, test_gateway, test_id); int expected_length = (int)(nlh->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg))); - dump_nexthop_msg(nlh); + m_mockRouteSync.onNextHopMsg(nlh, expected_length); auto it = m_mockRouteSync.m_nh_groups.find(test_id); From face28e99304c8d7017f298783dfdb6af903b5df Mon Sep 17 00:00:00 2001 From: Joy Date: Sun, 8 Dec 2024 04:41:20 +0000 Subject: [PATCH 10/15] add #pragma GCC diagnostic ignored "-Wcast-align" in test_routesync.cpp --- tests/mock_tests/fpmsyncd/test_routesync.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/mock_tests/fpmsyncd/test_routesync.cpp b/tests/mock_tests/fpmsyncd/test_routesync.cpp index 0c34bebfae..d2c58146a8 100644 --- a/tests/mock_tests/fpmsyncd/test_routesync.cpp +++ b/tests/mock_tests/fpmsyncd/test_routesync.cpp @@ -24,6 +24,8 @@ using ::testing::_; int rt_build_ret = 0; bool nlmsg_alloc_ret = true; +#pragma GCC diagnostic ignored "-Wcast-align" + class MockRouteSync : public RouteSync { public: From aeaca480ae105e7cd88e8fa9af42b78f39a07ecc Mon Sep 17 00:00:00 2001 From: Joy Date: Mon, 9 Dec 2024 01:10:34 +0000 Subject: [PATCH 11/15] add testcases for grp_count > 0 --- tests/mock_tests/fpmsyncd/test_routesync.cpp | 147 +++++++++++++++++++ 1 file changed, 147 insertions(+) diff --git a/tests/mock_tests/fpmsyncd/test_routesync.cpp b/tests/mock_tests/fpmsyncd/test_routesync.cpp index d2c58146a8..e5243c20f4 100644 --- a/tests/mock_tests/fpmsyncd/test_routesync.cpp +++ b/tests/mock_tests/fpmsyncd/test_routesync.cpp @@ -619,3 +619,150 @@ TEST_F(FpmSyncdResponseTest, TestDeleteNextHopGroup) m_mockRouteSync.deleteNextHopGroup(999); EXPECT_EQ(m_mockRouteSync.m_nh_groups.find(999), m_mockRouteSync.m_nh_groups.end()); } + +struct nlmsghdr* createNewNextHopMsgHdr(const vector>& group_members, uint32_t id) { + struct nlmsghdr *nlh = (struct nlmsghdr *)malloc(NLMSG_SPACE(MAX_PAYLOAD)); + memset(nlh, 0, NLMSG_SPACE(MAX_PAYLOAD)); + + // Set header + nlh->nlmsg_type = RTM_NEWNEXTHOP; + nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_REPLACE; + nlh->nlmsg_len = NLMSG_LENGTH(sizeof(struct nhmsg)); + + // Set nhmsg + struct nhmsg *nhm = (struct nhmsg *)NLMSG_DATA(nlh); + nhm->nh_family = AF_INET; + + // Add NHA_ID + struct rtattr *rta = (struct rtattr *)((char *)nlh + NLMSG_ALIGN(nlh->nlmsg_len)); + rta->rta_type = NHA_ID; + rta->rta_len = RTA_LENGTH(sizeof(uint32_t)); + *(uint32_t *)RTA_DATA(rta) = id; + nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + RTA_ALIGN(rta->rta_len); + + // Add NHA_GROUP + rta = (struct rtattr *)((char *)nlh + NLMSG_ALIGN(nlh->nlmsg_len)); + rta->rta_type = NHA_GROUP; + struct nexthop_grp* grp = (struct nexthop_grp*)malloc(group_members.size() * sizeof(struct nexthop_grp)); + + for (size_t i = 0; i < group_members.size(); i++) { + grp[i].id = group_members[i].first; + grp[i].weight = group_members[i].second - 1; // kernel stores weight-1 + } + + size_t payload_size = group_members.size() * sizeof(struct nexthop_grp); + if (payload_size > USHRT_MAX - RTA_LENGTH(0)) { + free(nlh); + return nullptr; + } + + rta->rta_len = static_cast(RTA_LENGTH(group_members.size() * sizeof(struct nexthop_grp))); + memcpy(RTA_DATA(rta), grp, group_members.size() * sizeof(struct nexthop_grp)); + nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + RTA_ALIGN(rta->rta_len); + + free(grp); + return nlh; +} + +TEST_F(FpmSyncdResponseTest, TestNextHopGroupAdd) +{ + // 1. create nexthops + uint32_t nh1_id = 1; + uint32_t nh2_id = 2; + uint32_t nh3_id = 3; + + struct nlmsghdr* nlh1 = createNewNextHopMsgHdr(1, "192.168.1.1", nh1_id); + struct nlmsghdr* nlh2 = createNewNextHopMsgHdr(2, "192.168.1.2", nh2_id); + struct nlmsghdr* nlh3 = createNewNextHopMsgHdr(3, "192.168.1.3", nh3_id); + + EXPECT_CALL(m_mockRouteSync, getIfName(1, _, _)) + .WillOnce(DoAll( + [](int32_t, char* ifname, size_t size) { + strncpy(ifname, "Ethernet1", size); + ifname[size-1] = '\0'; + }, + Return(true) + )); + + EXPECT_CALL(m_mockRouteSync, getIfName(2, _, _)) + .WillOnce(DoAll( + [](int32_t, char* ifname, size_t size) { + strncpy(ifname, "Ethernet2", size); + ifname[size-1] = '\0'; + }, + Return(true) + )); + + EXPECT_CALL(m_mockRouteSync, getIfName(3, _, _)) + .WillOnce(DoAll( + [](int32_t, char* ifname, size_t size) { + strncpy(ifname, "Ethernet3", size); + ifname[size-1] = '\0'; + }, + Return(true) + )); + + m_mockRouteSync.onNextHopMsg(nlh1, (int)(nlh1->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg)))); + m_mockRouteSync.onNextHopMsg(nlh2, (int)(nlh2->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg)))); + m_mockRouteSync.onNextHopMsg(nlh3, (int)(nlh3->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg)))); + + // 2. create a nexthop group with these nexthops + uint32_t group_id = 10; + vector> group_members = { + {nh1_id, 1}, // id=1, weight=1 + {nh2_id, 2}, // id=2, weight=2 + {nh3_id, 3} // id=3, weight=3 + }; + + struct nlmsghdr* group_nlh = createNewNextHopMsgHdr(group_members, group_id); + ASSERT_NE(group_nlh, nullptr) << "Failed to create group nexthop message"; + m_mockRouteSync.onNextHopMsg(group_nlh, (int)(group_nlh->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg)))); + + // Verify the group was added correctly + auto it = m_mockRouteSync.m_nh_groups.find(group_id); + ASSERT_NE(it, m_mockRouteSync.m_nh_groups.end()) << "Failed to add nexthop group"; + + // Verify group members + const auto& group = it->second.group; + ASSERT_EQ(group.size(), 3) << "Wrong number of group members"; + + // Check each member's ID and weight + EXPECT_EQ(group[0].first, nh1_id); + EXPECT_EQ(group[0].second, 1); + EXPECT_EQ(group[1].first, nh2_id); + EXPECT_EQ(group[1].second, 2); + EXPECT_EQ(group[2].first, nh3_id); + EXPECT_EQ(group[2].second, 3); + + // Mark the group as installed and verify DB update + m_mockRouteSync.updateNextHopGroup(group_id); + + Table nexthop_group_table(m_db.get(), APP_NEXTHOP_GROUP_TABLE_NAME); + vector fieldValues; + string key = "ID" + to_string(group_id); + nexthop_group_table.get(key, fieldValues); + + ASSERT_EQ(fieldValues.size(), 3) << "Wrong number of fields in DB"; + + // Verify the DB fields + string nexthops, ifnames, weights; + for (const auto& fv : fieldValues) { + if (fvField(fv) == "nexthop") { + nexthops = fvValue(fv); + } else if (fvField(fv) == "ifname") { + ifnames = fvValue(fv); + } else if (fvField(fv) == "weight") { + weights = fvValue(fv); + } + } + + EXPECT_EQ(nexthops, "192.168.1.1,192.168.1.2,192.168.1.3"); + EXPECT_EQ(ifnames, "Ethernet1,Ethernet2,Ethernet3"); + EXPECT_EQ(weights, "1,2,3"); + + // Cleanup + free(nlh1); + free(nlh2); + free(nlh3); + free(group_nlh); +} From a8130c907fea9b593873d985c589af0432598abc Mon Sep 17 00:00:00 2001 From: Joy Date: Mon, 9 Dec 2024 06:01:08 +0000 Subject: [PATCH 12/15] add testcase for onRouteMsg --- tests/mock_tests/fpmsyncd/test_routesync.cpp | 254 ++++++++++++++----- 1 file changed, 196 insertions(+), 58 deletions(-) diff --git a/tests/mock_tests/fpmsyncd/test_routesync.cpp b/tests/mock_tests/fpmsyncd/test_routesync.cpp index e5243c20f4..f68d4ad7be 100644 --- a/tests/mock_tests/fpmsyncd/test_routesync.cpp +++ b/tests/mock_tests/fpmsyncd/test_routesync.cpp @@ -82,6 +82,10 @@ class FpmSyncdResponseTest : public ::testing::Test RouteSync m_routeSync{m_pipeline.get()}; MockFpm m_mockFpm{&m_routeSync}; MockRouteSync m_mockRouteSync{m_pipeline.get()}; + + const char* test_gateway = "192.168.1.1"; + const char* test_gateway_ = "192.168.1.2"; + const char* test_gateway__ = "192.168.1.3"; }; TEST_F(FpmSyncdResponseTest, RouteResponseFeedbackV4) @@ -237,7 +241,7 @@ TEST_F(FpmSyncdResponseTest, testEvpn) return true; }); m_mockRouteSync.onMsgRaw(nlh); - + vector keys; vector fieldValues; app_route_table.getKeys(keys); @@ -281,6 +285,13 @@ struct nlmsghdr* createNewNextHopMsgHdr(int32_t ifindex, const char* gateway, ui *(uint32_t *)RTA_DATA(rta) = id; nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + RTA_ALIGN(rta->rta_len); + // Add NHA_OIF + rta = (struct rtattr *)((char *)nlh + NLMSG_ALIGN(nlh->nlmsg_len)); + rta->rta_type = NHA_OIF; + rta->rta_len = RTA_LENGTH(sizeof(int32_t)); + *(int32_t *)RTA_DATA(rta) = ifindex; + nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + RTA_ALIGN(rta->rta_len); + // Add NHA_GATEWAY rta = (struct rtattr *)((char *)nlh + NLMSG_ALIGN(nlh->nlmsg_len)); rta->rta_type = NHA_GATEWAY; @@ -292,7 +303,7 @@ struct nlmsghdr* createNewNextHopMsgHdr(int32_t ifindex, const char* gateway, ui memcpy(RTA_DATA(rta), &gw_addr6, sizeof(struct in6_addr)); } else - { + { struct in_addr gw_addr; inet_pton(AF_INET, gateway, &gw_addr); rta->rta_len = RTA_LENGTH(sizeof(struct in_addr)); @@ -300,13 +311,6 @@ struct nlmsghdr* createNewNextHopMsgHdr(int32_t ifindex, const char* gateway, ui } nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + RTA_ALIGN(rta->rta_len); - // Add NHA_OIF - rta = (struct rtattr *)((char *)nlh + NLMSG_ALIGN(nlh->nlmsg_len)); - rta->rta_type = NHA_OIF; - rta->rta_len = RTA_LENGTH(sizeof(int32_t)); - *(int32_t *)RTA_DATA(rta) = ifindex; - nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + RTA_ALIGN(rta->rta_len); - return nlh; } @@ -332,7 +336,6 @@ TEST_F(FpmSyncdResponseTest, TestNoNHAId) TEST_F(FpmSyncdResponseTest, TestNextHopAdd) { uint32_t test_id = 10; - const char* test_gateway = "192.168.1.1"; int32_t test_ifindex = 5; struct nlmsghdr* nlh = createNewNextHopMsgHdr(test_ifindex, test_gateway, test_id); @@ -380,7 +383,7 @@ TEST_F(FpmSyncdResponseTest, TestIPv6NextHopAdd) vector fieldValues; string key = "ID" + to_string(test_id); nexthop_group_table.get(key, fieldValues); - + // onNextHopMsg only updates m_nh_groups unless the nhg is marked as installed ASSERT_TRUE(fieldValues.empty()); @@ -396,7 +399,7 @@ TEST_F(FpmSyncdResponseTest, TestIPv6NextHopAdd) ifname = fvValue(fv); } } - + EXPECT_EQ(nexthop, test_gateway); EXPECT_EQ(ifname, "Ethernet2"); @@ -407,7 +410,6 @@ TEST_F(FpmSyncdResponseTest, TestIPv6NextHopAdd) TEST_F(FpmSyncdResponseTest, TestGetIfNameFailure) { uint32_t test_id = 22; - const char* test_gateway = "192.168.1.1"; int32_t test_ifindex = 9; struct nlmsghdr* nlh = createNewNextHopMsgHdr(test_ifindex, test_gateway, test_id); @@ -427,7 +429,6 @@ TEST_F(FpmSyncdResponseTest, TestGetIfNameFailure) TEST_F(FpmSyncdResponseTest, TestSkipSpecialInterfaces) { uint32_t test_id = 11; - const char* test_gateway = "192.168.1.1"; int32_t test_ifindex = 6; EXPECT_CALL(m_mockRouteSync, getIfName(test_ifindex, _, _)) @@ -459,13 +460,13 @@ TEST_F(FpmSyncdResponseTest, TestGetNextHopGroupFields) { // Test single next hop case { - NextHopGroup nhg(1, "192.168.1.1", "Ethernet0"); + NextHopGroup nhg(1, test_gateway, "Ethernet0"); m_mockRouteSync.m_nh_groups.insert({1, nhg}); - + string nexthops, ifnames, weights; m_mockRouteSync.getNextHopGroupFields(nhg, nexthops, ifnames, weights); - - EXPECT_EQ(nexthops, "192.168.1.1"); + + EXPECT_EQ(nexthops, test_gateway); EXPECT_EQ(ifnames, "Ethernet0"); EXPECT_TRUE(weights.empty()); } @@ -473,22 +474,22 @@ TEST_F(FpmSyncdResponseTest, TestGetNextHopGroupFields) // Test multiple next hops with weights { // Create the component next hops first - NextHopGroup nhg1(1, "192.168.1.1", "Ethernet0"); - NextHopGroup nhg2(2, "192.168.1.2", "Ethernet1"); + NextHopGroup nhg1(1, test_gateway, "Ethernet0"); + NextHopGroup nhg2(2, test_gateway_, "Ethernet1"); m_mockRouteSync.m_nh_groups.insert({1, nhg1}); m_mockRouteSync.m_nh_groups.insert({2, nhg2}); - + // Create the group with multiple next hops vector> group_members; group_members.push_back(make_pair(1, 1)); // id=1, weight=1 group_members.push_back(make_pair(2, 2)); // id=2, weight=2 - + NextHopGroup nhg(3, group_members); m_mockRouteSync.m_nh_groups.insert({3, nhg}); - + string nexthops, ifnames, weights; m_mockRouteSync.getNextHopGroupFields(nhg, nexthops, ifnames, weights); - + EXPECT_EQ(nexthops, "192.168.1.1,192.168.1.2"); EXPECT_EQ(ifnames, "Ethernet0,Ethernet1"); EXPECT_EQ(weights, "1,2"); @@ -498,10 +499,10 @@ TEST_F(FpmSyncdResponseTest, TestGetNextHopGroupFields) { NextHopGroup nhg(4, "", "Ethernet0"); m_mockRouteSync.m_nh_groups.insert({4, nhg}); - + string nexthops, ifnames, weights; m_mockRouteSync.getNextHopGroupFields(nhg, nexthops, ifnames, weights, AF_INET6); - + EXPECT_EQ(nexthops, "::"); EXPECT_EQ(ifnames, "Ethernet0"); EXPECT_TRUE(weights.empty()); @@ -512,7 +513,7 @@ TEST_F(FpmSyncdResponseTest, TestGetNextHopGroupFields) NextHopGroup nhg(5, "", ""); string nexthops, ifnames, weights; m_mockRouteSync.getNextHopGroupFields(nhg, nexthops, ifnames, weights, AF_INET); - + EXPECT_EQ(nexthops, "0.0.0.0"); EXPECT_TRUE(ifnames.empty()); EXPECT_TRUE(weights.empty()); @@ -525,15 +526,15 @@ TEST_F(FpmSyncdResponseTest, TestUpdateNextHopGroupDb) // Test single next hop group { - NextHopGroup nhg(1, "192.168.1.1", "Ethernet0"); + NextHopGroup nhg(1, test_gateway, "Ethernet0"); m_mockRouteSync.updateNextHopGroupDb(nhg); vector fieldValues; nexthop_group_table.get("ID1", fieldValues); - + EXPECT_EQ(fieldValues.size(), 2); EXPECT_EQ(fvField(fieldValues[0]), "nexthop"); - EXPECT_EQ(fvValue(fieldValues[0]), "192.168.1.1"); + EXPECT_EQ(fvValue(fieldValues[0]), test_gateway); EXPECT_EQ(fvField(fieldValues[1]), "ifname"); EXPECT_EQ(fvValue(fieldValues[1]), "Ethernet0"); } @@ -543,17 +544,17 @@ TEST_F(FpmSyncdResponseTest, TestUpdateNextHopGroupDb) vector> group_members; group_members.push_back(make_pair(1, 1)); group_members.push_back(make_pair(2, 2)); - - NextHopGroup nhg1(1, "192.168.1.1", "Ethernet0"); - NextHopGroup nhg2(2, "192.168.1.2", "Ethernet1"); + + NextHopGroup nhg1(1, test_gateway, "Ethernet0"); + NextHopGroup nhg2(2, test_gateway_, "Ethernet1"); NextHopGroup group(3, group_members); - + m_mockRouteSync.m_nh_groups.insert({1, nhg1}); m_mockRouteSync.m_nh_groups.insert({2, nhg2}); m_mockRouteSync.m_nh_groups.insert({3, group}); - + m_mockRouteSync.updateNextHopGroup(3); - + auto it = m_mockRouteSync.m_nh_groups.find(3); ASSERT_NE(it, m_mockRouteSync.m_nh_groups.end()); EXPECT_TRUE(it->second.installed); @@ -572,10 +573,10 @@ TEST_F(FpmSyncdResponseTest, TestUpdateNextHopGroupDb) { NextHopGroup nhg(4, "", "Ethernet0"); m_mockRouteSync.updateNextHopGroupDb(nhg); - + vector fieldValues; nexthop_group_table.get("ID4", fieldValues); - + EXPECT_EQ(fieldValues.size(), 2); EXPECT_EQ(fvField(fieldValues[0]), "nexthop"); EXPECT_EQ(fvValue(fieldValues[0]), "0.0.0.0"); @@ -585,15 +586,15 @@ TEST_F(FpmSyncdResponseTest, TestUpdateNextHopGroupDb) // Empty interface name { - NextHopGroup nhg(5, "192.168.1.1", ""); + NextHopGroup nhg(5, test_gateway, ""); m_mockRouteSync.updateNextHopGroupDb(nhg); - + vector fieldValues; nexthop_group_table.get("ID5", fieldValues); - + EXPECT_EQ(fieldValues.size(), 2); EXPECT_EQ(fvField(fieldValues[0]), "nexthop"); - EXPECT_EQ(fvValue(fieldValues[0]), "192.168.1.1"); + EXPECT_EQ(fvValue(fieldValues[0]), test_gateway); EXPECT_EQ(fvField(fieldValues[1]), "ifname"); EXPECT_EQ(fvValue(fieldValues[1]), ""); } @@ -602,14 +603,14 @@ TEST_F(FpmSyncdResponseTest, TestUpdateNextHopGroupDb) TEST_F(FpmSyncdResponseTest, TestDeleteNextHopGroup) { // Setup test groups - NextHopGroup nhg1(1, "192.168.1.1", "Ethernet0"); - NextHopGroup nhg2(2, "192.168.1.2", "Ethernet1"); + NextHopGroup nhg1(1, test_gateway, "Ethernet0"); + NextHopGroup nhg2(2, test_gateway_, "Ethernet1"); nhg1.installed = true; nhg2.installed = true; - + m_mockRouteSync.m_nh_groups.insert({1, nhg1}); m_mockRouteSync.m_nh_groups.insert({2, nhg2}); - + // Test deletion m_mockRouteSync.deleteNextHopGroup(1); EXPECT_EQ(m_mockRouteSync.m_nh_groups.find(1), m_mockRouteSync.m_nh_groups.end()); @@ -644,7 +645,7 @@ struct nlmsghdr* createNewNextHopMsgHdr(const vector>& g rta = (struct rtattr *)((char *)nlh + NLMSG_ALIGN(nlh->nlmsg_len)); rta->rta_type = NHA_GROUP; struct nexthop_grp* grp = (struct nexthop_grp*)malloc(group_members.size() * sizeof(struct nexthop_grp)); - + for (size_t i = 0; i < group_members.size(); i++) { grp[i].id = group_members[i].first; grp[i].weight = group_members[i].second - 1; // kernel stores weight-1 @@ -671,9 +672,9 @@ TEST_F(FpmSyncdResponseTest, TestNextHopGroupAdd) uint32_t nh2_id = 2; uint32_t nh3_id = 3; - struct nlmsghdr* nlh1 = createNewNextHopMsgHdr(1, "192.168.1.1", nh1_id); - struct nlmsghdr* nlh2 = createNewNextHopMsgHdr(2, "192.168.1.2", nh2_id); - struct nlmsghdr* nlh3 = createNewNextHopMsgHdr(3, "192.168.1.3", nh3_id); + struct nlmsghdr* nlh1 = createNewNextHopMsgHdr(1, test_gateway, nh1_id); + struct nlmsghdr* nlh2 = createNewNextHopMsgHdr(2, test_gateway_, nh2_id); + struct nlmsghdr* nlh3 = createNewNextHopMsgHdr(3, test_gateway__, nh3_id); EXPECT_CALL(m_mockRouteSync, getIfName(1, _, _)) .WillOnce(DoAll( @@ -683,7 +684,7 @@ TEST_F(FpmSyncdResponseTest, TestNextHopGroupAdd) }, Return(true) )); - + EXPECT_CALL(m_mockRouteSync, getIfName(2, _, _)) .WillOnce(DoAll( [](int32_t, char* ifname, size_t size) { @@ -692,7 +693,7 @@ TEST_F(FpmSyncdResponseTest, TestNextHopGroupAdd) }, Return(true) )); - + EXPECT_CALL(m_mockRouteSync, getIfName(3, _, _)) .WillOnce(DoAll( [](int32_t, char* ifname, size_t size) { @@ -721,11 +722,11 @@ TEST_F(FpmSyncdResponseTest, TestNextHopGroupAdd) // Verify the group was added correctly auto it = m_mockRouteSync.m_nh_groups.find(group_id); ASSERT_NE(it, m_mockRouteSync.m_nh_groups.end()) << "Failed to add nexthop group"; - + // Verify group members const auto& group = it->second.group; ASSERT_EQ(group.size(), 3) << "Wrong number of group members"; - + // Check each member's ID and weight EXPECT_EQ(group[0].first, nh1_id); EXPECT_EQ(group[0].second, 1); @@ -736,14 +737,14 @@ TEST_F(FpmSyncdResponseTest, TestNextHopGroupAdd) // Mark the group as installed and verify DB update m_mockRouteSync.updateNextHopGroup(group_id); - + Table nexthop_group_table(m_db.get(), APP_NEXTHOP_GROUP_TABLE_NAME); vector fieldValues; string key = "ID" + to_string(group_id); nexthop_group_table.get(key, fieldValues); - + ASSERT_EQ(fieldValues.size(), 3) << "Wrong number of fields in DB"; - + // Verify the DB fields string nexthops, ifnames, weights; for (const auto& fv : fieldValues) { @@ -755,7 +756,7 @@ TEST_F(FpmSyncdResponseTest, TestNextHopGroupAdd) weights = fvValue(fv); } } - + EXPECT_EQ(nexthops, "192.168.1.1,192.168.1.2,192.168.1.3"); EXPECT_EQ(ifnames, "Ethernet1,Ethernet2,Ethernet3"); EXPECT_EQ(weights, "1,2,3"); @@ -766,3 +767,140 @@ TEST_F(FpmSyncdResponseTest, TestNextHopGroupAdd) free(nlh3); free(group_nlh); } + +TEST_F(FpmSyncdResponseTest, TestRouteMsgWithNHG) +{ + Table route_table(m_db.get(), APP_ROUTE_TABLE_NAME); + auto createRoute = [](const char* prefix, uint8_t prefixlen) -> rtnl_route* { + rtnl_route* route = rtnl_route_alloc(); + nl_addr* dst_addr; + nl_addr_parse(prefix, AF_INET, &dst_addr); + rtnl_route_set_dst(route, dst_addr); + rtnl_route_set_type(route, RTN_UNICAST); + rtnl_route_set_protocol(route, RTPROT_STATIC); + rtnl_route_set_family(route, AF_INET); + rtnl_route_set_scope(route, RT_SCOPE_UNIVERSE); + rtnl_route_set_table(route, RT_TABLE_MAIN); + nl_addr_put(dst_addr); + return route; + }; + + uint32_t test_nh_id = 1; + uint32_t test_nhg_id = 2; + uint32_t test_nh_id_ = 3; + uint32_t test_nh_id__ = 4; + + // create a route + const char* test_destipprefix = "10.1.1.0"; + rtnl_route* test_route = createRoute(test_destipprefix, 24); + + // Test 1: use a non-existent nh_id + { + rtnl_route_set_nh_id(test_route, test_nh_id); + + m_mockRouteSync.onRouteMsg(RTM_NEWROUTE, (nl_object*)test_route, nullptr); + + vector keys; + route_table.getKeys(keys); + + // verify the route is discarded + EXPECT_TRUE(std::find(keys.begin(), keys.end(), test_destipprefix) == keys.end()); + } + + // Test 2: using a nexthop + { + // create the nexthop + struct nlmsghdr* nlh = createNewNextHopMsgHdr(1, test_gateway, test_nh_id); + + EXPECT_CALL(m_mockRouteSync, getIfName(1, _, _)) + .WillOnce(DoAll( + [](int32_t, char* ifname, size_t size) { + strncpy(ifname, "Ethernet1", size); + ifname[size-1] = '\0'; + }, + Return(true) + )); + + m_mockRouteSync.onNextHopMsg(nlh, (int)(nlh->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg)))); + + free(nlh); + + rtnl_route_set_nh_id(test_route, test_nh_id); + + m_mockRouteSync.onRouteMsg(RTM_NEWROUTE, (nl_object*)test_route, nullptr); + + vector fvs; + EXPECT_TRUE(route_table.get(test_destipprefix, fvs)); + EXPECT_EQ(fvs.size(), 3); + for (const auto& fv : fvs) { + if (fvField(fv) == "nexthop") { + EXPECT_EQ(fvValue(fv), test_gateway); + } else if (fvField(fv) == "ifname") { + EXPECT_EQ(fvValue(fv), "Ethernet1"); + } else if (fvField(fv) == "protocol") { + EXPECT_EQ(fvValue(fv), "static"); + } + } + } + + // Test 3: using an nhg + { + struct nlmsghdr* nlh1 = createNewNextHopMsgHdr(2, test_gateway_, test_nh_id_); + struct nlmsghdr* nlh2 = createNewNextHopMsgHdr(3, test_gateway__, test_nh_id__); + + EXPECT_CALL(m_mockRouteSync, getIfName(2, _, _)) + .WillOnce(DoAll( + [](int32_t, char* ifname, size_t size) { + strncpy(ifname, "Ethernet2", size); + ifname[size-1] = '\0'; + }, + Return(true) + )); + + EXPECT_CALL(m_mockRouteSync, getIfName(3, _, _)) + .WillOnce(DoAll( + [](int32_t, char* ifname, size_t size) { + strncpy(ifname, "Ethernet3", size); + ifname[size-1] = '\0'; + }, + Return(true) + )); + + m_mockRouteSync.onNextHopMsg(nlh1, (int)(nlh1->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg)))); + m_mockRouteSync.onNextHopMsg(nlh2, (int)(nlh2->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg)))); + + vector> group_members = { + {test_nh_id_, 1}, + {test_nh_id__, 2} + }; + + struct nlmsghdr* group_nlh = createNewNextHopMsgHdr(group_members, test_nhg_id); + m_mockRouteSync.onNextHopMsg(group_nlh, (int)(group_nlh->nlmsg_len - NLMSG_LENGTH(sizeof(struct nhmsg)))); + + // create the route object referring to this next hop group + rtnl_route_set_nh_id(test_route, test_nhg_id); + m_mockRouteSync.onRouteMsg(RTM_NEWROUTE, (nl_object*)test_route, nullptr); + + vector fvs; + EXPECT_TRUE(route_table.get(test_destipprefix, fvs)); + + for (const auto& fv : fvs) { + if (fvField(fv) == "nexthop_group") { + EXPECT_EQ(fvValue(fv), "ID2"); + } else if (fvField(fv) == "protocol") { + EXPECT_EQ(fvValue(fv), "static"); + } + } + + vector group_fvs; + Table nexthop_group_table(m_db.get(), APP_NEXTHOP_GROUP_TABLE_NAME); + EXPECT_TRUE(nexthop_group_table.get("ID2", group_fvs)); + + // clean up + free(nlh1); + free(nlh2); + free(group_nlh); + } + + rtnl_route_put(test_route); +} From ffdd4706a1380f64d5bebd5b784304f1c7bf9108 Mon Sep 17 00:00:00 2001 From: Kanji Nakano Date: Wed, 11 Dec 2024 10:36:16 +0900 Subject: [PATCH 13/15] update nexthop group support Signed-off-by: Kanji Nakano --- fpmsyncd/routesync.cpp | 24 +++++++++++--------- fpmsyncd/routesync.h | 2 +- tests/mock_tests/fpmsyncd/test_routesync.cpp | 6 ++--- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index 565e987ece..4f4429a81d 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -1678,22 +1678,25 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) NextHopGroup& nhg = itg->second; if(nhg.group.size() == 0) { - //Using route-table only for single next-hop - string nexthops, ifnames, weights; + // Using route-table only for single next-hop + string nexthops = nhg.nexthop.empty() ? (rtnl_route_get_family(route_obj) == AF_INET ? "0.0.0.0" : "::") : nhg.nexthop; + string ifnames, weights; - getNextHopGroupFields(nhg, nexthops, ifnames, weights, rtnl_route_get_family(route_obj)); - FieldValueTuple gw("nexthop", nexthops.c_str()); - FieldValueTuple intf("ifname", ifnames.c_str()); - fvVector.push_back(gw); - fvVector.push_back(intf); - SWSS_LOG_DEBUG("NextHop group id %d is a single nexthop address. Filling the route table %s with nexthop and ifname", nhg_id, destipprefix); + getNextHopGroupFields(nhg, nexthops, ifnames, weights, rtnl_route_get_family(route_obj)); + + FieldValueTuple gw("nexthop", nexthops.c_str()); + FieldValueTuple intf("ifname", ifnames.c_str()); + fvVector.push_back(gw); + fvVector.push_back(intf); + + SWSS_LOG_DEBUG("NextHop group id %d is a single nexthop address. Filling the route table %s with nexthop and ifname", nhg_id, destipprefix); } else { nhg_id_key = getNextHopGroupKeyAsString(nhg_id); FieldValueTuple nhg("nexthop_group", nhg_id_key.c_str()); fvVector.push_back(nhg); - updateNextHopGroup(nhg_id); + installNextHopGroup(nhg_id); } auto proto_num = rtnl_route_get_protocol(route_obj); @@ -2635,7 +2638,7 @@ const string RouteSync::getNextHopGroupKeyAsString(uint32_t id) const * @arg nh_id nexthop group id * */ -void RouteSync::updateNextHopGroup(uint32_t nh_id) +void RouteSync::installNextHopGroup(uint32_t nh_id) { auto git = m_nh_groups.find(nh_id); if(git == m_nh_groups.end()) @@ -2705,7 +2708,6 @@ void RouteSync::updateNextHopGroupDb(const NextHopGroup& nhg) } SWSS_LOG_INFO("NextHopGroup table set: key [%s] nexthop[%s] ifname[%s] weight[%s]", key.c_str(), nexthops.c_str(), ifnames.c_str(), weights.c_str()); - //TODO: Take care of warm reboot m_nexthop_groupTable.set(key.c_str(), fvVector); } diff --git a/fpmsyncd/routesync.h b/fpmsyncd/routesync.h index f009d50110..fbc042d009 100644 --- a/fpmsyncd/routesync.h +++ b/fpmsyncd/routesync.h @@ -188,7 +188,7 @@ class RouteSync : public NetMsg void onNextHopMsg(struct nlmsghdr *h, int len); /* Get next hop group key */ const string getNextHopGroupKeyAsString(uint32_t id) const; - void updateNextHopGroup(uint32_t nh_id); + void installNextHopGroup(uint32_t nh_id); void deleteNextHopGroup(uint32_t nh_id); void updateNextHopGroupDb(const NextHopGroup& nhg); void getNextHopGroupFields(const NextHopGroup& nhg, string& nexthops, string& ifnames, string& weights, uint8_t af = AF_INET); diff --git a/tests/mock_tests/fpmsyncd/test_routesync.cpp b/tests/mock_tests/fpmsyncd/test_routesync.cpp index f68d4ad7be..28d0449386 100644 --- a/tests/mock_tests/fpmsyncd/test_routesync.cpp +++ b/tests/mock_tests/fpmsyncd/test_routesync.cpp @@ -388,7 +388,7 @@ TEST_F(FpmSyncdResponseTest, TestIPv6NextHopAdd) ASSERT_TRUE(fieldValues.empty()); // Update the nexthop group to mark it as installed and write to DB - m_mockRouteSync.updateNextHopGroup(test_id); + m_mockRouteSync.installNextHopGroup(test_id); nexthop_group_table.get(key, fieldValues); string nexthop, ifname; @@ -553,7 +553,7 @@ TEST_F(FpmSyncdResponseTest, TestUpdateNextHopGroupDb) m_mockRouteSync.m_nh_groups.insert({2, nhg2}); m_mockRouteSync.m_nh_groups.insert({3, group}); - m_mockRouteSync.updateNextHopGroup(3); + m_mockRouteSync.installNextHopGroup(3); auto it = m_mockRouteSync.m_nh_groups.find(3); ASSERT_NE(it, m_mockRouteSync.m_nh_groups.end()); @@ -736,7 +736,7 @@ TEST_F(FpmSyncdResponseTest, TestNextHopGroupAdd) EXPECT_EQ(group[2].second, 3); // Mark the group as installed and verify DB update - m_mockRouteSync.updateNextHopGroup(group_id); + m_mockRouteSync.installNextHopGroup(group_id); Table nexthop_group_table(m_db.get(), APP_NEXTHOP_GROUP_TABLE_NAME); vector fieldValues; From f017e8c90abea61474e2e6ac3f9686ef409aa1a8 Mon Sep 17 00:00:00 2001 From: Kanji Nakano Date: Wed, 29 Jan 2025 09:55:02 +0900 Subject: [PATCH 14/15] update nhg Signed-off-by: Kanji Nakano --- fpmsyncd/routesync.cpp | 24 +++++++++++--------- tests/mock_tests/fpmsyncd/test_routesync.cpp | 20 ++++++++-------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index 4f4429a81d..ac88c66ac5 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -1672,7 +1672,7 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) const auto itg = m_nh_groups.find(nhg_id); if(itg == m_nh_groups.end()) { - SWSS_LOG_DEBUG("NextHop group id %d not found. Dropping the route %s", nhg_id, destipprefix); + SWSS_LOG_ERROR("NextHop group id %d not found. Dropping the route %s", nhg_id, destipprefix); return; } NextHopGroup& nhg = itg->second; @@ -1866,10 +1866,11 @@ void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) SWSS_LOG_INFO("New nexthop group message!"); struct nexthop_grp *nha_grp = (struct nexthop_grp *)RTA_DATA(tb[NHA_GROUP]); grp_count = (int)(RTA_PAYLOAD(tb[NHA_GROUP]) / sizeof(*nha_grp)); - - if(grp_count > MAX_MULTIPATH_NUM) + if (grp_count > MAX_MULTIPATH_NUM) + { + SWSS_LOG_ERROR("Nexthop group count (%d) exceeds the maximum allowed (%d). Clamping to maximum.", grp_count, MAX_MULTIPATH_NUM); grp_count = MAX_MULTIPATH_NUM; - + } for (int i = 0; i < grp_count; i++) { grp[i].id = nha_grp[i].id; /* @@ -1917,19 +1918,20 @@ void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) } } } - if(grp_count) + if (grp_count > 0) { - vector> group; - for(int i = 0; i < grp_count; i++) + vector> group(grp_count); + for (int i = 0; i < grp_count; i++) { - group.push_back(std::make_pair(grp[i].id, grp[i].weight)); + group[i] = std::make_pair(grp[i].id, grp[i].weight); } + auto it = m_nh_groups.find(id); - if(it != m_nh_groups.end()) + if (it != m_nh_groups.end()) { NextHopGroup &nhg = it->second; nhg.group = group; - if(nhg.installed) + if (nhg.installed) { updateNextHopGroupDb(nhg); } @@ -2630,7 +2632,7 @@ void RouteSync::onWarmStartEnd(DBConnector& applStateDb) */ const string RouteSync::getNextHopGroupKeyAsString(uint32_t id) const { - return string("ID") + to_string(id); + return to_string(id); } /* diff --git a/tests/mock_tests/fpmsyncd/test_routesync.cpp b/tests/mock_tests/fpmsyncd/test_routesync.cpp index 28d0449386..147900a48e 100644 --- a/tests/mock_tests/fpmsyncd/test_routesync.cpp +++ b/tests/mock_tests/fpmsyncd/test_routesync.cpp @@ -381,7 +381,7 @@ TEST_F(FpmSyncdResponseTest, TestIPv6NextHopAdd) Table nexthop_group_table(m_db.get(), APP_NEXTHOP_GROUP_TABLE_NAME); vector fieldValues; - string key = "ID" + to_string(test_id); + string key = to_string(test_id); nexthop_group_table.get(key, fieldValues); // onNextHopMsg only updates m_nh_groups unless the nhg is marked as installed @@ -452,8 +452,8 @@ TEST_F(FpmSyncdResponseTest, TestSkipSpecialInterfaces) TEST_F(FpmSyncdResponseTest, TestNextHopGroupKeyString) { - EXPECT_EQ(m_mockRouteSync.getNextHopGroupKeyAsString(1), "ID1"); - EXPECT_EQ(m_mockRouteSync.getNextHopGroupKeyAsString(1234), "ID1234"); + EXPECT_EQ(m_mockRouteSync.getNextHopGroupKeyAsString(1), "1"); + EXPECT_EQ(m_mockRouteSync.getNextHopGroupKeyAsString(1234), "1234"); } TEST_F(FpmSyncdResponseTest, TestGetNextHopGroupFields) @@ -530,7 +530,7 @@ TEST_F(FpmSyncdResponseTest, TestUpdateNextHopGroupDb) m_mockRouteSync.updateNextHopGroupDb(nhg); vector fieldValues; - nexthop_group_table.get("ID1", fieldValues); + nexthop_group_table.get("1", fieldValues); EXPECT_EQ(fieldValues.size(), 2); EXPECT_EQ(fvField(fieldValues[0]), "nexthop"); @@ -559,7 +559,7 @@ TEST_F(FpmSyncdResponseTest, TestUpdateNextHopGroupDb) ASSERT_NE(it, m_mockRouteSync.m_nh_groups.end()); EXPECT_TRUE(it->second.installed); vector fieldValues; - nexthop_group_table.get("ID3", fieldValues); + nexthop_group_table.get("3", fieldValues); EXPECT_EQ(fieldValues.size(), 3); EXPECT_EQ(fvField(fieldValues[0]), "nexthop"); EXPECT_EQ(fvValue(fieldValues[0]), "192.168.1.1,192.168.1.2"); @@ -575,7 +575,7 @@ TEST_F(FpmSyncdResponseTest, TestUpdateNextHopGroupDb) m_mockRouteSync.updateNextHopGroupDb(nhg); vector fieldValues; - nexthop_group_table.get("ID4", fieldValues); + nexthop_group_table.get("4", fieldValues); EXPECT_EQ(fieldValues.size(), 2); EXPECT_EQ(fvField(fieldValues[0]), "nexthop"); @@ -590,7 +590,7 @@ TEST_F(FpmSyncdResponseTest, TestUpdateNextHopGroupDb) m_mockRouteSync.updateNextHopGroupDb(nhg); vector fieldValues; - nexthop_group_table.get("ID5", fieldValues); + nexthop_group_table.get("5", fieldValues); EXPECT_EQ(fieldValues.size(), 2); EXPECT_EQ(fvField(fieldValues[0]), "nexthop"); @@ -740,7 +740,7 @@ TEST_F(FpmSyncdResponseTest, TestNextHopGroupAdd) Table nexthop_group_table(m_db.get(), APP_NEXTHOP_GROUP_TABLE_NAME); vector fieldValues; - string key = "ID" + to_string(group_id); + string key = to_string(group_id); nexthop_group_table.get(key, fieldValues); ASSERT_EQ(fieldValues.size(), 3) << "Wrong number of fields in DB"; @@ -886,7 +886,7 @@ TEST_F(FpmSyncdResponseTest, TestRouteMsgWithNHG) for (const auto& fv : fvs) { if (fvField(fv) == "nexthop_group") { - EXPECT_EQ(fvValue(fv), "ID2"); + EXPECT_EQ(fvValue(fv), "2"); } else if (fvField(fv) == "protocol") { EXPECT_EQ(fvValue(fv), "static"); } @@ -894,7 +894,7 @@ TEST_F(FpmSyncdResponseTest, TestRouteMsgWithNHG) vector group_fvs; Table nexthop_group_table(m_db.get(), APP_NEXTHOP_GROUP_TABLE_NAME); - EXPECT_TRUE(nexthop_group_table.get("ID2", group_fvs)); + EXPECT_TRUE(nexthop_group_table.get("2", group_fvs)); // clean up free(nlh1); From 1534ffd069ef77c19947a2d685a7cbf26e0c5a77 Mon Sep 17 00:00:00 2001 From: Kanji Nakano Date: Fri, 7 Feb 2025 12:57:58 +0900 Subject: [PATCH 15/15] update nhg Signed-off-by: Kanji Nakano --- fpmsyncd/routesync.cpp | 66 +++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 39 deletions(-) diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp index ac88c66ac5..7c2afe5597 100644 --- a/fpmsyncd/routesync.cpp +++ b/fpmsyncd/routesync.cpp @@ -1822,7 +1822,7 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf) /* * Handle Nexthop msg - * @arg nlmsghdr Netlink message + * @arg nlmsghdr Netlink messaged */ void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) { @@ -1833,7 +1833,6 @@ void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) string ifname; struct nhmsg *nhm = NULL; struct rtattr *tb[NHA_MAX + 1] = {}; - struct nexthop_grp grp[MAX_MULTIPATH_NUM]; struct in_addr ipv4 = {0}; struct in6_addr ipv6 = {0}; char gateway[INET6_ADDRSTRLEN] = {0}; @@ -1861,23 +1860,38 @@ void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) if (nlmsg_type == RTM_NEWNEXTHOP) { - if(tb[NHA_GROUP]) + if (tb[NHA_GROUP]) { SWSS_LOG_INFO("New nexthop group message!"); + struct nexthop_grp *nha_grp = (struct nexthop_grp *)RTA_DATA(tb[NHA_GROUP]); grp_count = (int)(RTA_PAYLOAD(tb[NHA_GROUP]) / sizeof(*nha_grp)); + if (grp_count > MAX_MULTIPATH_NUM) { SWSS_LOG_ERROR("Nexthop group count (%d) exceeds the maximum allowed (%d). Clamping to maximum.", grp_count, MAX_MULTIPATH_NUM); grp_count = MAX_MULTIPATH_NUM; } - for (int i = 0; i < grp_count; i++) { - grp[i].id = nha_grp[i].id; - /* - The minimum weight value is 1, but kernel store it as zero (https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/ip/iproute.c?h=v5.19.0#n1028). - Adding one to weight to write the right value to the database. - */ - grp[i].weight = nha_grp[i].weight + 1; + + vector> group(grp_count); + for (int i = 0; i < grp_count; i++) + { + group[i] = std::make_pair(nha_grp[i].id, nha_grp[i].weight + 1); + } + + auto it = m_nh_groups.find(id); + if (it != m_nh_groups.end()) + { + NextHopGroup &nhg = it->second; + nhg.group = group; + if (nhg.installed) + { + updateNextHopGroupDb(nhg); + } + } + else + { + m_nh_groups.insert({id, NextHopGroup(id, group)}); } } else @@ -1896,13 +1910,12 @@ void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) } else { - SWSS_LOG_ERROR( - "Unexpected nexthop address family"); + SWSS_LOG_ERROR("Unexpected nexthop address family"); return; } } - if(tb[NHA_OIF]) + if (tb[NHA_OIF]) { ifindex = *((int32_t *)RTA_DATA(tb[NHA_OIF])); char if_name[IFNAMSIZ] = {0}; @@ -1913,36 +1926,11 @@ void RouteSync::onNextHopMsg(struct nlmsghdr *h, int len) ifname = string(if_name); if (ifname == "eth0" || ifname == "docker0") { - SWSS_LOG_DEBUG("Skip routes to inteface: %s id[%d]", ifname.c_str(), id); + SWSS_LOG_DEBUG("Skip routes to interface: %s id[%d]", ifname.c_str(), id); return; } } - } - if (grp_count > 0) - { - vector> group(grp_count); - for (int i = 0; i < grp_count; i++) - { - group[i] = std::make_pair(grp[i].id, grp[i].weight); - } - auto it = m_nh_groups.find(id); - if (it != m_nh_groups.end()) - { - NextHopGroup &nhg = it->second; - nhg.group = group; - if (nhg.installed) - { - updateNextHopGroupDb(nhg); - } - } - else - { - m_nh_groups.insert({id, NextHopGroup(id, group)}); - } - } - else - { SWSS_LOG_DEBUG("Received: id[%d], if[%d/%s] address[%s]", id, ifindex, ifname.c_str(), gateway); m_nh_groups.insert({id, NextHopGroup(id, string(gateway), ifname)}); }