diff --git a/orchagent/p4orch/gre_tunnel_manager.cpp b/orchagent/p4orch/gre_tunnel_manager.cpp index 84f48a57b928..f98249950419 100644 --- a/orchagent/p4orch/gre_tunnel_manager.cpp +++ b/orchagent/p4orch/gre_tunnel_manager.cpp @@ -89,9 +89,10 @@ std::vector getSaiAttrs(const P4GreTunnelEntry &gre_tunnel_entr } // namespace P4GreTunnelEntry::P4GreTunnelEntry(const std::string &tunnel_id, const std::string &router_interface_id, - const swss::IpAddress &encap_src_ip, const swss::IpAddress &encap_dst_ip) + const swss::IpAddress &encap_src_ip, const swss::IpAddress &encap_dst_ip, + const swss::IpAddress &neighbor_id) : tunnel_id(tunnel_id), router_interface_id(router_interface_id), encap_src_ip(encap_src_ip), - encap_dst_ip(encap_dst_ip) + encap_dst_ip(encap_dst_ip), neighbor_id(neighbor_id) { SWSS_LOG_ENTER(); tunnel_key = KeyGenerator::generateTunnelKey(tunnel_id); @@ -188,7 +189,7 @@ P4GreTunnelEntry *GreTunnelManager::getGreTunnelEntry(const std::string &tunnel_ } }; -ReturnCodeOr GreTunnelManager::getUnderlayIfFromGreTunnelEntry(const std::string &tunnel_key) +ReturnCodeOr GreTunnelManager::getConstGreTunnelEntry(const std::string &tunnel_key) { SWSS_LOG_ENTER(); @@ -200,7 +201,7 @@ ReturnCodeOr GreTunnelManager::getUnderlayIfFromGreTunnelEntr } else { - return tunnel->router_interface_id; + return *tunnel; } } @@ -274,7 +275,7 @@ ReturnCode GreTunnelManager::processAddRequest(const P4GreTunnelAppDbEntry &app_ SWSS_LOG_ENTER(); P4GreTunnelEntry gre_tunnel_entry(app_db_entry.tunnel_id, app_db_entry.router_interface_id, - app_db_entry.encap_src_ip, app_db_entry.encap_dst_ip); + app_db_entry.encap_src_ip, app_db_entry.encap_dst_ip, app_db_entry.encap_dst_ip); auto status = createGreTunnel(gre_tunnel_entry); if (!status.ok()) { @@ -570,6 +571,15 @@ std::string GreTunnelManager::verifyStateCache(const P4GreTunnelAppDbEntry &app_ return msg.str(); } + if (gre_tunnel_entry->neighbor_id.to_string() != app_db_entry.encap_dst_ip.to_string()) + { + std::stringstream msg; + msg << "GreTunnel " << QuotedVar(app_db_entry.tunnel_id) << " with destination IP " + << QuotedVar(app_db_entry.encap_dst_ip.to_string()) << " does not match internal cache " + << QuotedVar(gre_tunnel_entry->neighbor_id.to_string()) << " fo neighbor_id in GreTunnel manager."; + return msg.str(); + } + return m_p4OidMapper->verifyOIDMapping(SAI_OBJECT_TYPE_TUNNEL, gre_tunnel_entry->tunnel_key, gre_tunnel_entry->tunnel_oid); } @@ -616,4 +626,4 @@ std::string GreTunnelManager::verifyStateAsicDb(const P4GreTunnelEntry *gre_tunn return verifyAttrs(values, exp, std::vector{}, /*allow_unknown=*/false); -} \ No newline at end of file +} diff --git a/orchagent/p4orch/gre_tunnel_manager.h b/orchagent/p4orch/gre_tunnel_manager.h index deb5b319e348..c1f17f102417 100644 --- a/orchagent/p4orch/gre_tunnel_manager.h +++ b/orchagent/p4orch/gre_tunnel_manager.h @@ -36,6 +36,9 @@ struct P4GreTunnelEntry std::string router_interface_id; swss::IpAddress encap_src_ip; swss::IpAddress encap_dst_ip; + // neighbor_id is required to be equal to encap_dst_ip by BRCM. And the + // neighbor entry needs to be created before GRE tunnel object + swss::IpAddress neighbor_id; // SAI OID associated with this entry. sai_object_id_t tunnel_oid = SAI_NULL_OBJECT_ID; @@ -45,7 +48,8 @@ struct P4GreTunnelEntry sai_object_id_t underlay_if_oid = SAI_NULL_OBJECT_ID; P4GreTunnelEntry(const std::string &tunnel_id, const std::string &router_interface_id, - const swss::IpAddress &encap_src_ip, const swss::IpAddress &encap_dst_ip); + const swss::IpAddress &encap_src_ip, const swss::IpAddress &encap_dst_ip, + const swss::IpAddress &neighbor_id); }; // GreTunnelManager listens to changes in table APP_P4RT_TUNNEL_TABLE_NAME and @@ -69,7 +73,7 @@ class GreTunnelManager : public ObjectManagerInterface void drain() override; std::string verifyState(const std::string &key, const std::vector &tuple) override; - ReturnCodeOr getUnderlayIfFromGreTunnelEntry(const std::string &gre_tunnel_key); + ReturnCodeOr getConstGreTunnelEntry(const std::string &gre_tunnel_key); private: // Gets the internal cached GRE tunnel entry by its key. diff --git a/orchagent/p4orch/next_hop_manager.cpp b/orchagent/p4orch/next_hop_manager.cpp index 2a9bbcf8f941..3ff7d3d3d638 100644 --- a/orchagent/p4orch/next_hop_manager.cpp +++ b/orchagent/p4orch/next_hop_manager.cpp @@ -41,18 +41,22 @@ namespace ReturnCode validateAppDbEntry(const P4NextHopAppDbEntry &app_db_entry) { + // TODO(b/225242372): remove kSetNexthop action after P4RT and Orion update + // naming if (app_db_entry.action_str != p4orch::kSetIpNexthop && app_db_entry.action_str != p4orch::kSetNexthop && app_db_entry.action_str != p4orch::kSetTunnelNexthop) { return ReturnCode(StatusCode::SWSS_RC_INVALID_PARAM) << "Invalid action " << QuotedVar(app_db_entry.action_str) << " of Nexthop App DB entry"; } - if (app_db_entry.neighbor_id.isZero()) + if (app_db_entry.action_str == p4orch::kSetIpNexthop && app_db_entry.neighbor_id.isZero()) { return ReturnCode(StatusCode::SWSS_RC_INVALID_PARAM) << "Missing field " << QuotedVar(prependParamField(p4orch::kNeighborId)) << " for action " << QuotedVar(p4orch::kSetIpNexthop) << " in table entry"; } + // TODO(b/225242372): remove kSetNexthop action after P4RT and Orion update + // naming if (app_db_entry.action_str == p4orch::kSetIpNexthop || app_db_entry.action_str == p4orch::kSetNexthop) { if (!app_db_entry.gre_tunnel_id.empty()) @@ -321,23 +325,27 @@ ReturnCode NextHopManager::createNextHop(P4NextHopEntry &next_hop_entry) << " already exists in centralized mapper"); } - std::string router_interface_id = next_hop_entry.router_interface_id; if (!next_hop_entry.gre_tunnel_id.empty()) { - auto underlay_if_or = gP4Orch->getGreTunnelManager()->getUnderlayIfFromGreTunnelEntry( + auto gre_tunnel_or = gP4Orch->getGreTunnelManager()->getConstGreTunnelEntry( KeyGenerator::generateTunnelKey(next_hop_entry.gre_tunnel_id)); - if (!underlay_if_or.ok()) + if (!gre_tunnel_or.ok()) { LOG_ERROR_AND_RETURN(ReturnCode(StatusCode::SWSS_RC_NOT_FOUND) << "GRE Tunnel " << QuotedVar(next_hop_entry.gre_tunnel_id) << " does not exist in GRE Tunnel Manager"); } - router_interface_id = *underlay_if_or; + next_hop_entry.router_interface_id = (*gre_tunnel_or).router_interface_id; + // BRCM requires neighbor object to be created before GRE tunnel, referring + // to the one in GRE tunnel object when creating next_hop_entry_with + // setTunnelAction + next_hop_entry.neighbor_id = (*gre_tunnel_or).neighbor_id; } // Neighbor doesn't have OID and the IP addr needed in next hop creation is // neighbor_id, so only check neighbor existence in centralized mapper. - const auto neighbor_key = KeyGenerator::generateNeighborKey(router_interface_id, next_hop_entry.neighbor_id); + const auto neighbor_key = + KeyGenerator::generateNeighborKey(next_hop_entry.router_interface_id, next_hop_entry.neighbor_id); if (!m_p4OidMapper->existsOID(SAI_OBJECT_TYPE_NEIGHBOR_ENTRY, neighbor_key)) { LOG_ERROR_AND_RETURN(ReturnCode(StatusCode::SWSS_RC_NOT_FOUND) @@ -456,15 +464,15 @@ ReturnCode NextHopManager::removeNextHop(const std::string &next_hop_key) std::string router_interface_id = next_hop_entry->router_interface_id; if (!next_hop_entry->gre_tunnel_id.empty()) { - auto underlay_if_or = gP4Orch->getGreTunnelManager()->getUnderlayIfFromGreTunnelEntry( + auto gre_tunnel_or = gP4Orch->getGreTunnelManager()->getConstGreTunnelEntry( KeyGenerator::generateTunnelKey(next_hop_entry->gre_tunnel_id)); - if (!underlay_if_or.ok()) + if (!gre_tunnel_or.ok()) { LOG_ERROR_AND_RETURN(ReturnCode(StatusCode::SWSS_RC_NOT_FOUND) << "GRE Tunnel " << QuotedVar(next_hop_entry->gre_tunnel_id) << " does not exist in GRE Tunnel Manager"); } - router_interface_id = *underlay_if_or; + router_interface_id = (*gre_tunnel_or).router_interface_id; } m_p4OidMapper->decreaseRefCount( SAI_OBJECT_TYPE_NEIGHBOR_ENTRY, @@ -560,7 +568,8 @@ std::string NextHopManager::verifyStateCache(const P4NextHopAppDbEntry &app_db_e << QuotedVar(next_hop_entry->next_hop_id) << " in nexthop manager."; return msg.str(); } - if (next_hop_entry->router_interface_id != app_db_entry.router_interface_id) + if (app_db_entry.action_str == p4orch::kSetIpNexthop && + next_hop_entry->router_interface_id != app_db_entry.router_interface_id) { std::stringstream msg; msg << "Nexthop " << QuotedVar(app_db_entry.next_hop_id) << " with ritf ID " @@ -568,7 +577,8 @@ std::string NextHopManager::verifyStateCache(const P4NextHopAppDbEntry &app_db_e << QuotedVar(next_hop_entry->router_interface_id) << " in nexthop manager."; return msg.str(); } - if (next_hop_entry->neighbor_id.to_string() != app_db_entry.neighbor_id.to_string()) + if (app_db_entry.action_str == p4orch::kSetIpNexthop && + next_hop_entry->neighbor_id.to_string() != app_db_entry.neighbor_id.to_string()) { std::stringstream msg; msg << "Nexthop " << QuotedVar(app_db_entry.next_hop_id) << " with neighbor ID " @@ -577,7 +587,8 @@ std::string NextHopManager::verifyStateCache(const P4NextHopAppDbEntry &app_db_e return msg.str(); } - if (next_hop_entry->gre_tunnel_id != app_db_entry.gre_tunnel_id) + if (app_db_entry.action_str == p4orch::kSetTunnelNexthop && + next_hop_entry->gre_tunnel_id != app_db_entry.gre_tunnel_id) { std::stringstream msg; msg << "Nexthop " << QuotedVar(app_db_entry.next_hop_id) << " with GRE tunnel ID " @@ -585,6 +596,36 @@ std::string NextHopManager::verifyStateCache(const P4NextHopAppDbEntry &app_db_e << QuotedVar(next_hop_entry->gre_tunnel_id) << " in nexthop manager."; return msg.str(); } + if (!next_hop_entry->gre_tunnel_id.empty()) + { + auto gre_tunnel_or = gP4Orch->getGreTunnelManager()->getConstGreTunnelEntry( + KeyGenerator::generateTunnelKey(next_hop_entry->gre_tunnel_id)); + if (!gre_tunnel_or.ok()) + { + std::stringstream msg; + msg << "GRE Tunnel " << QuotedVar(next_hop_entry->gre_tunnel_id) << " does not exist in GRE Tunnel Manager"; + return msg.str(); + } + P4GreTunnelEntry gre_tunnel = *gre_tunnel_or; + if (gre_tunnel.neighbor_id.to_string() != next_hop_entry->neighbor_id.to_string()) + { + std::stringstream msg; + msg << "Nexthop " << QuotedVar(next_hop_entry->next_hop_id) << " with neighbor ID " + << QuotedVar(next_hop_entry->neighbor_id.to_string()) + << " in nexthop manager does not match internal cache " << QuotedVar(gre_tunnel.neighbor_id.to_string()) + << " with tunnel ID " << QuotedVar(gre_tunnel.tunnel_id) << " in GRE tunnel manager."; + return msg.str(); + } + if (gre_tunnel.router_interface_id != next_hop_entry->router_interface_id) + { + std::stringstream msg; + msg << "Nexthop " << QuotedVar(next_hop_entry->next_hop_id) << " with rif ID " + << QuotedVar(next_hop_entry->router_interface_id) + << " in nexthop manager does not match internal cache " << QuotedVar(gre_tunnel.router_interface_id) + << " with tunnel ID " << QuotedVar(gre_tunnel.tunnel_id) << " in GRE tunnel manager."; + return msg.str(); + } + } return m_p4OidMapper->verifyOIDMapping(SAI_OBJECT_TYPE_NEXT_HOP, next_hop_entry->next_hop_key, next_hop_entry->next_hop_oid); diff --git a/orchagent/p4orch/p4orch.cpp b/orchagent/p4orch/p4orch.cpp index 717f23bc9313..6a9beea83007 100644 --- a/orchagent/p4orch/p4orch.cpp +++ b/orchagent/p4orch/p4orch.cpp @@ -30,8 +30,8 @@ P4Orch::P4Orch(swss::DBConnector *db, std::vector tableNames, VRFOr SWSS_LOG_ENTER(); m_routerIntfManager = std::make_unique(&m_p4OidMapper, &m_publisher); - m_greTunnelManager = std::make_unique(&m_p4OidMapper, &m_publisher); m_neighborManager = std::make_unique(&m_p4OidMapper, &m_publisher); + m_greTunnelManager = std::make_unique(&m_p4OidMapper, &m_publisher); m_nextHopManager = std::make_unique(&m_p4OidMapper, &m_publisher); m_routeManager = std::make_unique(&m_p4OidMapper, vrfOrch, &m_publisher); m_mirrorSessionManager = std::make_unique(&m_p4OidMapper, &m_publisher); @@ -52,8 +52,8 @@ P4Orch::P4Orch(swss::DBConnector *db, std::vector tableNames, VRFOr m_p4TableToManagerMap[APP_P4RT_L3_ADMIT_TABLE_NAME] = m_l3AdmitManager.get(); m_p4ManagerPrecedence.push_back(m_routerIntfManager.get()); - m_p4ManagerPrecedence.push_back(m_greTunnelManager.get()); m_p4ManagerPrecedence.push_back(m_neighborManager.get()); + m_p4ManagerPrecedence.push_back(m_greTunnelManager.get()); m_p4ManagerPrecedence.push_back(m_nextHopManager.get()); m_p4ManagerPrecedence.push_back(m_wcmpManager.get()); m_p4ManagerPrecedence.push_back(m_routeManager.get()); diff --git a/orchagent/p4orch/p4orch_util.h b/orchagent/p4orch/p4orch_util.h index 995ccf9b2732..198442ef8171 100644 --- a/orchagent/p4orch/p4orch_util.h +++ b/orchagent/p4orch/p4orch_util.h @@ -40,7 +40,7 @@ constexpr char *kSetWcmpGroupIdAndMetadata = "set_wcmp_group_id_and_metadata"; constexpr char *kSetMetadataAndDrop = "set_metadata_and_drop"; constexpr char *kSetNexthop = "set_nexthop"; constexpr char *kSetIpNexthop = "set_ip_nexthop"; -constexpr char *kSetTunnelNexthop = "set_tunnel_encap_nexthop"; +constexpr char *kSetTunnelNexthop = "set_p2p_tunnel_encap_nexthop"; constexpr char *kDrop = "drop"; constexpr char *kTrap = "trap"; constexpr char *kStage = "stage"; @@ -79,7 +79,7 @@ constexpr char *kTtl = "ttl"; constexpr char *kTos = "tos"; constexpr char *kMirrorAsIpv4Erspan = "mirror_as_ipv4_erspan"; constexpr char *kL3AdmitAction = "admit_to_l3"; -constexpr char *kTunnelAction = "mark_for_tunnel_encap"; +constexpr char *kTunnelAction = "mark_for_p2p_tunnel_encap"; } // namespace p4orch // Prepends "match/" to the input string str to construct a new string. diff --git a/orchagent/p4orch/tests/gre_tunnel_manager_test.cpp b/orchagent/p4orch/tests/gre_tunnel_manager_test.cpp index ebd0b54ce42a..0177eca80bb4 100644 --- a/orchagent/p4orch/tests/gre_tunnel_manager_test.cpp +++ b/orchagent/p4orch/tests/gre_tunnel_manager_test.cpp @@ -52,7 +52,7 @@ const P4GreTunnelAppDbEntry kP4GreTunnelAppDbEntry1{/*tunnel_id=*/"tunnel-1", /*router_interface_id=*/"intf-eth-1/2/3", /*encap_src_ip=*/swss::IpAddress("2607:f8b0:8096:3110::1"), /*encap_dst_ip=*/swss::IpAddress("2607:f8b0:8096:311a::2"), - /*action_str=*/"mark_for_tunnel_encap"}; + /*action_str=*/"mark_for_p2p_tunnel_encap"}; std::unordered_map CreateAttributeListForGreTunnelObject( const P4GreTunnelAppDbEntry &app_entry, const sai_object_id_t &rif_oid) @@ -304,6 +304,7 @@ bool GreTunnelManagerTest::ValidateGreTunnelEntryAdd(const P4GreTunnelAppDbEntry const auto *p4_gre_tunnel_entry = GetGreTunnelEntry(KeyGenerator::generateTunnelKey(app_db_entry.tunnel_id)); if (p4_gre_tunnel_entry == nullptr || p4_gre_tunnel_entry->encap_src_ip != app_db_entry.encap_src_ip || p4_gre_tunnel_entry->encap_dst_ip != app_db_entry.encap_dst_ip || + p4_gre_tunnel_entry->neighbor_id != app_db_entry.encap_dst_ip || p4_gre_tunnel_entry->router_interface_id != app_db_entry.router_interface_id || p4_gre_tunnel_entry->tunnel_id != app_db_entry.tunnel_id) { @@ -334,7 +335,7 @@ TEST_F(GreTunnelManagerTest, ProcessAddRequestShouldFailWhenDependingPortIsNotPr /*router_interface_id=*/"intf-eth-1/2/3", /*encap_src_ip=*/swss::IpAddress("2607:f8b0:8096:3110::1"), /*encap_dst_ip=*/swss::IpAddress("2607:f8b0:8096:311a::2"), - /*action_str=*/"mark_for_tunnel_encap"}; + /*action_str=*/"mark_for_p2p_tunnel_encap"}; const auto gre_tunnel_key = KeyGenerator::generateTunnelKey(kAppDbEntry.tunnel_id); EXPECT_EQ(StatusCode::SWSS_RC_NOT_FOUND, ProcessAddRequest(kAppDbEntry)); @@ -817,6 +818,12 @@ TEST_F(GreTunnelManagerTest, VerifyStateTest) EXPECT_FALSE(VerifyState(db_key, attributes).empty()); p4_tunnel_entry->encap_dst_ip = saved_DST_IP; + // Verification should fail if IP mask mismatches. + auto saved_NEIGHBOR_ID = p4_tunnel_entry->neighbor_id; + p4_tunnel_entry->neighbor_id = swss::IpAddress("2.2.2.2"); + EXPECT_FALSE(VerifyState(db_key, attributes).empty()); + p4_tunnel_entry->neighbor_id = saved_NEIGHBOR_ID; + // Verification should fail if tunnel_id mismatches. auto saved_tunnel_id = p4_tunnel_entry->tunnel_id; p4_tunnel_entry->tunnel_id = "invalid"; diff --git a/orchagent/p4orch/tests/mock_sai_next_hop_group.h b/orchagent/p4orch/tests/mock_sai_next_hop_group.h index 5398ec5a7076..c1ffedc1759a 100644 --- a/orchagent/p4orch/tests/mock_sai_next_hop_group.h +++ b/orchagent/p4orch/tests/mock_sai_next_hop_group.h @@ -6,6 +6,7 @@ extern "C" { #include "sai.h" +#include "sainexthopgroup.h" } // Mock class including mock functions mapping to SAI next hop group's @@ -27,6 +28,16 @@ class MockSaiNextHopGroup MOCK_METHOD2(set_next_hop_group_member_attribute, sai_status_t(_In_ sai_object_id_t next_hop_group_member_id, _In_ const sai_attribute_t *attr)); + + MOCK_METHOD7(create_next_hop_group_members, + sai_status_t(_In_ sai_object_id_t switch_id, _In_ uint32_t object_count, + _In_ const uint32_t *attr_count, _In_ const sai_attribute_t **attr_list, + _In_ sai_bulk_op_error_mode_t mode, _Out_ sai_object_id_t *object_id, + _Out_ sai_status_t *object_statuses)); + + MOCK_METHOD4(remove_next_hop_group_members, + sai_status_t(_In_ uint32_t object_count, _In_ const sai_object_id_t *object_id, + _In_ sai_bulk_op_error_mode_t mode, _Out_ sai_status_t *object_statuses)); }; // Note that before mock functions below are used, mock_sai_next_hop_group must @@ -62,3 +73,18 @@ sai_status_t set_next_hop_group_member_attribute(_In_ sai_object_id_t next_hop_g { return mock_sai_next_hop_group->set_next_hop_group_member_attribute(next_hop_group_member_id, attr); } + +sai_status_t create_next_hop_group_members(_In_ sai_object_id_t switch_id, _In_ uint32_t object_count, + _In_ const uint32_t *attr_count, _In_ const sai_attribute_t **attr_list, + _In_ sai_bulk_op_error_mode_t mode, _Out_ sai_object_id_t *object_id, + _Out_ sai_status_t *object_statuses) +{ + return mock_sai_next_hop_group->create_next_hop_group_members(switch_id, object_count, attr_count, attr_list, mode, + object_id, object_statuses); +} + +sai_status_t remove_next_hop_group_members(_In_ uint32_t object_count, _In_ const sai_object_id_t *object_id, + _In_ sai_bulk_op_error_mode_t mode, _Out_ sai_status_t *object_statuses) +{ + return mock_sai_next_hop_group->remove_next_hop_group_members(object_count, object_id, mode, object_statuses); +} diff --git a/orchagent/p4orch/tests/next_hop_manager_test.cpp b/orchagent/p4orch/tests/next_hop_manager_test.cpp index 64416bb3b3ce..91c59a9b69b9 100644 --- a/orchagent/p4orch/tests/next_hop_manager_test.cpp +++ b/orchagent/p4orch/tests/next_hop_manager_test.cpp @@ -86,29 +86,32 @@ const P4NextHopAppDbEntry kP4NextHopAppDbEntry3{/*next_hop_id=*/kNextHopId, const P4NextHopAppDbEntry kP4TunnelNextHopAppDbEntry1{/*next_hop_id=*/kTunnelNextHopId, /*router_interface_id=*/"", /*gre_tunnel_id=*/kTunnelId1, - /*neighbor_id=*/swss::IpAddress(kNeighborId1), - /*action_str=*/"set_tunnel_encap_nexthop"}; + /*neighbor_id=*/swss::IpAddress("0.0.0.0"), + /*action_str=*/"set_p2p_tunnel_encap_nexthop"}; const P4NextHopAppDbEntry kP4TunnelNextHopAppDbEntry2{/*next_hop_id=*/kTunnelNextHopId, /*router_interface_id=*/"", /*gre_tunnel_id=*/kTunnelId2, - /*neighbor_id=*/swss::IpAddress(kNeighborId2), - /*action_str=*/"set_tunnel_encap_nexthop"}; + /*neighbor_id=*/swss::IpAddress("0.0.0.0"), + /*action_str=*/"set_p2p_tunnel_encap_nexthop"}; const P4GreTunnelEntry kP4TunnelEntry1( /*tunnel_id=*/kTunnelId1, /*router_interface_id=*/kRouterInterfaceId1, - /*src_ip=*/swss::IpAddress("1.2.3.4"), - /*dst_ip=*/swss::IpAddress("5.6.7.8")); + /*encap_src_ip=*/swss::IpAddress("1.2.3.4"), + /*encap_dst_ip=*/swss::IpAddress(kNeighborId1), + /*neighbor_id=*/swss::IpAddress(kNeighborId1)); const P4GreTunnelEntry kP4TunnelEntry2( /*tunnel_id=*/kTunnelId2, /*router_interface_id=*/kRouterInterfaceId2, - /*src_ip=*/swss::IpAddress("1.2.3.4"), - /*dst_ip=*/swss::IpAddress("5.6.7.8")); + /*encap_src_ip=*/swss::IpAddress("1.2.3.4"), + /*encap_dst_ip=*/swss::IpAddress(kNeighborId2), + /*neighbor_id=*/swss::IpAddress(kNeighborId2)); std::unordered_map CreateAttributeListForNextHopObject( - const P4NextHopAppDbEntry &app_entry, const sai_object_id_t &oid) + const P4NextHopAppDbEntry &app_entry, const sai_object_id_t &oid, + const swss::IpAddress &neighbor_id = swss::IpAddress("0.0.0.0")) { std::unordered_map next_hop_attrs; sai_attribute_t next_hop_attr; @@ -133,7 +136,14 @@ std::unordered_map CreateAttributeListForN } next_hop_attr.id = SAI_NEXT_HOP_ATTR_IP; - swss::copy(next_hop_attr.value.ipaddr, app_entry.neighbor_id); + if (!neighbor_id.isZero()) + { + swss::copy(next_hop_attr.value.ipaddr, neighbor_id); + } + else + { + swss::copy(next_hop_attr.value.ipaddr, app_entry.neighbor_id); + } next_hop_attrs.insert({next_hop_attr.id, next_hop_attr.value}); return next_hop_attrs; @@ -307,6 +317,12 @@ class NextHopManagerTest : public ::testing::Test // Returns a valid pointer to next hop entry on success. P4NextHopEntry *AddNextHopEntry1(); + // Adds the next hop entry -- kP4TunnelNextHopAppDbEntry1, via next hop + // manager's ProcessAddRequest (). This function also takes care of all the + // dependencies of the next hop entry. Returns a valid pointer to next hop + // entry on success. + P4NextHopEntry *AddTunnelNextHopEntry1(); + // Validates that a P4 App next hop entry is correctly added in next hop // manager and centralized mapper. Returns true on success. bool ValidateNextHopEntryAdd(const P4NextHopAppDbEntry &app_db_entry, const sai_object_id_t &expected_next_hop_oid); @@ -333,7 +349,8 @@ class NextHopManagerTest : public ::testing::Test bool NextHopManagerTest::ResolveNextHopEntryDependency(const P4NextHopAppDbEntry &app_db_entry, const sai_object_id_t &oid) { - std::string rif_id; + std::string rif_id = app_db_entry.router_interface_id; + auto neighbor_id = app_db_entry.neighbor_id; if (app_db_entry.action_str == p4orch::kSetTunnelNexthop) { const std::string tunnel_key = KeyGenerator::generateTunnelKey(app_db_entry.gre_tunnel_id); @@ -343,21 +360,27 @@ bool NextHopManagerTest::ResolveNextHopEntryDependency(const P4NextHopAppDbEntry } gP4Orch->getGreTunnelManager()->m_greTunnelTable.emplace( tunnel_key, app_db_entry.gre_tunnel_id == kTunnelId1 ? kP4TunnelEntry1 : kP4TunnelEntry2); - auto rif_id_or = gP4Orch->getGreTunnelManager()->getUnderlayIfFromGreTunnelEntry(tunnel_key); - EXPECT_TRUE(rif_id_or.ok()); - rif_id = *rif_id_or; + auto gre_tunnel_or = gP4Orch->getGreTunnelManager()->getConstGreTunnelEntry(tunnel_key); + EXPECT_TRUE(gre_tunnel_or.ok()); + rif_id = (*gre_tunnel_or).router_interface_id; + auto rif_oid = rif_id == kRouterInterfaceId1 ? kRouterInterfaceOid1 : kRouterInterfaceOid2; + neighbor_id = (*gre_tunnel_or).neighbor_id; + const std::string rif_key = KeyGenerator::generateRouterInterfaceKey(rif_id); + if (!p4_oid_mapper_.setOID(SAI_OBJECT_TYPE_ROUTER_INTERFACE, rif_key, rif_oid)) + { + return false; + } } else { - rif_id = app_db_entry.router_interface_id; - const std::string rif_key = KeyGenerator::generateRouterInterfaceKey(app_db_entry.router_interface_id); + const std::string rif_key = KeyGenerator::generateRouterInterfaceKey(rif_id); if (!p4_oid_mapper_.setOID(SAI_OBJECT_TYPE_ROUTER_INTERFACE, rif_key, oid)) { return false; } } - const std::string neighbor_key = KeyGenerator::generateNeighborKey(rif_id, app_db_entry.neighbor_id); + const std::string neighbor_key = KeyGenerator::generateNeighborKey(rif_id, neighbor_id); if (!p4_oid_mapper_.setDummyOID(SAI_OBJECT_TYPE_NEIGHBOR_ENTRY, neighbor_key)) { return false; @@ -385,12 +408,32 @@ P4NextHopEntry *NextHopManagerTest::AddNextHopEntry1() return GetNextHopEntry(KeyGenerator::generateNextHopKey(kP4NextHopAppDbEntry1.next_hop_id)); } +P4NextHopEntry *NextHopManagerTest::AddTunnelNextHopEntry1() +{ + if (!ResolveNextHopEntryDependency(kP4TunnelNextHopAppDbEntry1, kTunnelOid1)) + { + return nullptr; + } + + // Set up mock call. + EXPECT_CALL( + mock_sai_next_hop_, + create_next_hop(::testing::NotNull(), Eq(gSwitchId), Eq(3), + Truly(std::bind(MatchCreateNextHopArgAttrList, std::placeholders::_1, + CreateAttributeListForNextHopObject(kP4TunnelNextHopAppDbEntry1, kTunnelOid1, + swss::IpAddress(kNeighborId1)))))) + .WillOnce(DoAll(SetArgPointee<0>(kTunnelNextHopOid), Return(SAI_STATUS_SUCCESS))); + + EXPECT_EQ(StatusCode::SWSS_RC_SUCCESS, ProcessAddRequest(kP4TunnelNextHopAppDbEntry1)); + + return GetNextHopEntry(KeyGenerator::generateNextHopKey(kP4TunnelNextHopAppDbEntry1.next_hop_id)); +} + bool NextHopManagerTest::ValidateNextHopEntryAdd(const P4NextHopAppDbEntry &app_db_entry, const sai_object_id_t &expected_next_hop_oid) { const auto *p4_next_hop_entry = GetNextHopEntry(KeyGenerator::generateNextHopKey(app_db_entry.next_hop_id)); if (p4_next_hop_entry == nullptr || p4_next_hop_entry->next_hop_id != app_db_entry.next_hop_id || - p4_next_hop_entry->neighbor_id != app_db_entry.neighbor_id || p4_next_hop_entry->next_hop_oid != expected_next_hop_oid) { return false; @@ -403,7 +446,8 @@ bool NextHopManagerTest::ValidateNextHopEntryAdd(const P4NextHopAppDbEntry &app_ } if (app_db_entry.action_str == p4orch::kSetIpNexthop && - p4_next_hop_entry->router_interface_id != app_db_entry.router_interface_id) + (p4_next_hop_entry->router_interface_id != app_db_entry.router_interface_id || + p4_next_hop_entry->neighbor_id != app_db_entry.neighbor_id)) { return false; } @@ -467,8 +511,8 @@ TEST_F(NextHopManagerTest, ProcessAddRequestShouldFailWhenDependingRifIsAbsentIn TEST_F(NextHopManagerTest, ProcessAddRequestShouldFailWhenDependingTunnelIsAbsentInCentralMapper) { - const std::string neighbor_key = KeyGenerator::generateNeighborKey(kP4TunnelNextHopAppDbEntry1.router_interface_id, - kP4TunnelNextHopAppDbEntry1.neighbor_id); + const std::string neighbor_key = + KeyGenerator::generateNeighborKey(kP4TunnelNextHopAppDbEntry1.router_interface_id, kP4TunnelEntry1.neighbor_id); ASSERT_TRUE(p4_oid_mapper_.setDummyOID(SAI_OBJECT_TYPE_NEIGHBOR_ENTRY, neighbor_key)); EXPECT_EQ(StatusCode::SWSS_RC_NOT_FOUND, ProcessAddRequest(kP4TunnelNextHopAppDbEntry1)); @@ -526,11 +570,12 @@ TEST_F(NextHopManagerTest, ProcessAddRequestShouldSuccessForTunnelNexthop) ASSERT_TRUE(ResolveNextHopEntryDependency(kP4TunnelNextHopAppDbEntry1, kTunnelOid1)); // Set up mock call. - EXPECT_CALL(mock_sai_next_hop_, - create_next_hop( - ::testing::NotNull(), Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchCreateNextHopArgAttrList, std::placeholders::_1, - CreateAttributeListForNextHopObject(kP4TunnelNextHopAppDbEntry1, kTunnelOid1))))) + EXPECT_CALL( + mock_sai_next_hop_, + create_next_hop(::testing::NotNull(), Eq(gSwitchId), Eq(3), + Truly(std::bind(MatchCreateNextHopArgAttrList, std::placeholders::_1, + CreateAttributeListForNextHopObject(kP4TunnelNextHopAppDbEntry1, kTunnelOid1, + swss::IpAddress(kNeighborId1)))))) .WillOnce(DoAll(SetArgPointee<0>(kTunnelNextHopOid), Return(SAI_STATUS_SUCCESS))); EXPECT_EQ(StatusCode::SWSS_RC_SUCCESS, ProcessAddRequest(kP4TunnelNextHopAppDbEntry1)); @@ -545,7 +590,7 @@ TEST_F(NextHopManagerTest, ProcessAddRequestShouldSuccessForTunnelNexthop) EXPECT_TRUE(ValidateNextHopEntryAdd(kP4TunnelNextHopAppDbEntry1, kTunnelNextHopOid)); const std::string tunnel_key = KeyGenerator::generateTunnelKey(kP4TunnelNextHopAppDbEntry1.gre_tunnel_id); const std::string neighbor_key = - KeyGenerator::generateNeighborKey(kP4TunnelEntry1.router_interface_id, kP4TunnelNextHopAppDbEntry1.neighbor_id); + KeyGenerator::generateNeighborKey(kP4TunnelEntry1.router_interface_id, kP4TunnelEntry1.neighbor_id); EXPECT_TRUE(ValidateRefCnt(SAI_OBJECT_TYPE_TUNNEL, tunnel_key, 1)); EXPECT_TRUE(ValidateRefCnt(SAI_OBJECT_TYPE_NEIGHBOR_ENTRY, neighbor_key, 1)); } @@ -814,7 +859,7 @@ TEST_F(NextHopManagerTest, DrainValidTunnelNexthopAppEntryShouldSucceed) // Validate ref count decrement. const std::string tunnel_key = KeyGenerator::generateTunnelKey(kP4TunnelNextHopAppDbEntry2.gre_tunnel_id); const std::string neighbor_key = - KeyGenerator::generateNeighborKey(kP4TunnelEntry2.router_interface_id, kP4TunnelNextHopAppDbEntry2.neighbor_id); + KeyGenerator::generateNeighborKey(kP4TunnelEntry2.router_interface_id, kP4TunnelEntry2.neighbor_id); EXPECT_TRUE(ValidateRefCnt(SAI_OBJECT_TYPE_TUNNEL, tunnel_key, 0)); EXPECT_TRUE(ValidateRefCnt(SAI_OBJECT_TYPE_NEIGHBOR_ENTRY, neighbor_key, 0)); } @@ -896,7 +941,7 @@ TEST_F(NextHopManagerTest, DrainAppEntryWithInvalidFieldShouldBeNoOp) Drain(); EXPECT_FALSE(ValidateNextHopEntryAdd(kP4NextHopAppDbEntry2, kNextHopOid)); - // set_tunnel_encap_nexthop + invalid router_interface_id + // set_p2p_tunnel_encap_nexthop + invalid router_interface_id fvs = {{p4orch::kAction, p4orch::kSetTunnelNexthop}, {prependParamField(p4orch::kNeighborId), kNeighborId2}, {prependParamField(p4orch::kRouterInterfaceId), kRouterInterfaceId1}}; @@ -907,7 +952,7 @@ TEST_F(NextHopManagerTest, DrainAppEntryWithInvalidFieldShouldBeNoOp) Drain(); EXPECT_FALSE(ValidateNextHopEntryAdd(kP4TunnelNextHopAppDbEntry2, kNextHopOid)); - // set_tunnel_encap_nexthop + missing tunnel_id + // set_p2p_tunnel_encap_nexthop + missing tunnel_id fvs = {{p4orch::kAction, p4orch::kSetTunnelNexthop}, {prependParamField(p4orch::kNeighborId), kNeighborId2}}; app_db_entry = {std::string(APP_P4RT_NEXTHOP_TABLE_NAME) + kTableKeyDelimiter + j.dump(), SET_COMMAND, fvs}; @@ -976,7 +1021,7 @@ TEST_F(NextHopManagerTest, DrainDeleteRequestShouldSucceedForExistingNextHop) EXPECT_TRUE(ValidateRefCnt(SAI_OBJECT_TYPE_NEIGHBOR_ENTRY, neighbor_key, 0)); } -TEST_F(NextHopManagerTest, VerifyStateTest) +TEST_F(NextHopManagerTest, VerifyIpNextHopStateTest) { auto *p4_next_hop_entry = AddNextHopEntry1(); ASSERT_NE(p4_next_hop_entry, nullptr); @@ -1045,6 +1090,74 @@ TEST_F(NextHopManagerTest, VerifyStateTest) p4_next_hop_entry->gre_tunnel_id = saved_gre_tunnel_id; } +TEST_F(NextHopManagerTest, VerifyTunnelNextHopStateTest) +{ + ASSERT_TRUE(ResolveNextHopEntryDependency(kP4TunnelNextHopAppDbEntry1, kTunnelOid1)); + + // Set up mock call. + EXPECT_CALL( + mock_sai_next_hop_, + create_next_hop(::testing::NotNull(), Eq(gSwitchId), Eq(3), + Truly(std::bind(MatchCreateNextHopArgAttrList, std::placeholders::_1, + CreateAttributeListForNextHopObject(kP4TunnelNextHopAppDbEntry1, kTunnelOid1, + swss::IpAddress(kNeighborId1)))))) + .WillOnce(DoAll(SetArgPointee<0>(kTunnelNextHopOid), Return(SAI_STATUS_SUCCESS))); + + EXPECT_EQ(StatusCode::SWSS_RC_SUCCESS, ProcessAddRequest(kP4TunnelNextHopAppDbEntry1)); + + auto p4_next_hop_entry = GetNextHopEntry(KeyGenerator::generateNextHopKey(kP4TunnelNextHopAppDbEntry1.next_hop_id)); + ASSERT_NE(p4_next_hop_entry, nullptr); + + // Setup ASIC DB. + swss::Table table(nullptr, "ASIC_STATE"); + table.set("SAI_OBJECT_TYPE_NEXT_HOP:oid:0x66", + std::vector{ + swss::FieldValueTuple{"SAI_NEXT_HOP_ATTR_TYPE", "SAI_NEXT_HOP_TYPE_TUNNEL_ENCAP"}, + swss::FieldValueTuple{"SAI_NEXT_HOP_ATTR_IP", "10.0.0.1"}, + swss::FieldValueTuple{"SAI_NEXT_HOP_ATTR_TUNNEL_ID", "oid:0xb"}}); + + nlohmann::json j; + j[prependMatchField(p4orch::kNexthopId)] = kTunnelNextHopId; + const std::string db_key = std::string(APP_P4RT_TABLE_NAME) + kTableKeyDelimiter + APP_P4RT_NEXTHOP_TABLE_NAME + + kTableKeyDelimiter + j.dump(); + std::vector attributes; + + // Verification should succeed with vaild key and value. + attributes.push_back(swss::FieldValueTuple{prependParamField(p4orch::kNeighborId), kNeighborId1}); + attributes.push_back(swss::FieldValueTuple{prependParamField(p4orch::kRouterInterfaceId), kRouterInterfaceId1}); + EXPECT_EQ(VerifyState(db_key, attributes), ""); + + // Verification should fail if nexthop key mismatches. + auto saved_next_hop_key = p4_next_hop_entry->next_hop_key; + p4_next_hop_entry->next_hop_key = "invalid"; + EXPECT_FALSE(VerifyState(db_key, attributes).empty()); + p4_next_hop_entry->next_hop_key = saved_next_hop_key; + + // Verification should fail if nexthop ID mismatches. + auto saved_next_hop_id = p4_next_hop_entry->next_hop_id; + p4_next_hop_entry->next_hop_id = "invalid"; + EXPECT_FALSE(VerifyState(db_key, attributes).empty()); + p4_next_hop_entry->next_hop_id = saved_next_hop_id; + + // Verification should fail if ritf ID mismatches. + auto saved_router_interface_id = p4_next_hop_entry->router_interface_id; + p4_next_hop_entry->router_interface_id = kRouterInterfaceId2; + EXPECT_FALSE(VerifyState(db_key, attributes).empty()); + p4_next_hop_entry->router_interface_id = saved_router_interface_id; + + // Verification should fail if neighbor ID mismatches. + auto saved_neighbor_id = p4_next_hop_entry->neighbor_id; + p4_next_hop_entry->neighbor_id = swss::IpAddress(kNeighborId2); + EXPECT_FALSE(VerifyState(db_key, attributes).empty()); + p4_next_hop_entry->neighbor_id = saved_neighbor_id; + + // Verification should fail if tunnel ID mismatches. + auto saved_gre_tunnel_id = p4_next_hop_entry->gre_tunnel_id; + p4_next_hop_entry->gre_tunnel_id = "invalid"; + EXPECT_FALSE(VerifyState(db_key, attributes).empty()); + p4_next_hop_entry->gre_tunnel_id = saved_gre_tunnel_id; +} + TEST_F(NextHopManagerTest, VerifyStateAsicDbTest) { auto *p4_next_hop_entry = AddNextHopEntry1(); diff --git a/orchagent/p4orch/tests/wcmp_manager_test.cpp b/orchagent/p4orch/tests/wcmp_manager_test.cpp index 1c4981b66511..cf8d22b6841f 100644 --- a/orchagent/p4orch/tests/wcmp_manager_test.cpp +++ b/orchagent/p4orch/tests/wcmp_manager_test.cpp @@ -45,6 +45,7 @@ using ::testing::DoAll; using ::testing::Eq; using ::testing::Return; using ::testing::SetArgPointee; +using ::testing::SetArrayArgument; using ::testing::StrictMock; using ::testing::Truly; @@ -70,6 +71,60 @@ const std::string kNexthopKey1 = KeyGenerator::generateNextHopKey(kNexthopId1); const std::string kNexthopKey2 = KeyGenerator::generateNextHopKey(kNexthopId2); const std::string kNexthopKey3 = KeyGenerator::generateNextHopKey(kNexthopId3); +// Matches two SAI attributes. +bool MatchSaiAttribute(const sai_attribute_t &attr, const sai_attribute_t &exp_attr) +{ + if (exp_attr.id == SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID) + { + if (attr.id != SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID || attr.value.oid != exp_attr.value.oid) + { + return false; + } + } + if (exp_attr.id == SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID) + { + if (attr.id != SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID || attr.value.oid != exp_attr.value.oid) + { + return false; + } + } + if (exp_attr.id == SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT) + { + if (attr.id != SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT || attr.value.u32 != exp_attr.value.u32) + { + return false; + } + } + return true; +} + +MATCHER_P(ArrayEq, array, "") +{ + for (size_t i = 0; i < array.size(); ++i) + { + if (arg[i] != array[i]) + { + return false; + } + } + return true; +} + +MATCHER_P(AttrArrayArrayEq, array, "") +{ + for (size_t i = 0; i < array.size(); ++i) + { + for (size_t j = 0; j < array[i].size(); j++) + { + if (!MatchSaiAttribute(arg[i][j], array[i][j])) + { + return false; + } + } + } + return true; +} + // Matches the next hop group type sai_attribute_t argument. bool MatchSaiNextHopGroupAttribute(const sai_attribute_t *attr) { @@ -118,6 +173,26 @@ bool MatchSaiNextHopGroupMemberAttribute(const sai_object_id_t expected_next_hop return true; } +std::vector GetSaiNextHopGroupMemberAttribute(sai_object_id_t next_hop_oid, uint32_t weight, + sai_object_id_t group_oid) +{ + std::vector attrs; + sai_attribute_t attr; + attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID; + attr.value.oid = group_oid; + attrs.push_back(attr); + + attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID; + attr.value.oid = next_hop_oid; + attrs.push_back(attr); + + attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT; + attr.value.u32 = weight; + attrs.push_back(attr); + + return attrs; +} + void VerifyWcmpGroupMemberEntry(const std::string &expected_next_hop_id, const int expected_weight, std::shared_ptr wcmp_gm_entry) { @@ -172,6 +247,8 @@ class WcmpManagerTest : public ::testing::Test sai_next_hop_group_api->create_next_hop_group_member = create_next_hop_group_member; sai_next_hop_group_api->remove_next_hop_group_member = remove_next_hop_group_member; sai_next_hop_group_api->set_next_hop_group_member_attribute = set_next_hop_group_member_attribute; + sai_next_hop_group_api->create_next_hop_group_members = create_next_hop_group_members; + sai_next_hop_group_api->remove_next_hop_group_members = remove_next_hop_group_members; sai_hostif_api->create_hostif_table_entry = mock_create_hostif_table_entry; sai_hostif_api->create_hostif_trap = mock_create_hostif_trap; @@ -229,18 +306,6 @@ class WcmpManagerTest : public ::testing::Test wcmp_group_manager_->restorePrunedNextHops(port); } - bool VerifyWcmpGroupMemberInPrunedSet(std::shared_ptr gm, bool expected_member_present, - long unsigned int expected_set_size) - { - if (wcmp_group_manager_->pruned_wcmp_members_set.size() != expected_set_size) - return false; - - return expected_member_present ? (wcmp_group_manager_->pruned_wcmp_members_set.find(gm) != - wcmp_group_manager_->pruned_wcmp_members_set.end()) - : (wcmp_group_manager_->pruned_wcmp_members_set.find(gm) == - wcmp_group_manager_->pruned_wcmp_members_set.end()); - } - bool VerifyWcmpGroupMemberInPortMap(std::shared_ptr gm, bool expected_member_present, long unsigned int expected_set_size) { @@ -338,13 +403,18 @@ P4WcmpGroupEntry WcmpManagerTest::AddWcmpGroupEntryWithWatchport(const std::stri .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupOid1), Return(SAI_STATUS_SUCCESS))); // For members with non empty watchport field, member creation in SAI happens // for operationally up ports only.. + std::vector return_oids{kWcmpGroupMemberOid1}; + std::vector exp_status{SAI_STATUS_SUCCESS}; if (oper_up) { - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL( + mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 2, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_status.begin(), exp_status.end()), Return(SAI_STATUS_SUCCESS))); } EXPECT_EQ(StatusCode::SWSS_RC_SUCCESS, ProcessAddRequest(&app_db_entry)); EXPECT_NE(nullptr, GetWcmpGroupEntry(kWcmpGroupId1)); @@ -361,16 +431,16 @@ P4WcmpGroupEntry WcmpManagerTest::AddWcmpGroupEntry1() create_next_hop_group(_, Eq(gSwitchId), Eq(1), Truly(std::bind(MatchSaiNextHopGroupAttribute, std::placeholders::_1)))) .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid2), Return(SAI_STATUS_SUCCESS))); + std::vector return_oids{kWcmpGroupMemberOid1, kWcmpGroupMemberOid2}; + std::vector exp_status{SAI_STATUS_SUCCESS, SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(2), ArrayEq(std::vector{3, 3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 2, kWcmpGroupOid1), + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_status.begin(), exp_status.end()), Return(SAI_STATUS_SUCCESS))); EXPECT_EQ(StatusCode::SWSS_RC_SUCCESS, ProcessAddRequest(&app_db_entry)); EXPECT_NE(nullptr, GetWcmpGroupEntry(kWcmpGroupId1)); return app_db_entry; @@ -420,21 +490,26 @@ TEST_F(WcmpManagerTest, CreateWcmpGroupFailsWhenCreateGroupMemberSaiCallFails) // WCMP group creation fails when one of the group member creation fails EXPECT_CALL(mock_sai_next_hop_group_, create_next_hop_group(_, _, _, _)) .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(Return(SAI_STATUS_ITEM_NOT_FOUND)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); + std::vector return_oids{kWcmpGroupMemberOid1, SAI_NULL_OBJECT_ID}; + std::vector exp_create_status{SAI_STATUS_SUCCESS, SAI_STATUS_ITEM_NOT_FOUND}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(2), ArrayEq(std::vector{3, 3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 2, kWcmpGroupOid1), + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_ITEM_NOT_FOUND))); + std::vector exp_remove_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group(Eq(kWcmpGroupOid1))) .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_EQ(StatusCode::SWSS_RC_NOT_FOUND, ProcessAddRequest(&app_db_entry)); + EXPECT_EQ(StatusCode::SWSS_RC_UNKNOWN, ProcessAddRequest(&app_db_entry)); std::string key = KeyGenerator::generateWcmpGroupKey(kWcmpGroupId1); auto *wcmp_group_entry_ptr = GetWcmpGroupEntry(kWcmpGroupId1); EXPECT_EQ(nullptr, wcmp_group_entry_ptr); @@ -454,23 +529,28 @@ TEST_F(WcmpManagerTest, CreateWcmpGroupFailsWhenCreateGroupMemberSaiCallFailsPlu // WCMP group creation fails when one of the group member creation fails EXPECT_CALL(mock_sai_next_hop_group_, create_next_hop_group(_, _, _, _)) .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(Return(SAI_STATUS_ITEM_NOT_FOUND)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_FAILURE)); + std::vector return_oids{kWcmpGroupMemberOid1, SAI_NULL_OBJECT_ID}; + std::vector exp_create_status{SAI_STATUS_SUCCESS, SAI_STATUS_ITEM_NOT_FOUND}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(2), ArrayEq(std::vector{3, 3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 2, kWcmpGroupOid1), + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_ITEM_NOT_FOUND))); + std::vector exp_remove_status{SAI_STATUS_FAILURE}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_FAILURE))); EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group(Eq(kWcmpGroupOid1))) .WillOnce(Return(SAI_STATUS_SUCCESS)); // TODO: Expect critical state. - EXPECT_EQ(StatusCode::SWSS_RC_NOT_FOUND, ProcessAddRequest(&app_db_entry)); + EXPECT_EQ(StatusCode::SWSS_RC_UNKNOWN, ProcessAddRequest(&app_db_entry)); } TEST_F(WcmpManagerTest, CreateWcmpGroupFailsWhenCreateGroupMemberSaiCallFailsPlusGroupRecoveryFails) @@ -481,23 +561,28 @@ TEST_F(WcmpManagerTest, CreateWcmpGroupFailsWhenCreateGroupMemberSaiCallFailsPlu // WCMP group creation fails when one of the group member creation fails EXPECT_CALL(mock_sai_next_hop_group_, create_next_hop_group(_, _, _, _)) .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(Return(SAI_STATUS_ITEM_NOT_FOUND)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); + std::vector return_oids{kWcmpGroupMemberOid1, SAI_NULL_OBJECT_ID}; + std::vector exp_create_status{SAI_STATUS_SUCCESS, SAI_STATUS_ITEM_NOT_FOUND}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(2), ArrayEq(std::vector{3, 3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 2, kWcmpGroupOid1), + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_ITEM_NOT_FOUND))); + std::vector exp_remove_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group(Eq(kWcmpGroupOid1))) .WillOnce(Return(SAI_STATUS_FAILURE)); // TODO: Expect critical state. - EXPECT_EQ(StatusCode::SWSS_RC_NOT_FOUND, ProcessAddRequest(&app_db_entry)); + EXPECT_EQ(StatusCode::SWSS_RC_UNKNOWN, ProcessAddRequest(&app_db_entry)); } TEST_F(WcmpManagerTest, CreateWcmpGroupFailsWhenCreateGroupSaiCallFails) @@ -534,52 +619,72 @@ TEST_F(WcmpManagerTest, RemoveWcmpGroupFailsWhenNotExist) TEST_F(WcmpManagerTest, RemoveWcmpGroupFailsWhenSaiCallFails) { P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntry1(); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid2))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); + std::vector exp_remove_status{SAI_STATUS_SUCCESS, SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members( + Eq(2), ArrayEq(std::vector{kWcmpGroupMemberOid2, kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group(Eq(kWcmpGroupOid1))) .WillOnce(Return(SAI_STATUS_FAILURE)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid2), Return(SAI_STATUS_SUCCESS))); + std::vector return_oids{kWcmpGroupMemberOid1, kWcmpGroupMemberOid2}; + std::vector exp_create_status{SAI_STATUS_SUCCESS, SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(2), ArrayEq(std::vector{3, 3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 2, kWcmpGroupOid1), + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); EXPECT_EQ(StatusCode::SWSS_RC_UNKNOWN, RemoveWcmpGroup(kWcmpGroupId1)); } TEST_F(WcmpManagerTest, RemoveWcmpGroupFailsWhenMemberRemovalFails) { P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntry1(); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid2))) - .WillOnce(Return(SAI_STATUS_FAILURE)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); + std::vector exp_remove_status{SAI_STATUS_FAILURE, SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members( + Eq(2), ArrayEq(std::vector{kWcmpGroupMemberOid2, kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_FAILURE))); + std::vector return_oids{kWcmpGroupMemberOid1}; + std::vector exp_create_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 2, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); EXPECT_EQ(StatusCode::SWSS_RC_UNKNOWN, RemoveWcmpGroup(kWcmpGroupId1)); } TEST_F(WcmpManagerTest, RemoveWcmpGroupFailsWhenMemberRemovalFailsPlusRecoveryFails) { P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntry1(); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid2))) - .WillOnce(Return(SAI_STATUS_FAILURE)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_FAILURE))); + std::vector exp_remove_status{SAI_STATUS_FAILURE, SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members( + Eq(2), ArrayEq(std::vector{kWcmpGroupMemberOid2, kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_FAILURE))); + std::vector return_oids{SAI_NULL_OBJECT_ID}; + std::vector exp_create_status{SAI_STATUS_FAILURE}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 2, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_FAILURE))); // TODO: Expect critical state. EXPECT_EQ(StatusCode::SWSS_RC_UNKNOWN, RemoveWcmpGroup(kWcmpGroupId1)); } @@ -594,20 +699,36 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupMembersSucceed) std::shared_ptr gm2 = createWcmpGroupMemberEntry(kNexthopId2, 15); wcmp_group.wcmp_group_members.push_back(gm1); wcmp_group.wcmp_group_members.push_back(gm2); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid2))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 3, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid4), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 15, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid5), Return(SAI_STATUS_SUCCESS))); + std::vector exp_remove_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid2}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + std::vector return_oids_4{kWcmpGroupMemberOid4}; + std::vector return_oids_5{kWcmpGroupMemberOid5}; + std::vector exp_create_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 3, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_4.begin(), return_oids_4.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 15, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_5.begin(), return_oids_5.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); EXPECT_TRUE(ProcessUpdateRequest(&wcmp_group).ok()); VerifyWcmpGroupEntry(wcmp_group, *GetWcmpGroupEntry(kWcmpGroupId1)); uint32_t wcmp_group_refcount = 0; @@ -622,15 +743,27 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupMembersSucceed) wcmp_group.wcmp_group_members.clear(); gm2 = createWcmpGroupMemberEntry(kNexthopId2, 15); wcmp_group.wcmp_group_members.push_back(gm2); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid4))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid5))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 15, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid2), Return(SAI_STATUS_SUCCESS))); + exp_remove_status = {SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid4}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid5}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + std::vector return_oids_2{kWcmpGroupMemberOid2}; + exp_create_status = {SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 15, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_2.begin(), return_oids_2.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); EXPECT_TRUE(ProcessUpdateRequest(&wcmp_group).ok()); VerifyWcmpGroupEntry(wcmp_group, *GetWcmpGroupEntry(kWcmpGroupId1)); ASSERT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP_GROUP, kWcmpGroupKey1, &wcmp_group_refcount)); @@ -645,18 +778,30 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupMembersSucceed) std::shared_ptr updated_gm1 = createWcmpGroupMemberEntry(kNexthopId1, 20); wcmp_group.wcmp_group_members.push_back(updated_gm1); wcmp_group.wcmp_group_members.push_back(updated_gm2); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 20, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid2))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 15, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid5), Return(SAI_STATUS_SUCCESS))); + exp_remove_status = {SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid2}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + std::vector return_oids_1{kWcmpGroupMemberOid1}; + exp_create_status = {SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 20, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_1.begin(), return_oids_1.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 15, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_5.begin(), return_oids_5.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); EXPECT_TRUE(ProcessUpdateRequest(&wcmp_group).ok()); VerifyWcmpGroupEntry(wcmp_group, *GetWcmpGroupEntry(kWcmpGroupId1)); ASSERT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP_GROUP, kWcmpGroupKey1, &wcmp_group_refcount)); @@ -668,10 +813,17 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupMembersSucceed) // Update WCMP without group members wcmp_group.wcmp_group_members.clear(); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid5))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); + exp_remove_status = {SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid5}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); EXPECT_TRUE(ProcessUpdateRequest(&wcmp_group).ok()); VerifyWcmpGroupEntry(wcmp_group, *GetWcmpGroupEntry(kWcmpGroupId1)); ASSERT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP_GROUP, kWcmpGroupKey1, &wcmp_group_refcount)); @@ -696,25 +848,38 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupFailsWhenRemoveGroupMemberSaiCallFails) wcmp_group.wcmp_group_members.push_back(gm1); wcmp_group.wcmp_group_members.push_back(gm2); wcmp_group.wcmp_group_members.push_back(gm3); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 3, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid4), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 10, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid5), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid3, 30, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid3), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid2))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); + std::vector return_oids_4{kWcmpGroupMemberOid4}; + std::vector return_oids_5_6{kWcmpGroupMemberOid5, kWcmpGroupMemberOid3}; + std::vector exp_create_status_1{SAI_STATUS_SUCCESS}; + std::vector exp_create_status_2{SAI_STATUS_SUCCESS, SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 3, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_4.begin(), return_oids_4.end()), + SetArrayArgument<6>(exp_create_status_1.begin(), exp_create_status_1.end()), + Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(2), ArrayEq(std::vector{3, 3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 10, kWcmpGroupOid1), + GetSaiNextHopGroupMemberAttribute(kNexthopOid3, 30, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_5_6.begin(), return_oids_5_6.end()), + SetArrayArgument<6>(exp_create_status_2.begin(), exp_create_status_2.end()), + Return(SAI_STATUS_SUCCESS))); + std::vector exp_remove_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid2}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); EXPECT_TRUE(ProcessUpdateRequest(&wcmp_group).ok()); VerifyWcmpGroupEntry(wcmp_group, *GetWcmpGroupEntry(kWcmpGroupId1)); uint32_t wcmp_group_refcount = 0; @@ -732,16 +897,23 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupFailsWhenRemoveGroupMemberSaiCallFails) wcmp_group.wcmp_group_members.clear(); wcmp_group.wcmp_group_members.push_back(gm1); wcmp_group.wcmp_group_members.push_back(gm3); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid5))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid3))) - .WillOnce(Return(SAI_STATUS_OBJECT_IN_USE)); - // Clean up - revert deletions -success + exp_remove_status = {SAI_STATUS_OBJECT_IN_USE, SAI_STATUS_SUCCESS}; EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 10, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid5), Return(SAI_STATUS_SUCCESS))); + remove_next_hop_group_members( + Eq(2), ArrayEq(std::vector{kWcmpGroupMemberOid3, kWcmpGroupMemberOid5}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce(DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), + Return(SAI_STATUS_OBJECT_IN_USE))); + // Clean up - revert deletions -success + std::vector return_oids_5{kWcmpGroupMemberOid5}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 10, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_5.begin(), return_oids_5.end()), + SetArrayArgument<6>(exp_create_status_1.begin(), exp_create_status_1.end()), + Return(SAI_STATUS_SUCCESS))); EXPECT_EQ(StatusCode::SWSS_RC_IN_USE, ProcessUpdateRequest(&wcmp_group)); P4WcmpGroupEntry expected_wcmp_group = {.wcmp_group_id = kWcmpGroupId1, .wcmp_group_members = {}}; expected_wcmp_group.wcmp_group_members.push_back(gm1); @@ -760,19 +932,26 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupFailsWhenRemoveGroupMemberSaiCallFails) // Remove WCMP group member with nexthop_id=kNexthopId1 and // nexthop_id=kNexthopId3(fail) - fail to clean up - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid5))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid3))) - .WillOnce(Return(SAI_STATUS_OBJECT_IN_USE)); - // Clean up - revert deletions -failure + exp_remove_status = {SAI_STATUS_OBJECT_IN_USE, SAI_STATUS_SUCCESS}; EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 10, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(Return(SAI_STATUS_TABLE_FULL)); + remove_next_hop_group_members( + Eq(2), ArrayEq(std::vector{kWcmpGroupMemberOid3, kWcmpGroupMemberOid5}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce(DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), + Return(SAI_STATUS_OBJECT_IN_USE))); + // Clean up - revert deletions -failure + std::vector return_oids{SAI_NULL_OBJECT_ID}; + std::vector exp_create_status{SAI_STATUS_TABLE_FULL}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 10, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_TABLE_FULL))); // TODO: Expect critical state. - EXPECT_EQ("Failed to remove WCMP group member with nexthop id " - "'ju1u32m3.atl11:qe-3/7'", + EXPECT_EQ("Failed to delete WCMP group member: 'ju1u32m3.atl11:qe-3/7'", ProcessUpdateRequest(&wcmp_group).message()); // WCMP group is as expected, but refcounts are not VerifyWcmpGroupEntry(expected_wcmp_group, *GetWcmpGroupEntry(kWcmpGroupId1)); @@ -796,15 +975,27 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupFailsWhenCreateNewGroupMemberSaiCallFails wcmp_group.wcmp_group_members.clear(); std::shared_ptr gm = createWcmpGroupMemberEntry(kNexthopId2, 15); wcmp_group.wcmp_group_members.push_back(gm); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid2))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 15, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid5), Return(SAI_STATUS_SUCCESS))); + std::vector exp_remove_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid2}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + std::vector return_oids_5{kWcmpGroupMemberOid5}; + std::vector exp_create_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 15, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_5.begin(), return_oids_5.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); EXPECT_TRUE(ProcessUpdateRequest(&wcmp_group).ok()); VerifyWcmpGroupEntry(wcmp_group, *GetWcmpGroupEntry(kWcmpGroupId1)); uint32_t wcmp_group_refcount = 0; @@ -827,33 +1018,47 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupFailsWhenCreateNewGroupMemberSaiCallFails updated_wcmp_group.wcmp_group_members.push_back(updated_gm1); updated_wcmp_group.wcmp_group_members.push_back(updated_gm2); updated_wcmp_group.wcmp_group_members.push_back(updated_gm3); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid5))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 3, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 20, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid2), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid3, 30, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(Return(SAI_STATUS_TABLE_FULL)); + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid5}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + std::vector return_oids_1{kWcmpGroupMemberOid1}; + std::vector exp_create_status_fail{SAI_STATUS_SUCCESS, SAI_STATUS_TABLE_FULL}; + std::vector return_oids_2_null{kWcmpGroupMemberOid2, SAI_NULL_OBJECT_ID}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 3, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_1.begin(), return_oids_1.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(2), ArrayEq(std::vector{3, 3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 20, kWcmpGroupOid1), + GetSaiNextHopGroupMemberAttribute(kNexthopOid3, 30, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_2_null.begin(), return_oids_2_null.end()), + SetArrayArgument<6>(exp_create_status_fail.begin(), exp_create_status_fail.end()), + Return(SAI_STATUS_TABLE_FULL))); // Clean up - success - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid2))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 15, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid5), Return(SAI_STATUS_SUCCESS))); + std::vector exp_remove_status_2{SAI_STATUS_SUCCESS, SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members( + Eq(2), ArrayEq(std::vector{kWcmpGroupMemberOid2, kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce(DoAll(SetArrayArgument<3>(exp_remove_status_2.begin(), exp_remove_status_2.end()), + Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 15, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_5.begin(), return_oids_5.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); EXPECT_FALSE(ProcessUpdateRequest(&updated_wcmp_group).ok()); P4WcmpGroupEntry expected_wcmp_group = {.wcmp_group_id = kWcmpGroupId1, .wcmp_group_members = {}}; std::shared_ptr expected_gm = createWcmpGroupMemberEntry(kNexthopId2, 15); @@ -869,44 +1074,48 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupFailsWhenCreateNewGroupMemberSaiCallFails EXPECT_EQ(0, nexthop_refcount); // Try again, but this time clean up failed to remove created group member - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid5))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 3, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 20, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid2), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid3, 30, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(Return(SAI_STATUS_TABLE_FULL)); + exp_remove_status = {SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid5}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 3, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_1.begin(), return_oids_1.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(2), ArrayEq(std::vector{3, 3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 20, kWcmpGroupOid1), + GetSaiNextHopGroupMemberAttribute(kNexthopOid3, 30, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_2_null.begin(), return_oids_2_null.end()), + SetArrayArgument<6>(exp_create_status_fail.begin(), exp_create_status_fail.end()), + Return(SAI_STATUS_TABLE_FULL))); // Clean up - revert creation - failure - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); + std::vector exp_remove_status_fail{SAI_STATUS_OBJECT_IN_USE, SAI_STATUS_SUCCESS}; EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 15, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid5), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid2))) - .WillOnce(Return(SAI_STATUS_OBJECT_IN_USE)); + remove_next_hop_group_members( + Eq(2), ArrayEq(std::vector{kWcmpGroupMemberOid2, kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce(DoAll(SetArrayArgument<3>(exp_remove_status_fail.begin(), exp_remove_status_fail.end()), + Return(SAI_STATUS_OBJECT_IN_USE))); // TODO: Expect critical state. - EXPECT_EQ("Failed to create next hop group member 'ju1u32m3.atl11:qe-3/7'", + EXPECT_EQ("Fail to create wcmp group member: 'ju1u32m3.atl11:qe-3/7'", ProcessUpdateRequest(&updated_wcmp_group).message()); // WCMP group is as expected, but refcounts are not VerifyWcmpGroupEntry(expected_wcmp_group, *GetWcmpGroupEntry(kWcmpGroupId1)); ASSERT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP_GROUP, kWcmpGroupKey1, &wcmp_group_refcount)); - EXPECT_EQ(2, wcmp_group_refcount); // Corrupt status due to clean up failure + EXPECT_EQ(1, wcmp_group_refcount); ASSERT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey1, &nexthop_refcount)); EXPECT_EQ(0, nexthop_refcount); // Corrupt status due to clean up failure ASSERT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey2, &nexthop_refcount)); - EXPECT_EQ(2, nexthop_refcount); + EXPECT_EQ(1, nexthop_refcount); ASSERT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey3, &nexthop_refcount)); EXPECT_EQ(0, nexthop_refcount); } @@ -922,18 +1131,32 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupFailsWhenReduceGroupMemberWeightSaiCallFa std::shared_ptr gm2 = createWcmpGroupMemberEntry(kNexthopId2, 10); wcmp_group.wcmp_group_members.push_back(gm1); wcmp_group.wcmp_group_members.push_back(gm2); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(Return(SAI_STATUS_NOT_SUPPORTED)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); + std::vector exp_remove_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + std::vector return_oids_1{kWcmpGroupMemberOid1}; + std::vector return_oids_null{SAI_NULL_OBJECT_ID}; + std::vector exp_create_status{SAI_STATUS_SUCCESS}; + std::vector exp_create_status_fail{SAI_STATUS_NOT_SUPPORTED}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_null.begin(), return_oids_null.end()), + SetArrayArgument<6>(exp_create_status_fail.begin(), exp_create_status_fail.end()), + Return(SAI_STATUS_NOT_SUPPORTED))); + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 2, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_1.begin(), return_oids_1.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_NOT_SUPPORTED))); EXPECT_FALSE(ProcessUpdateRequest(&wcmp_group).ok()); P4WcmpGroupEntry expected_wcmp_group = {.wcmp_group_id = kWcmpGroupId1, .wcmp_group_members = {}}; std::shared_ptr expected_gm1 = createWcmpGroupMemberEntry(kNexthopId1, 2); @@ -962,33 +1185,54 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupFailsWhenIncreaseGroupMemberWeightSaiCall std::shared_ptr gm2 = createWcmpGroupMemberEntry(kNexthopId2, 10); wcmp_group.wcmp_group_members.push_back(gm1); wcmp_group.wcmp_group_members.push_back(gm2); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid2))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid4), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 10, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(Return(SAI_STATUS_NOT_SUPPORTED)); + std::vector exp_remove_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid2}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + std::vector return_oids_4{kWcmpGroupMemberOid4}; + std::vector return_oids_null{SAI_NULL_OBJECT_ID}; + std::vector exp_create_status{SAI_STATUS_SUCCESS}; + std::vector exp_create_status_fail{SAI_STATUS_NOT_SUPPORTED}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_4.begin(), return_oids_4.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 10, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_null.begin(), return_oids_null.end()), + SetArrayArgument<6>(exp_create_status_fail.begin(), exp_create_status_fail.end()), + Return(SAI_STATUS_NOT_SUPPORTED))); // Clean up modified members - success - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid4))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid2), Return(SAI_STATUS_SUCCESS))); + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid4}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + std::vector return_oids_1_2{kWcmpGroupMemberOid1, kWcmpGroupMemberOid2}; + std::vector exp_create_status_2{SAI_STATUS_SUCCESS, SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(2), ArrayEq(std::vector{3, 3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 2, kWcmpGroupOid1), + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_1_2.begin(), return_oids_1_2.end()), + SetArrayArgument<6>(exp_create_status_2.begin(), exp_create_status_2.end()), + Return(SAI_STATUS_SUCCESS))); EXPECT_FALSE(ProcessUpdateRequest(&wcmp_group).ok()); P4WcmpGroupEntry expected_wcmp_group = {.wcmp_group_id = kWcmpGroupId1, .wcmp_group_members = {}}; std::shared_ptr expected_gm1 = createWcmpGroupMemberEntry(kNexthopId1, 2); @@ -1005,38 +1249,52 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupFailsWhenIncreaseGroupMemberWeightSaiCall ASSERT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey2, &nexthop_refcount)); EXPECT_EQ(1, nexthop_refcount); // Try again, the same error happens when update and new error during clean up - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid2))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid4), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 10, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(Return(SAI_STATUS_NOT_SUPPORTED)); + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid2}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_4.begin(), return_oids_4.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 10, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_null.begin(), return_oids_null.end()), + SetArrayArgument<6>(exp_create_status_fail.begin(), exp_create_status_fail.end()), + Return(SAI_STATUS_NOT_SUPPORTED))); // Clean up modified members - failure - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid4))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid2), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_NOT_SUPPORTED))); + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid4}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + std::vector return_oids_2_null{SAI_NULL_OBJECT_ID, kWcmpGroupMemberOid2}; + std::vector exp_create_status_2_fail{SAI_STATUS_NOT_SUPPORTED, SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(2), ArrayEq(std::vector{3, 3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 2, kWcmpGroupOid1), + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_2_null.begin(), return_oids_2_null.end()), + SetArrayArgument<6>(exp_create_status_2_fail.begin(), exp_create_status_2_fail.end()), + Return(SAI_STATUS_NOT_SUPPORTED))); // TODO: Expect critical state. - EXPECT_EQ("Failed to create next hop group member " - "'ju1u32m2.atl11:qe-3/7'", - ProcessUpdateRequest(&wcmp_group).message()); + EXPECT_EQ("Fail to create wcmp group member: 'ju1u32m2.atl11:qe-3/7'", ProcessUpdateRequest(&wcmp_group).message()); // weight of wcmp_group_members[kNexthopId1] unable to revert // SAI object in ASIC DB: missing group member with // next_hop_id=kNexthopId1 @@ -1052,19 +1310,58 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupFailsWhenIncreaseGroupMemberWeightSaiCall TEST_F(WcmpManagerTest, ValidateWcmpGroupEntryFailsWhenNextHopDoesNotExist) { - P4WcmpGroupEntry app_db_entry = {.wcmp_group_id = kWcmpGroupId1, .wcmp_group_members = {}}; - std::shared_ptr gm = createWcmpGroupMemberEntry("Unregistered-Nexthop", 1); - app_db_entry.wcmp_group_members.push_back(gm); - EXPECT_EQ(StatusCode::SWSS_RC_NOT_FOUND, ProcessAddRequest(&app_db_entry)); + const std::string kKeyPrefix = std::string(APP_P4RT_WCMP_GROUP_TABLE_NAME) + kTableKeyDelimiter; + p4_oid_mapper_->setOID(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey1, kNexthopOid1); + nlohmann::json j; + j[prependMatchField(p4orch::kWcmpGroupId)] = kWcmpGroupId1; + std::vector attributes; + nlohmann::json actions; + nlohmann::json action; + action[p4orch::kAction] = p4orch::kSetNexthopId; + action[prependParamField(p4orch::kNexthopId)] = kNexthopId1; + actions.push_back(action); + action[prependParamField(p4orch::kNexthopId)] = kNexthopId2; + actions.push_back(action); + attributes.push_back(swss::FieldValueTuple{p4orch::kActions, actions.dump()}); + Enqueue(swss::KeyOpFieldsValuesTuple(kKeyPrefix + j.dump(), SET_COMMAND, attributes)); + Drain(); + std::string key = KeyGenerator::generateWcmpGroupKey(kWcmpGroupId1); + auto *wcmp_group_entry_ptr = GetWcmpGroupEntry(kWcmpGroupId1); + EXPECT_EQ(nullptr, wcmp_group_entry_ptr); + EXPECT_FALSE(p4_oid_mapper_->existsOID(SAI_OBJECT_TYPE_NEXT_HOP_GROUP, key)); + uint32_t ref_cnt; + EXPECT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey1, &ref_cnt)); + EXPECT_EQ(0, ref_cnt); } TEST_F(WcmpManagerTest, ValidateWcmpGroupEntryFailsWhenWeightLessThanOne) { - P4WcmpGroupEntry app_db_entry = {.wcmp_group_id = kWcmpGroupId1, .wcmp_group_members = {}}; - std::shared_ptr gm = createWcmpGroupMemberEntry(kNexthopId1, 0); - app_db_entry.wcmp_group_members.push_back(gm); + const std::string kKeyPrefix = std::string(APP_P4RT_WCMP_GROUP_TABLE_NAME) + kTableKeyDelimiter; p4_oid_mapper_->setOID(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey1, kNexthopOid1); - EXPECT_EQ(StatusCode::SWSS_RC_INVALID_PARAM, ProcessAddRequest(&app_db_entry)); + p4_oid_mapper_->setOID(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey2, kNexthopOid2); + nlohmann::json j; + j[prependMatchField(p4orch::kWcmpGroupId)] = kWcmpGroupId1; + std::vector attributes; + nlohmann::json actions; + nlohmann::json action; + action[p4orch::kAction] = p4orch::kSetNexthopId; + action[prependParamField(p4orch::kNexthopId)] = kNexthopId1; + actions.push_back(action); + action[p4orch::kWeight] = -1; + action[prependParamField(p4orch::kNexthopId)] = kNexthopId2; + actions.push_back(action); + attributes.push_back(swss::FieldValueTuple{p4orch::kActions, actions.dump()}); + Enqueue(swss::KeyOpFieldsValuesTuple(kKeyPrefix + j.dump(), SET_COMMAND, attributes)); + Drain(); + std::string key = KeyGenerator::generateWcmpGroupKey(kWcmpGroupId1); + auto *wcmp_group_entry_ptr = GetWcmpGroupEntry(kWcmpGroupId1); + EXPECT_EQ(nullptr, wcmp_group_entry_ptr); + EXPECT_FALSE(p4_oid_mapper_->existsOID(SAI_OBJECT_TYPE_NEXT_HOP_GROUP, key)); + uint32_t ref_cnt; + EXPECT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey1, &ref_cnt)); + EXPECT_EQ(0, ref_cnt); + EXPECT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey2, &ref_cnt)); + EXPECT_EQ(0, ref_cnt); } TEST_F(WcmpManagerTest, WcmpGroupInvalidOperationInDrainFails) @@ -1137,16 +1434,17 @@ TEST_F(WcmpManagerTest, WcmpGroupCreateAndDeleteInDrainSucceeds) EXPECT_CALL(mock_sai_next_hop_group_, create_next_hop_group(_, _, _, _)) .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid2), Return(SAI_STATUS_SUCCESS))); + std::vector return_oids{kWcmpGroupMemberOid1, kWcmpGroupMemberOid2}; + std::vector exp_create_status{SAI_STATUS_SUCCESS, SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(2), ArrayEq(std::vector{3, 3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 1, kWcmpGroupOid1), + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); Drain(); std::string key = KeyGenerator::generateWcmpGroupKey(kWcmpGroupId1); auto *wcmp_group_entry_ptr = GetWcmpGroupEntry(kWcmpGroupId1); @@ -1158,10 +1456,13 @@ TEST_F(WcmpManagerTest, WcmpGroupCreateAndDeleteInDrainSucceeds) EXPECT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey2, &ref_cnt)); EXPECT_EQ(1, ref_cnt); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid2))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); + std::vector exp_remove_status{SAI_STATUS_SUCCESS, SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members( + Eq(2), ArrayEq(std::vector{kWcmpGroupMemberOid2, kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group(Eq(kWcmpGroupOid1))) .WillOnce(Return(SAI_STATUS_SUCCESS)); attributes.clear(); @@ -1195,11 +1496,16 @@ TEST_F(WcmpManagerTest, WcmpGroupCreateAndUpdateInDrainSucceeds) Enqueue(swss::KeyOpFieldsValuesTuple(kKeyPrefix + j.dump(), SET_COMMAND, attributes)); EXPECT_CALL(mock_sai_next_hop_group_, create_next_hop_group(_, _, _, _)) .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); + std::vector return_oids{kWcmpGroupMemberOid1}; + std::vector exp_create_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); Drain(); std::string key = KeyGenerator::generateWcmpGroupKey(kWcmpGroupId1); auto *wcmp_group_entry_ptr = GetWcmpGroupEntry(kWcmpGroupId1); @@ -1215,13 +1521,22 @@ TEST_F(WcmpManagerTest, WcmpGroupCreateAndUpdateInDrainSucceeds) // Update WCMP group with exact same members, the same entry will be removed // and created again Enqueue(swss::KeyOpFieldsValuesTuple(kKeyPrefix + j.dump(), SET_COMMAND, attributes)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid3), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); + return_oids = {kWcmpGroupMemberOid3}; + exp_create_status = {SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); + std::vector exp_remove_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); Drain(); wcmp_group_entry_ptr = GetWcmpGroupEntry(kWcmpGroupId1); EXPECT_NE(nullptr, wcmp_group_entry_ptr); @@ -1239,13 +1554,22 @@ TEST_F(WcmpManagerTest, WcmpGroupCreateAndUpdateInDrainSucceeds) attributes.clear(); attributes.push_back(swss::FieldValueTuple{p4orch::kActions, actions.dump()}); Enqueue(swss::KeyOpFieldsValuesTuple(kKeyPrefix + j.dump(), SET_COMMAND, attributes)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid2), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid3))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); + return_oids = {kWcmpGroupMemberOid2}; + exp_create_status = {SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); + exp_remove_status = {SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid3}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); Drain(); wcmp_group_entry_ptr = GetWcmpGroupEntry(kWcmpGroupId1); EXPECT_NE(nullptr, wcmp_group_entry_ptr); @@ -1264,13 +1588,22 @@ TEST_F(WcmpManagerTest, WcmpGroupCreateAndUpdateInDrainSucceeds) attributes.clear(); attributes.push_back(swss::FieldValueTuple{p4orch::kActions, actions.dump()}); Enqueue(swss::KeyOpFieldsValuesTuple(kKeyPrefix + j.dump(), SET_COMMAND, attributes)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 2, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid4), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid2))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); + return_oids = {kWcmpGroupMemberOid4}; + exp_create_status = {SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 2, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); + exp_remove_status = {SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid2}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); Drain(); wcmp_group_entry_ptr = GetWcmpGroupEntry(kWcmpGroupId1); EXPECT_NE(nullptr, wcmp_group_entry_ptr); @@ -1402,13 +1735,27 @@ TEST_F(WcmpManagerTest, DeserializeWcmpGroupFailsWithUndefinedAttributes) TEST_F(WcmpManagerTest, ValidateWcmpGroupEntryWithInvalidWatchportAttributeFails) { - P4WcmpGroupEntry app_db_entry = {.wcmp_group_id = kWcmpGroupId1, .wcmp_group_members = {}}; - std::shared_ptr gm = - createWcmpGroupMemberEntryWithWatchport(kNexthopId1, 1, "EthernetXX", kWcmpGroupId1, kNexthopOid1); - app_db_entry.wcmp_group_members.push_back(gm); - EXPECT_EQ(StatusCode::SWSS_RC_INVALID_PARAM, ProcessAddRequest(&app_db_entry)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(gm, false, 0)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(gm, false, 0)); + const std::string kKeyPrefix = std::string(APP_P4RT_WCMP_GROUP_TABLE_NAME) + kTableKeyDelimiter; + p4_oid_mapper_->setOID(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey1, kNexthopOid1); + nlohmann::json j; + j[prependMatchField(p4orch::kWcmpGroupId)] = kWcmpGroupId1; + std::vector attributes; + nlohmann::json actions; + nlohmann::json action; + action[p4orch::kAction] = p4orch::kSetNexthopId; + action[p4orch::kWatchPort] = "EthernetXX"; + action[prependParamField(p4orch::kNexthopId)] = kNexthopId1; + actions.push_back(action); + attributes.push_back(swss::FieldValueTuple{p4orch::kActions, actions.dump()}); + Enqueue(swss::KeyOpFieldsValuesTuple(kKeyPrefix + j.dump(), SET_COMMAND, attributes)); + Drain(); + std::string key = KeyGenerator::generateWcmpGroupKey(kWcmpGroupId1); + auto *wcmp_group_entry_ptr = GetWcmpGroupEntry(kWcmpGroupId1); + EXPECT_EQ(nullptr, wcmp_group_entry_ptr); + EXPECT_FALSE(p4_oid_mapper_->existsOID(SAI_OBJECT_TYPE_NEXT_HOP_GROUP, key)); + uint32_t ref_cnt; + EXPECT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey1, &ref_cnt)); + EXPECT_EQ(0, ref_cnt); } TEST_F(WcmpManagerTest, PruneNextHopSucceeds) @@ -1417,13 +1764,13 @@ TEST_F(WcmpManagerTest, PruneNextHopSucceeds) std::string port_name = "Ethernet6"; P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name, true); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); + EXPECT_FALSE(app_db_entry.wcmp_group_members[0]->pruned); EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) .WillOnce(Return(SAI_STATUS_SUCCESS)); // Prune next hops associated with port PruneNextHops(port_name); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); } TEST_F(WcmpManagerTest, PruneNextHopFailsWithNextHopRemovalFailure) @@ -1432,13 +1779,14 @@ TEST_F(WcmpManagerTest, PruneNextHopFailsWithNextHopRemovalFailure) std::string port_name = "Ethernet6"; P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name, true); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); + EXPECT_FALSE(app_db_entry.wcmp_group_members[0]->pruned); EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) .WillOnce(Return(SAI_STATUS_FAILURE)); + // TODO: Expect critical state. // Prune next hops associated with port (fails) PruneNextHops(port_name); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); + EXPECT_FALSE(app_db_entry.wcmp_group_members[0]->pruned); } TEST_F(WcmpManagerTest, RestorePrunedNextHopSucceeds) @@ -1449,7 +1797,7 @@ TEST_F(WcmpManagerTest, RestorePrunedNextHopSucceeds) std::string port_name = "Ethernet1"; P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); EXPECT_CALL(mock_sai_next_hop_group_, create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, @@ -1459,7 +1807,7 @@ TEST_F(WcmpManagerTest, RestorePrunedNextHopSucceeds) // Restore next hops associated with port RestorePrunedNextHops(port_name); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); + EXPECT_FALSE(app_db_entry.wcmp_group_members[0]->pruned); } TEST_F(WcmpManagerTest, RestorePrunedNextHopFailsWithNoOidMappingForWcmpGroup) @@ -1470,12 +1818,12 @@ TEST_F(WcmpManagerTest, RestorePrunedNextHopFailsWithNoOidMappingForWcmpGroup) std::string port_name = "Ethernet1"; P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); p4_oid_mapper_->eraseOID(SAI_OBJECT_TYPE_NEXT_HOP_GROUP, KeyGenerator::generateWcmpGroupKey(kWcmpGroupId1)); // TODO: Expect critical state. RestorePrunedNextHops(port_name); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); } TEST_F(WcmpManagerTest, RestorePrunedNextHopFailsWithNextHopCreationFailure) @@ -1486,7 +1834,7 @@ TEST_F(WcmpManagerTest, RestorePrunedNextHopFailsWithNextHopCreationFailure) std::string port_name = "Ethernet1"; P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); EXPECT_CALL(mock_sai_next_hop_group_, create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, @@ -1495,7 +1843,7 @@ TEST_F(WcmpManagerTest, RestorePrunedNextHopFailsWithNextHopCreationFailure) // TODO: Expect critical state. RestorePrunedNextHops(port_name); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); } TEST_F(WcmpManagerTest, CreateGroupWithWatchportFailsWithNextHopCreationFailure) @@ -1514,26 +1862,31 @@ TEST_F(WcmpManagerTest, CreateGroupWithWatchportFailsWithNextHopCreationFailure) create_next_hop_group(_, Eq(gSwitchId), Eq(1), Truly(std::bind(MatchSaiNextHopGroupAttribute, std::placeholders::_1)))) .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(Return(SAI_STATUS_FAILURE)); + std::vector return_oids{kWcmpGroupMemberOid1, SAI_NULL_OBJECT_ID}; + std::vector exp_create_status{SAI_STATUS_SUCCESS, SAI_STATUS_FAILURE}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(2), ArrayEq(std::vector{3, 3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 1, kWcmpGroupOid1), + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_FAILURE))); // Clean up created members - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); + std::vector exp_remove_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group(Eq(kWcmpGroupOid1))) .WillOnce(Return(SAI_STATUS_SUCCESS)); EXPECT_EQ(StatusCode::SWSS_RC_UNKNOWN, ProcessAddRequest(&app_db_entry)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(gm1, false, 0)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(gm2, false, 0)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(gm1, false, 0)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(gm2, false, 0)); + EXPECT_FALSE(gm1->pruned); + EXPECT_FALSE(gm2->pruned); } TEST_F(WcmpManagerTest, RemoveWcmpGroupAfterPruningSucceeds) @@ -1542,12 +1895,12 @@ TEST_F(WcmpManagerTest, RemoveWcmpGroupAfterPruningSucceeds) std::string port_name = "Ethernet6"; P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name, true); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); + EXPECT_FALSE(app_db_entry.wcmp_group_members[0]->pruned); EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) .WillOnce(Return(SAI_STATUS_SUCCESS)); PruneNextHops(port_name); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); // Remove Wcmp group. No SAI call for member removal is expected as it is // already pruned. @@ -1555,7 +1908,6 @@ TEST_F(WcmpManagerTest, RemoveWcmpGroupAfterPruningSucceeds) .WillOnce(Return(SAI_STATUS_SUCCESS)); EXPECT_EQ(StatusCode::SWSS_RC_SUCCESS, RemoveWcmpGroup(kWcmpGroupId1)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], false, 0)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); } TEST_F(WcmpManagerTest, RemoveWcmpGroupWithOperationallyDownWatchportSucceeds) @@ -1565,7 +1917,29 @@ TEST_F(WcmpManagerTest, RemoveWcmpGroupWithOperationallyDownWatchportSucceeds) // directly added to the pruned set of WCMP group members. P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport("Ethernet1"); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); + + // Remove Wcmp group. No SAI call for member removal is expected as it is + // already pruned. + EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group(Eq(kWcmpGroupOid1))) + .WillOnce(Return(SAI_STATUS_SUCCESS)); + EXPECT_EQ(StatusCode::SWSS_RC_SUCCESS, RemoveWcmpGroup(kWcmpGroupId1)); + EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], false, 0)); +} + +TEST_F(WcmpManagerTest, RemoveNextHopWithPrunedMember) +{ + // Add member with operationally down watch port. Since associated watchport + // is operationally down, member will not be created in SAI but will be + // directly added to the pruned set of WCMP group members. + P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport("Ethernet1"); + EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); + + // Verify that next hop reference count is incremented due to the member. + uint32_t ref_cnt; + EXPECT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey1, &ref_cnt)); + EXPECT_EQ(1, ref_cnt); // Remove Wcmp group. No SAI call for member removal is expected as it is // already pruned. @@ -1573,7 +1947,81 @@ TEST_F(WcmpManagerTest, RemoveWcmpGroupWithOperationallyDownWatchportSucceeds) .WillOnce(Return(SAI_STATUS_SUCCESS)); EXPECT_EQ(StatusCode::SWSS_RC_SUCCESS, RemoveWcmpGroup(kWcmpGroupId1)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], false, 0)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); + + // Verify that the next hop reference count is now 0. + EXPECT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey1, &ref_cnt)); + EXPECT_EQ(0, ref_cnt); +} + +TEST_F(WcmpManagerTest, RemoveNextHopWithRestoredPrunedMember) +{ + // Add member with operationally down watch port. Since associated watchport + // is operationally down, member will not be created in SAI but will be + // directly added to the pruned set of WCMP group members. + std::string port_name = "Ethernet1"; + P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name); + EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); + + // Verify that next hop reference count is incremented due to the member. + uint32_t ref_cnt; + EXPECT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey1, &ref_cnt)); + EXPECT_EQ(1, ref_cnt); + + // Restore member associated with port. + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), + Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, + kWcmpGroupOid1, std::placeholders::_1)))) + .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); + RestorePrunedNextHops(port_name); + EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_FALSE(app_db_entry.wcmp_group_members[0]->pruned); + + // Verify that next hop reference count remains the same after restore. + EXPECT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey1, &ref_cnt)); + EXPECT_EQ(1, ref_cnt); + + // Remove Wcmp group. + std::vector exp_remove_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group(Eq(kWcmpGroupOid1))) + .WillOnce(Return(SAI_STATUS_SUCCESS)); + EXPECT_EQ(StatusCode::SWSS_RC_SUCCESS, RemoveWcmpGroup(kWcmpGroupId1)); + EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], false, 0)); + + // Verify that the next hop reference count is now 0. + EXPECT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey1, &ref_cnt)); + EXPECT_EQ(0, ref_cnt); +} + +TEST_F(WcmpManagerTest, VerifyNextHopRefCountWhenMemberPruned) +{ + // Add member with operationally up watch port + std::string port_name = "Ethernet6"; + P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name, true); + EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_FALSE(app_db_entry.wcmp_group_members[0]->pruned); + + // Verify that next hop reference count is incremented due to the member. + uint32_t ref_cnt; + EXPECT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey1, &ref_cnt)); + EXPECT_EQ(1, ref_cnt); + + // Prune member associated with port. + EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) + .WillOnce(Return(SAI_STATUS_SUCCESS)); + PruneNextHops(port_name); + EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); + + // Verify that next hop reference count does not change on pruning. + EXPECT_TRUE(p4_oid_mapper_->getRefCount(SAI_OBJECT_TYPE_NEXT_HOP, kNexthopKey1, &ref_cnt)); + EXPECT_EQ(1, ref_cnt); } TEST_F(WcmpManagerTest, UpdateWcmpGroupWithOperationallyUpWatchportMemberSucceeds) @@ -1582,7 +2030,7 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupWithOperationallyUpWatchportMemberSucceed std::string port_name = "Ethernet6"; P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name, true); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); + EXPECT_FALSE(app_db_entry.wcmp_group_members[0]->pruned); // Update WCMP group to remove kNexthopId1 and add kNexthopId2 P4WcmpGroupEntry updated_app_db_entry; @@ -1590,18 +2038,27 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupWithOperationallyUpWatchportMemberSucceed std::shared_ptr updated_gm = createWcmpGroupMemberEntryWithWatchport(kNexthopId2, 1, port_name, kWcmpGroupId1, kNexthopOid2); updated_app_db_entry.wcmp_group_members.push_back(updated_gm); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid2), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); + std::vector return_oids{kWcmpGroupMemberOid2}; + std::vector exp_create_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); + std::vector exp_remove_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); EXPECT_EQ(StatusCode::SWSS_RC_SUCCESS, ProcessUpdateRequest(&updated_app_db_entry)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], false, 1)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(updated_gm, true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(updated_gm, false, 0)); + EXPECT_FALSE(app_db_entry.wcmp_group_members[0]->pruned); + EXPECT_FALSE(updated_gm->pruned); } TEST_F(WcmpManagerTest, UpdateWcmpGroupWithOperationallyDownWatchportMemberSucceeds) @@ -1612,7 +2069,7 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupWithOperationallyDownWatchportMemberSucce std::string port_name = "Ethernet1"; P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); // Update WCMP group to remove kNexthopId1 and add kNexthopId2. No SAI calls // are expected as the associated watch port is operationally down. @@ -1624,8 +2081,7 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupWithOperationallyDownWatchportMemberSucce EXPECT_EQ(StatusCode::SWSS_RC_SUCCESS, ProcessUpdateRequest(&updated_app_db_entry)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], false, 1)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(updated_gm, true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(updated_gm, true, 1)); + EXPECT_TRUE(updated_gm->pruned); } TEST_F(WcmpManagerTest, PruneAfterWcmpGroupUpdateSucceeds) @@ -1634,7 +2090,7 @@ TEST_F(WcmpManagerTest, PruneAfterWcmpGroupUpdateSucceeds) std::string port_name = "Ethernet6"; P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name, true); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); + EXPECT_FALSE(app_db_entry.wcmp_group_members[0]->pruned); // Update WCMP group to modify weight of kNexthopId1. P4WcmpGroupEntry updated_app_db_entry; @@ -1642,25 +2098,33 @@ TEST_F(WcmpManagerTest, PruneAfterWcmpGroupUpdateSucceeds) std::shared_ptr updated_gm = createWcmpGroupMemberEntryWithWatchport(kNexthopId1, 10, port_name, kWcmpGroupId1, kNexthopOid1); updated_app_db_entry.wcmp_group_members.push_back(updated_gm); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 10, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); + std::vector exp_remove_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + std::vector return_oids{kWcmpGroupMemberOid1}; + std::vector exp_create_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 10, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids.begin(), return_oids.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); EXPECT_EQ(StatusCode::SWSS_RC_SUCCESS, ProcessUpdateRequest(&updated_app_db_entry)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], false, 1)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(updated_app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(updated_app_db_entry.wcmp_group_members[0], false, 0)); + EXPECT_FALSE(updated_app_db_entry.wcmp_group_members[0]->pruned); // Prune members associated with port. EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) .WillOnce(Return(SAI_STATUS_SUCCESS)); PruneNextHops(port_name); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(updated_app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(updated_app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(updated_app_db_entry.wcmp_group_members[0]->pruned); // Remove Wcmp group. No SAI call for member removal is expected as it is // already pruned. @@ -1671,7 +2135,6 @@ TEST_F(WcmpManagerTest, PruneAfterWcmpGroupUpdateSucceeds) .WillOnce(Return(SAI_STATUS_SUCCESS)); EXPECT_EQ(StatusCode::SWSS_RC_SUCCESS, RemoveWcmpGroup(kWcmpGroupId1)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(updated_app_db_entry.wcmp_group_members[0], false, 0)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(updated_app_db_entry.wcmp_group_members[0], false, 0)); } TEST_F(WcmpManagerTest, PrunedMemberUpdateOnRestoreSucceeds) @@ -1682,7 +2145,7 @@ TEST_F(WcmpManagerTest, PrunedMemberUpdateOnRestoreSucceeds) std::string port_name = "Ethernet1"; P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); // Update WCMP group to modify weight of kNexthopId1. P4WcmpGroupEntry updated_app_db_entry; @@ -1693,8 +2156,7 @@ TEST_F(WcmpManagerTest, PrunedMemberUpdateOnRestoreSucceeds) EXPECT_EQ(StatusCode::SWSS_RC_SUCCESS, ProcessUpdateRequest(&updated_app_db_entry)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], false, 1)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(updated_app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(updated_app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(updated_app_db_entry.wcmp_group_members[0]->pruned); // Restore members associated with port. // Verify that the weight of the restored member is updated. @@ -1705,7 +2167,7 @@ TEST_F(WcmpManagerTest, PrunedMemberUpdateOnRestoreSucceeds) .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); RestorePrunedNextHops(port_name); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(updated_app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(updated_app_db_entry.wcmp_group_members[0], false, 0)); + EXPECT_FALSE(updated_app_db_entry.wcmp_group_members[0]->pruned); } TEST_F(WcmpManagerTest, UpdateWcmpGroupWithOperationallyUpWatchportMemberFailsWithMemberRemovalFailure) @@ -1714,7 +2176,7 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupWithOperationallyUpWatchportMemberFailsWi std::string port_name = "Ethernet6"; P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name, true); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); + EXPECT_FALSE(app_db_entry.wcmp_group_members[0]->pruned); // Update WCMP group to remove kNexthopId1(fails) and add kNexthopId2 P4WcmpGroupEntry updated_app_db_entry; @@ -1725,61 +2187,96 @@ TEST_F(WcmpManagerTest, UpdateWcmpGroupWithOperationallyUpWatchportMemberFailsWi createWcmpGroupMemberEntryWithWatchport(kNexthopId1, 1, port_name, kWcmpGroupId1, kNexthopOid1); updated_app_db_entry.wcmp_group_members.push_back(updated_gm1); updated_app_db_entry.wcmp_group_members.push_back(updated_gm2); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid4), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 10, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(Return(SAI_STATUS_INSUFFICIENT_RESOURCES)); + std::vector exp_remove_status{SAI_STATUS_SUCCESS}; + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + std::vector return_oids_4{kWcmpGroupMemberOid4}; + std::vector return_oids_null{SAI_NULL_OBJECT_ID}; + std::vector exp_create_status{SAI_STATUS_SUCCESS}; + std::vector exp_create_status_fail{SAI_STATUS_INSUFFICIENT_RESOURCES}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_4.begin(), return_oids_4.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 10, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_null.begin(), return_oids_null.end()), + SetArrayArgument<6>(exp_create_status_fail.begin(), exp_create_status_fail.end()), + Return(SAI_STATUS_INSUFFICIENT_RESOURCES))); // Clean up created member-succeeds - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid4))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_EQ(StatusCode::SWSS_RC_FULL, ProcessUpdateRequest(&updated_app_db_entry)); + std::vector return_oids_1{kWcmpGroupMemberOid1}; + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 2, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_1.begin(), return_oids_1.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid4}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + EXPECT_EQ(StatusCode::SWSS_RC_UNKNOWN, ProcessUpdateRequest(&updated_app_db_entry)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(updated_gm2, false, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(updated_gm2, false, 0)); + EXPECT_FALSE(app_db_entry.wcmp_group_members[0]->pruned); + EXPECT_FALSE(updated_gm2->pruned); // Update again, this time clean up fails - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid1))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); - EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 1, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid4), Return(SAI_STATUS_SUCCESS))); EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid2, 10, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(Return(SAI_STATUS_INSUFFICIENT_RESOURCES)); + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid1}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 1, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_4.begin(), return_oids_4.end()), + SetArrayArgument<6>(exp_create_status.begin(), exp_create_status.end()), + Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid2, 10, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_null.begin(), return_oids_null.end()), + SetArrayArgument<6>(exp_create_status_fail.begin(), exp_create_status_fail.end()), + Return(SAI_STATUS_INSUFFICIENT_RESOURCES))); // Clean up created member(fails) - EXPECT_CALL(mock_sai_next_hop_group_, remove_next_hop_group_member(Eq(kWcmpGroupMemberOid4))) - .WillOnce(Return(SAI_STATUS_SUCCESS)); EXPECT_CALL(mock_sai_next_hop_group_, - create_next_hop_group_member(_, Eq(gSwitchId), Eq(3), - Truly(std::bind(MatchSaiNextHopGroupMemberAttribute, kNexthopOid1, 2, - kWcmpGroupOid1, std::placeholders::_1)))) - .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_INSUFFICIENT_RESOURCES))); + remove_next_hop_group_members(Eq(1), ArrayEq(std::vector{kWcmpGroupMemberOid4}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _)) + .WillOnce( + DoAll(SetArrayArgument<3>(exp_remove_status.begin(), exp_remove_status.end()), Return(SAI_STATUS_SUCCESS))); + EXPECT_CALL(mock_sai_next_hop_group_, + create_next_hop_group_members(Eq(gSwitchId), Eq(1), ArrayEq(std::vector{3}), + AttrArrayArrayEq(std::vector>{ + GetSaiNextHopGroupMemberAttribute(kNexthopOid1, 2, kWcmpGroupOid1)}), + Eq(SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR), _, _)) + .WillOnce(DoAll(SetArrayArgument<5>(return_oids_null.begin(), return_oids_null.end()), + SetArrayArgument<6>(exp_create_status_fail.begin(), exp_create_status_fail.end()), + Return(SAI_STATUS_INSUFFICIENT_RESOURCES))); // TODO: Expect critical state. - EXPECT_EQ("Failed to create next hop group member " - "'ju1u32m2.atl11:qe-3/7'", + EXPECT_EQ("Fail to create wcmp group member: 'ju1u32m2.atl11:qe-3/7'", ProcessUpdateRequest(&updated_app_db_entry).message()); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], false, 0)); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(updated_gm2, false, 0)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(updated_gm2, false, 0)); + EXPECT_FALSE(app_db_entry.wcmp_group_members[0]->pruned); + EXPECT_FALSE(updated_gm2->pruned); } TEST_F(WcmpManagerTest, WatchportStateChangetoOperDownSucceeds) @@ -1788,7 +2285,7 @@ TEST_F(WcmpManagerTest, WatchportStateChangetoOperDownSucceeds) std::string port_name = "Ethernet6"; P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name, true); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); + EXPECT_FALSE(app_db_entry.wcmp_group_members[0]->pruned); // Send port down signal // Verify that the next hop member associated with the port is pruned. @@ -1799,7 +2296,7 @@ TEST_F(WcmpManagerTest, WatchportStateChangetoOperDownSucceeds) .WillOnce(Return(SAI_STATUS_SUCCESS)); HandlePortStatusChangeNotification(op, data); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); } TEST_F(WcmpManagerTest, WatchportStateChangeToOperUpSucceeds) @@ -1810,7 +2307,7 @@ TEST_F(WcmpManagerTest, WatchportStateChangeToOperUpSucceeds) std::string port_name = "Ethernet1"; P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); // Send port up signal. // Verify that the pruned next hop member associated with the port is @@ -1825,7 +2322,7 @@ TEST_F(WcmpManagerTest, WatchportStateChangeToOperUpSucceeds) .WillOnce(DoAll(SetArgPointee<0>(kWcmpGroupMemberOid1), Return(SAI_STATUS_SUCCESS))); HandlePortStatusChangeNotification(op, data); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], false, 0)); + EXPECT_FALSE(app_db_entry.wcmp_group_members[0]->pruned); } TEST_F(WcmpManagerTest, WatchportStateChangeFromOperUnknownToDownPrunesMemberOnlyOnceSuceeds) @@ -1836,7 +2333,7 @@ TEST_F(WcmpManagerTest, WatchportStateChangeFromOperUnknownToDownPrunesMemberOnl std::string port_name = "Ethernet1"; P4WcmpGroupEntry app_db_entry = AddWcmpGroupEntryWithWatchport(port_name); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); // Send port down signal. // Verify that the pruned next hop member is not pruned again. @@ -1845,7 +2342,7 @@ TEST_F(WcmpManagerTest, WatchportStateChangeFromOperUnknownToDownPrunesMemberOnl "STATUS_DOWN\"}]"; HandlePortStatusChangeNotification(op, data); EXPECT_TRUE(VerifyWcmpGroupMemberInPortMap(app_db_entry.wcmp_group_members[0], true, 1)); - EXPECT_TRUE(VerifyWcmpGroupMemberInPrunedSet(app_db_entry.wcmp_group_members[0], true, 1)); + EXPECT_TRUE(app_db_entry.wcmp_group_members[0]->pruned); } TEST_F(WcmpManagerTest, VerifyStateTest) diff --git a/orchagent/p4orch/wcmp_manager.cpp b/orchagent/p4orch/wcmp_manager.cpp index 96c0a12eb30d..446a58e021c6 100644 --- a/orchagent/p4orch/wcmp_manager.cpp +++ b/orchagent/p4orch/wcmp_manager.cpp @@ -24,6 +24,7 @@ extern sai_object_id_t gSwitchId; extern sai_next_hop_group_api_t *sai_next_hop_group_api; extern CrmOrch *gCrmOrch; extern PortsOrch *gPortsOrch; +extern size_t gMaxBulkSize; namespace p4orch { @@ -51,6 +52,17 @@ std::vector getSaiGroupAttrs(const P4WcmpGroupEntry &wcmp_group } // namespace +WcmpManager::WcmpManager(P4OidMapper *p4oidMapper, ResponsePublisherInterface *publisher) + : gNextHopGroupMemberBulker(sai_next_hop_group_api, gSwitchId, gMaxBulkSize) +{ + SWSS_LOG_ENTER(); + + assert(p4oidMapper != nullptr); + m_p4OidMapper = p4oidMapper; + assert(publisher != nullptr); + m_publisher = publisher; +} + std::vector WcmpManager::getSaiMemberAttrs(const P4WcmpGroupMemberEntry &wcmp_member_entry, const sai_object_id_t group_oid) { @@ -165,6 +177,7 @@ ReturnCodeOr WcmpManager::deserializeP4WcmpGroupAppDbEntry( wcmp_group_member->watch_port = action_item[kWatchPort]; } wcmp_group_member->wcmp_group_id = app_db_entry.wcmp_group_id; + wcmp_group_member->pruned = false; app_db_entry.wcmp_group_members.push_back(wcmp_group_member); } } @@ -196,14 +209,7 @@ P4WcmpGroupEntry *WcmpManager::getWcmpGroupEntry(const std::string &wcmp_group_i ReturnCode WcmpManager::processAddRequest(P4WcmpGroupEntry *app_db_entry) { SWSS_LOG_ENTER(); - auto status = validateWcmpGroupEntry(*app_db_entry); - if (!status.ok()) - { - SWSS_LOG_ERROR("Invalid WCMP group with id %s: %s", QuotedVar(app_db_entry->wcmp_group_id).c_str(), - status.message().c_str()); - return status; - } - status = createWcmpGroup(app_db_entry); + auto status = createWcmpGroup(app_db_entry); if (!status.ok()) { SWSS_LOG_ERROR("Failed to create WCMP group with id %s: %s", QuotedVar(app_db_entry->wcmp_group_id).c_str(), @@ -223,14 +229,10 @@ ReturnCode WcmpManager::createWcmpGroupMember(std::shared_ptrnext_hop_id)); // Update reference count - const auto &next_hop_key = KeyGenerator::generateNextHopKey(wcmp_group_member->next_hop_id); m_p4OidMapper->setOID(SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER, getWcmpGroupMemberKey(wcmp_group_key, wcmp_group_member->member_oid), wcmp_group_member->member_oid); - gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP_MEMBER); - m_p4OidMapper->increaseRefCount(SAI_OBJECT_TYPE_NEXT_HOP, next_hop_key); m_p4OidMapper->increaseRefCount(SAI_OBJECT_TYPE_NEXT_HOP_GROUP, wcmp_group_key); - return ReturnCode(); } @@ -277,87 +279,81 @@ ReturnCode WcmpManager::fetchPortOperStatus(const std::string &port_name, sai_po return ReturnCode(); } -ReturnCode WcmpManager::createWcmpGroupMemberWithWatchport(P4WcmpGroupEntry *wcmp_group, - std::shared_ptr member, - const std::string &wcmp_group_key) +ReturnCode WcmpManager::processWcmpGroupMembersAddition( + const std::vector> &members, const std::string &wcmp_group_key, + sai_object_id_t wcmp_group_oid, std::vector> &created_wcmp_group_members) { - // Create member in SAI only for operationally up ports - sai_port_oper_status_t oper_status = SAI_PORT_OPER_STATUS_DOWN; - auto status = fetchPortOperStatus(member->watch_port, &oper_status); - if (!status.ok()) - { - return status; - } - - if (oper_status == SAI_PORT_OPER_STATUS_UP) + SWSS_LOG_ENTER(); + ReturnCode status; + vector nhgm_ids(members.size(), SAI_NULL_OBJECT_ID); + for (size_t i = 0; i < members.size(); ++i) { - auto status = createWcmpGroupMember(member, wcmp_group->wcmp_group_oid, wcmp_group_key); - if (!status.ok()) + bool insert_member = true; + auto &member = members[i]; + if (!member->watch_port.empty()) { - SWSS_LOG_ERROR("Failed to create next hop member %s with watch_port %s", member->next_hop_id.c_str(), - member->watch_port.c_str()); - return status; - } - } - else - { - pruned_wcmp_members_set.emplace(member); - SWSS_LOG_NOTICE("Member %s in group %s not created in asic as the associated watchport " - "(%s) is not operationally up", - member->next_hop_id.c_str(), member->wcmp_group_id.c_str(), member->watch_port.c_str()); - } - // Add member to port_name_to_wcmp_group_member_map - insertMemberInPortNameToWcmpGroupMemberMap(member); - return ReturnCode(); -} + // Create member in SAI only for operationally up ports + sai_port_oper_status_t oper_status = SAI_PORT_OPER_STATUS_DOWN; + status = fetchPortOperStatus(member->watch_port, &oper_status); + if (!status.ok()) + { + break; + } -ReturnCode WcmpManager::processWcmpGroupMemberAddition(std::shared_ptr member, - P4WcmpGroupEntry *wcmp_group, const std::string &wcmp_group_key) -{ - ReturnCode status = ReturnCode(); - if (!member->watch_port.empty()) - { - status = createWcmpGroupMemberWithWatchport(wcmp_group, member, wcmp_group_key); - if (!status.ok()) - { - SWSS_LOG_ERROR("Failed to create WCMP group member %s with watch_port %s", member->next_hop_id.c_str(), - member->watch_port.c_str()); + if (oper_status != SAI_PORT_OPER_STATUS_UP) + { + insert_member = false; + member->pruned = true; + SWSS_LOG_NOTICE("Member %s in group %s not created in asic as the associated " + "watchport " + "(%s) is not operationally up", + member->next_hop_id.c_str(), member->wcmp_group_id.c_str(), member->watch_port.c_str()); + } } - } - else - { - status = createWcmpGroupMember(member, wcmp_group->wcmp_group_oid, wcmp_group_key); - if (!status.ok()) + if (insert_member) { - SWSS_LOG_ERROR("Failed to create WCMP group member %s", member->next_hop_id.c_str()); + auto attrs = getSaiMemberAttrs(*(member.get()), wcmp_group_oid); + gNextHopGroupMemberBulker.create_entry(&nhgm_ids[i], (uint32_t)attrs.size(), attrs.data()); } } - return status; -} - -ReturnCode WcmpManager::processWcmpGroupMemberRemoval(std::shared_ptr member, - const std::string &wcmp_group_key) -{ - // If member exists in pruned_wcmp_members_set, remove from set. Else, remove - // member using SAI. - auto it = pruned_wcmp_members_set.find(member); - if (it != pruned_wcmp_members_set.end()) - { - pruned_wcmp_members_set.erase(it); - SWSS_LOG_NOTICE("Removed pruned member %s from group %s", member->next_hop_id.c_str(), - member->wcmp_group_id.c_str()); - } - else + if (status.ok()) { - auto status = removeWcmpGroupMember(member, wcmp_group_key); - if (!status.ok()) + gNextHopGroupMemberBulker.flush(); + for (size_t i = 0; i < members.size(); ++i) { - return status; + auto &member = members[i]; + if (!member->pruned) + { + if (nhgm_ids[i] == SAI_NULL_OBJECT_ID) + { + if (status.ok()) + { + status = ReturnCode(StatusCode::SWSS_RC_UNKNOWN) + << "Fail to create wcmp group member: " << QuotedVar(member->next_hop_id); + } + else + { + status << "; Fail to create wcmp group member: " << QuotedVar(member->next_hop_id); + } + continue; + } + member->member_oid = nhgm_ids[i]; + m_p4OidMapper->setOID(SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER, + getWcmpGroupMemberKey(wcmp_group_key, member->member_oid), member->member_oid); + m_p4OidMapper->increaseRefCount(SAI_OBJECT_TYPE_NEXT_HOP_GROUP, wcmp_group_key); + } + if (!member->watch_port.empty()) + { + // Add member to port_name_to_wcmp_group_member_map + insertMemberInPortNameToWcmpGroupMemberMap(member); + } + const std::string &next_hop_key = KeyGenerator::generateNextHopKey(member->next_hop_id); + gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP_MEMBER); + m_p4OidMapper->increaseRefCount(SAI_OBJECT_TYPE_NEXT_HOP, next_hop_key); + created_wcmp_group_members.push_back(member); } } - // Remove member from port_name_to_wcmp_group_member_map - removeMemberFromPortNameToWcmpGroupMemberMap(member); - return ReturnCode(); + return status; } ReturnCode WcmpManager::createWcmpGroup(P4WcmpGroupEntry *wcmp_group) @@ -375,16 +371,8 @@ ReturnCode WcmpManager::createWcmpGroup(P4WcmpGroupEntry *wcmp_group) // Create next hop group members std::vector> created_wcmp_group_members; - ReturnCode status; - for (auto &wcmp_group_member : wcmp_group->wcmp_group_members) - { - status = processWcmpGroupMemberAddition(wcmp_group_member, wcmp_group, wcmp_group_key); - if (!status.ok()) - { - break; - } - created_wcmp_group_members.push_back(wcmp_group_member); - } + ReturnCode status = processWcmpGroupMembersAddition(wcmp_group->wcmp_group_members, wcmp_group_key, + wcmp_group->wcmp_group_oid, created_wcmp_group_members); if (!status.ok()) { // Clean up created group members and the group @@ -401,6 +389,7 @@ ReturnCode WcmpManager::createWcmpGroup(P4WcmpGroupEntry *wcmp_group) m_p4OidMapper->eraseOID(SAI_OBJECT_TYPE_NEXT_HOP_GROUP, wcmp_group_key); return status; } + m_wcmpGroupTable[wcmp_group->wcmp_group_id] = *wcmp_group; return ReturnCode(); } @@ -410,33 +399,28 @@ void WcmpManager::recoverGroupMembers( const std::vector> &created_wcmp_group_members, const std::vector> &removed_wcmp_group_members) { - // Keep track of recovery status during clean up + SWSS_LOG_ENTER(); + std::vector> members; ReturnCode recovery_status; // Clean up created group members - remove created new members - for (const auto &new_member : created_wcmp_group_members) + if (created_wcmp_group_members.size() != 0) { - auto status = processWcmpGroupMemberRemoval(new_member, wcmp_group_key); - if (!status.ok()) - { - SWSS_LOG_ERROR("Failed to remove created next hop group member %s in " - "processUpdateRequest().", - QuotedVar(new_member->next_hop_id).c_str()); - recovery_status.ok() ? recovery_status = status.prepend("Error during recovery: ") - : recovery_status << "; Error during recovery: " << status.message(); - } + recovery_status = processWcmpGroupMembersRemoval(created_wcmp_group_members, wcmp_group_key, members) + .prepend("Error during recovery: "); } + // Clean up removed group members - create removed old members - for (auto &old_member : removed_wcmp_group_members) + if (recovery_status.ok() && removed_wcmp_group_members.size() != 0) { - auto status = processWcmpGroupMemberAddition(old_member, wcmp_group_entry, wcmp_group_key); - if (!status.ok()) - { - recovery_status.ok() ? recovery_status = status.prepend("Error during recovery: ") - : recovery_status << "; Error during recovery: " << status.message(); - } + recovery_status = processWcmpGroupMembersAddition(removed_wcmp_group_members, wcmp_group_key, + wcmp_group_entry->wcmp_group_oid, members) + .prepend("Error during recovery: "); } + if (!recovery_status.ok()) + { SWSS_RAISE_CRITICAL_STATE(recovery_status.message()); + } } ReturnCode WcmpManager::processUpdateRequest(P4WcmpGroupEntry *wcmp_group_entry) @@ -459,93 +443,88 @@ ReturnCode WcmpManager::processUpdateRequest(P4WcmpGroupEntry *wcmp_group_entry) // 5. Make SAI call to remove the reserved old member // 6. Make SAI calls to create remaining new members ReturnCode update_request_status; - auto find_smallest_index = [&](p4orch::P4WcmpGroupEntry *wcmp) { + auto find_smallest_index = [&](p4orch::P4WcmpGroupEntry *wcmp, + std::vector> &other_members) -> int { + other_members.clear(); if (wcmp->wcmp_group_members.empty()) + { return -1; + } int reserved_idx = 0; for (int i = 1; i < (int)wcmp->wcmp_group_members.size(); i++) { if (wcmp->wcmp_group_members[i]->weight < wcmp->wcmp_group_members[reserved_idx]->weight) { + other_members.push_back(wcmp->wcmp_group_members[reserved_idx]); reserved_idx = i; } + else + { + other_members.push_back(wcmp->wcmp_group_members[i]); + } } return reserved_idx; }; // Find the old member who has the smallest weight, -1 if the member list is // empty - int reserved_old_member_index = find_smallest_index(old_wcmp); + std::vector> other_old_members; + int reserved_old_member_index = find_smallest_index(old_wcmp, other_old_members); // Find the new member who has the smallest weight, -1 if the member list is // empty - int reserved_new_member_index = find_smallest_index(wcmp_group_entry); + std::vector> other_new_members; + int reserved_new_member_index = find_smallest_index(wcmp_group_entry, other_new_members); // Remove stale group members except the member with the smallest weight - for (int i = 0; i < (int)old_wcmp->wcmp_group_members.size(); i++) + if (other_old_members.size() != 0) { - // Reserve the old member with smallest weight - if (i == reserved_old_member_index) - continue; - auto &stale_member = old_wcmp->wcmp_group_members[i]; - update_request_status = processWcmpGroupMemberRemoval(stale_member, wcmp_group_key); + update_request_status = + processWcmpGroupMembersRemoval(other_old_members, wcmp_group_key, removed_wcmp_group_members); if (!update_request_status.ok()) { - SWSS_LOG_ERROR("Failed to remove stale next hop group member %s in " - "processUpdateRequest().", - QuotedVar(sai_serialize_object_id(stale_member->member_oid)).c_str()); recoverGroupMembers(wcmp_group_entry, wcmp_group_key, created_wcmp_group_members, removed_wcmp_group_members); return update_request_status; } - removed_wcmp_group_members.push_back(stale_member); } // Create the new member with the smallest weight if member list is nonempty - if (!wcmp_group_entry->wcmp_group_members.empty()) + if (reserved_new_member_index != -1) { - auto &member = wcmp_group_entry->wcmp_group_members[reserved_new_member_index]; - update_request_status = processWcmpGroupMemberAddition(member, wcmp_group_entry, wcmp_group_key); + update_request_status = processWcmpGroupMembersAddition( + {wcmp_group_entry->wcmp_group_members[reserved_new_member_index]}, wcmp_group_key, + wcmp_group_entry->wcmp_group_oid, created_wcmp_group_members); if (!update_request_status.ok()) { recoverGroupMembers(wcmp_group_entry, wcmp_group_key, created_wcmp_group_members, removed_wcmp_group_members); return update_request_status; } - created_wcmp_group_members.push_back(member); } // Remove the old member with the smallest weight if member list is nonempty - if (!old_wcmp->wcmp_group_members.empty()) + if (reserved_old_member_index != -1) { - auto &stale_member = old_wcmp->wcmp_group_members[reserved_old_member_index]; - update_request_status = processWcmpGroupMemberRemoval(stale_member, wcmp_group_key); + update_request_status = processWcmpGroupMembersRemoval( + {old_wcmp->wcmp_group_members[reserved_old_member_index]}, wcmp_group_key, removed_wcmp_group_members); if (!update_request_status.ok()) { - SWSS_LOG_ERROR("Failed to remove stale next hop group member %s in " - "processUpdateRequest().", - QuotedVar(sai_serialize_object_id(stale_member->member_oid)).c_str()); recoverGroupMembers(wcmp_group_entry, wcmp_group_key, created_wcmp_group_members, removed_wcmp_group_members); return update_request_status; } - removed_wcmp_group_members.push_back(stale_member); } // Create new group members - for (int i = 0; i < (int)wcmp_group_entry->wcmp_group_members.size(); i++) + if (other_new_members.size() != 0) { - // Skip the new member with the lowest weight as it is already created - if (i == reserved_new_member_index) - continue; - auto &member = wcmp_group_entry->wcmp_group_members[i]; - // Create new group member - update_request_status = processWcmpGroupMemberAddition(member, wcmp_group_entry, wcmp_group_key); + update_request_status = processWcmpGroupMembersAddition( + other_new_members, wcmp_group_key, wcmp_group_entry->wcmp_group_oid, created_wcmp_group_members); if (!update_request_status.ok()) { recoverGroupMembers(wcmp_group_entry, wcmp_group_key, created_wcmp_group_members, removed_wcmp_group_members); return update_request_status; } - created_wcmp_group_members.push_back(member); } m_wcmpGroupTable[wcmp_group_entry->wcmp_group_id] = *wcmp_group_entry; @@ -556,19 +535,72 @@ ReturnCode WcmpManager::removeWcmpGroupMember(const std::shared_ptrnext_hop_id); CHECK_ERROR_AND_LOG_AND_RETURN(sai_next_hop_group_api->remove_next_hop_group_member(wcmp_group_member->member_oid), "Failed to remove WCMP group member with nexthop id " << QuotedVar(wcmp_group_member->next_hop_id)); m_p4OidMapper->eraseOID(SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER, getWcmpGroupMemberKey(wcmp_group_key, wcmp_group_member->member_oid)); - gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP_MEMBER); - m_p4OidMapper->decreaseRefCount(SAI_OBJECT_TYPE_NEXT_HOP, next_hop_key); m_p4OidMapper->decreaseRefCount(SAI_OBJECT_TYPE_NEXT_HOP_GROUP, wcmp_group_key); return ReturnCode(); } +ReturnCode WcmpManager::processWcmpGroupMembersRemoval( + const std::vector> &members, const std::string &wcmp_group_key, + std::vector> &removed_wcmp_group_members) +{ + SWSS_LOG_ENTER(); + ReturnCode status; + std::vector statuses(members.size(), SAI_STATUS_FAILURE); + for (size_t i = 0; i < members.size(); ++i) + { + auto &member = members[i]; + if (!member->pruned) + { + gNextHopGroupMemberBulker.remove_entry(&statuses[i], member->member_oid); + } + } + gNextHopGroupMemberBulker.flush(); + for (size_t i = 0; i < members.size(); ++i) + { + auto &member = members[i]; + if (member->pruned) + { + SWSS_LOG_NOTICE("Removed pruned member %s from group %s", member->next_hop_id.c_str(), + member->wcmp_group_id.c_str()); + member->pruned = false; + } + else + { + if (statuses[i] != SAI_STATUS_SUCCESS) + { + if (status.ok()) + { + status = ReturnCode(statuses[i]) + << "Failed to delete WCMP group member: " << QuotedVar(member->next_hop_id); + } + else + { + status << "; Failed to delete WCMP group member: " << QuotedVar(member->next_hop_id); + } + continue; + } + else + { + m_p4OidMapper->eraseOID(SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER, + getWcmpGroupMemberKey(wcmp_group_key, member->member_oid)); + m_p4OidMapper->decreaseRefCount(SAI_OBJECT_TYPE_NEXT_HOP_GROUP, wcmp_group_key); + } + } + const std::string &next_hop_key = KeyGenerator::generateNextHopKey(member->next_hop_id); + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP_MEMBER); + m_p4OidMapper->decreaseRefCount(SAI_OBJECT_TYPE_NEXT_HOP, next_hop_key); + removeMemberFromPortNameToWcmpGroupMemberMap(member); + removed_wcmp_group_members.push_back(member); + } + return status; +} + ReturnCode WcmpManager::removeWcmpGroup(const std::string &wcmp_group_id) { SWSS_LOG_ENTER(); @@ -590,18 +622,12 @@ ReturnCode WcmpManager::removeWcmpGroup(const std::string &wcmp_group_id) << wcmp_group_refcount - expected_refcount << " more objects than its group members (size=" << expected_refcount << ") referencing it."); } - std::vector> removed_wcmp_group_members; - ReturnCode status; + // Delete group members - for (const auto &member : wcmp_group->wcmp_group_members) - { - status = processWcmpGroupMemberRemoval(member, wcmp_group_key); - if (!status.ok()) - { - break; - } - removed_wcmp_group_members.push_back(member); - } + std::vector> removed_wcmp_group_members; + ReturnCode status = + processWcmpGroupMembersRemoval(wcmp_group->wcmp_group_members, wcmp_group_key, removed_wcmp_group_members); + // Delete group if (status.ok()) { @@ -631,24 +657,22 @@ void WcmpManager::pruneNextHops(const std::string &port) { for (const auto &member : port_name_to_wcmp_group_member_map[port]) { - auto it = pruned_wcmp_members_set.find(member); // Prune a member if it is not already pruned. - if (it == pruned_wcmp_members_set.end()) + if (!member->pruned) { const auto &wcmp_group_key = KeyGenerator::generateWcmpGroupKey(member->wcmp_group_id); auto status = removeWcmpGroupMember(member, wcmp_group_key); if (!status.ok()) { - SWSS_LOG_NOTICE("Failed to remove member %s from group %s, rv: %s", member->next_hop_id.c_str(), - member->wcmp_group_id.c_str(), status.message().c_str()); - } - else - { - // Add pruned member to pruned set - pruned_wcmp_members_set.emplace(member); - SWSS_LOG_NOTICE("Pruned member %s from group %s", member->next_hop_id.c_str(), - member->wcmp_group_id.c_str()); + std::stringstream msg; + msg << "Failed to prune member " << member->next_hop_id << " from group " << member->wcmp_group_id + << ": " << status.message(); + SWSS_RAISE_CRITICAL_STATE(msg.str()); + return; } + member->pruned = true; + SWSS_LOG_NOTICE("Pruned member %s from group %s", member->next_hop_id.c_str(), + member->wcmp_group_id.c_str()); } } } @@ -665,8 +689,7 @@ void WcmpManager::restorePrunedNextHops(const std::string &port) ReturnCode status; for (auto member : port_name_to_wcmp_group_member_map[port]) { - auto it = pruned_wcmp_members_set.find(member); - if (it != pruned_wcmp_members_set.end()) + if (member->pruned) { const auto &wcmp_group_key = KeyGenerator::generateWcmpGroupKey(member->wcmp_group_id); sai_object_id_t wcmp_group_oid = SAI_NULL_OBJECT_ID; @@ -688,7 +711,7 @@ void WcmpManager::restorePrunedNextHops(const std::string &port) SWSS_RAISE_CRITICAL_STATE(status.message()); return; } - pruned_wcmp_members_set.erase(it); + member->pruned = false; SWSS_LOG_NOTICE("Restored pruned member %s in group %s", member->next_hop_id.c_str(), member->wcmp_group_id.c_str()); } @@ -744,6 +767,16 @@ void WcmpManager::drain() const std::string &operation = kfvOp(key_op_fvs_tuple); if (operation == SET_COMMAND) { + status = validateWcmpGroupEntry(app_db_entry); + if (!status.ok()) + { + SWSS_LOG_ERROR("Invalid WCMP group with id %s: %s", QuotedVar(app_db_entry.wcmp_group_id).c_str(), + status.message().c_str()); + m_publisher->publish(APP_P4RT_TABLE_NAME, kfvKey(key_op_fvs_tuple), kfvFieldsValues(key_op_fvs_tuple), + status, + /*replace=*/true); + continue; + } auto *wcmp_group_entry = getWcmpGroupEntry(app_db_entry.wcmp_group_id); if (wcmp_group_entry == nullptr) { @@ -901,9 +934,7 @@ std::string WcmpManager::verifyStateCache(const P4WcmpGroupEntry &app_db_entry, << QuotedVar(wcmp_group_entry->wcmp_group_members[i]->wcmp_group_id) << " in wcmp manager."; return msg.str(); } - // Group member might not be created if it is a watch port. - if (!app_db_entry.wcmp_group_members[i]->watch_port.empty() && - wcmp_group_entry->wcmp_group_members[i]->member_oid == SAI_NULL_OBJECT_ID) + if (!app_db_entry.wcmp_group_members[i]->watch_port.empty() && wcmp_group_entry->wcmp_group_members[i]->pruned) { continue; } @@ -944,8 +975,7 @@ std::string WcmpManager::verifyStateAsicDb(const P4WcmpGroupEntry *wcmp_group_en for (const auto &member : wcmp_group_entry->wcmp_group_members) { - // Group member might not be created if it is a watch port. - if (!member->watch_port.empty() && member->member_oid == SAI_NULL_OBJECT_ID) + if (!member->watch_port.empty() && member->pruned) { continue; } diff --git a/orchagent/p4orch/wcmp_manager.h b/orchagent/p4orch/wcmp_manager.h index d1a6e025bc32..abf5a091f19a 100644 --- a/orchagent/p4orch/wcmp_manager.h +++ b/orchagent/p4orch/wcmp_manager.h @@ -4,6 +4,7 @@ #include #include +#include "bulker.h" #include "notificationconsumer.h" #include "orch.h" #include "p4orch/object_manager_interface.h" @@ -28,6 +29,7 @@ struct P4WcmpGroupMemberEntry // Default ECMP(weight=1) int weight = 1; std::string watch_port; + bool pruned; sai_object_id_t member_oid = SAI_NULL_OBJECT_ID; std::string wcmp_group_id; }; @@ -63,15 +65,7 @@ struct P4WcmpGroupEntry class WcmpManager : public ObjectManagerInterface { public: - WcmpManager(P4OidMapper *p4oidMapper, ResponsePublisherInterface *publisher) - { - SWSS_LOG_ENTER(); - - assert(p4oidMapper != nullptr); - m_p4OidMapper = p4oidMapper; - assert(publisher != nullptr); - m_publisher = publisher; - } + WcmpManager(P4OidMapper *p4oidMapper, ResponsePublisherInterface *publisher); virtual ~WcmpManager() = default; @@ -112,20 +106,18 @@ class WcmpManager : public ObjectManagerInterface ReturnCode createWcmpGroupMember(std::shared_ptr wcmp_group_member, const sai_object_id_t group_oid, const std::string &wcmp_group_key); - // Creates WCMP group member with an associated watch_port. - ReturnCode createWcmpGroupMemberWithWatchport(P4WcmpGroupEntry *wcmp_group, - std::shared_ptr member, - const std::string &wcmp_group_key); - // Performs watchport related addition operations and creates WCMP group - // member. - ReturnCode processWcmpGroupMemberAddition(std::shared_ptr member, - P4WcmpGroupEntry *wcmp_group, const std::string &wcmp_group_key); + // members. + ReturnCode processWcmpGroupMembersAddition( + const std::vector> &members, const std::string &wcmp_group_key, + sai_object_id_t wcmp_group_oid, + std::vector> &created_wcmp_group_members); // Performs watchport related removal operations and removes WCMP group - // member. - ReturnCode processWcmpGroupMemberRemoval(std::shared_ptr member, - const std::string &wcmp_group_key); + // members. + ReturnCode processWcmpGroupMembersRemoval( + const std::vector> &members, const std::string &wcmp_group_key, + std::vector> &removed_wcmp_group_members); // Processes update operation for a WCMP group entry. ReturnCode processUpdateRequest(P4WcmpGroupEntry *wcmp_group_entry); @@ -172,9 +164,6 @@ class WcmpManager : public ObjectManagerInterface std::unordered_map>> port_name_to_wcmp_group_member_map; - // Set of pruned P4WcmpGroupMemberEntry entries - std::unordered_set> pruned_wcmp_members_set; - // Maps port name to oper-status std::unordered_map port_oper_status_map; @@ -182,6 +171,7 @@ class WcmpManager : public ObjectManagerInterface P4OidMapper *m_p4OidMapper; std::deque m_entries; ResponsePublisherInterface *m_publisher; + ObjectBulker gNextHopGroupMemberBulker; friend class p4orch::test::WcmpManagerTest; }; diff --git a/orchagent/response_publisher_interface.h b/orchagent/response_publisher_interface.h index 92d364a500fa..094238b82607 100644 --- a/orchagent/response_publisher_interface.h +++ b/orchagent/response_publisher_interface.h @@ -5,32 +5,31 @@ #include "return_code.h" #include "table.h" -class ResponsePublisherInterface { - public: - virtual ~ResponsePublisherInterface() = default; +class ResponsePublisherInterface +{ + public: + virtual ~ResponsePublisherInterface() = default; - // Publishes the response status. - // If intent attributes are empty, it is a delete operation. - // What "publish" needs to do is completely up to implementation. - // This API does not include redis DB namespace. So if implementation chooses - // to write to a redis DB, it will need to use a fixed namespace. - // The replace flag indicates the state attributes will replace the old ones. - virtual void publish(const std::string& table, const std::string& key, - const std::vector& intent_attrs, - const ReturnCode& status, - const std::vector& state_attrs, - bool replace = false) = 0; + // Publishes the response status. + // If intent attributes are empty, it is a delete operation. + // What "publish" needs to do is completely up to implementation. + // This API does not include redis DB namespace. So if implementation chooses + // to write to a redis DB, it will need to use a fixed namespace. + // The replace flag indicates the state attributes will replace the old ones. + virtual void publish(const std::string &table, const std::string &key, + const std::vector &intent_attrs, const ReturnCode &status, + const std::vector &state_attrs, bool replace = false) = 0; - // Publishes response status. If response status is OK then also writes the - // intent attributes into the DB. - // The replace flag indicates a replace operation. - virtual void publish(const std::string& table, const std::string& key, - const std::vector& intent_attrs, - const ReturnCode& status, bool replace = false) = 0; + // Publishes response status. If response status is OK then also writes the + // intent attributes into the DB. + // The replace flag indicates a replace operation. + virtual void publish(const std::string &table, const std::string &key, + const std::vector &intent_attrs, const ReturnCode &status, + bool replace = false) = 0; - // Write to DB only. This API does not send notification. - // The replace flag indicates the new attributes will replace the old ones. - virtual void writeToDB(const std::string& table, const std::string& key, - const std::vector& values, - const std::string& op, bool replace = false) = 0; + // Write to DB only. This API does not send notification. + // The replace flag indicates the new attributes will replace the old ones. + virtual void writeToDB(const std::string &table, const std::string &key, + const std::vector &values, const std::string &op, + bool replace = false) = 0; }; diff --git a/tests/p4rt/l3.py b/tests/p4rt/l3.py index 31cd0b3b95c1..915228a9b57a 100644 --- a/tests/p4rt/l3.py +++ b/tests/p4rt/l3.py @@ -95,8 +95,8 @@ class P4RtGreTunnelWrapper(util.DBInterface): DEFAULT_TUNNEL_ID = "tunnel-1" DEFAULT_ROUTER_INTERFACE_ID = "16" DEFAULT_ENCAP_SRC_IP = "1.2.3.4" - DEFAULT_ENCAP_DST_IP = "5.6.7.8" - DEFAULT_ACTION = "mark_for_tunnel_encap" + DEFAULT_ENCAP_DST_IP = "12.0.0.1" + DEFAULT_ACTION = "mark_for_p2p_tunnel_encap" def generate_app_db_key(self, tunnel_id): d = {} @@ -240,7 +240,7 @@ class P4RtNextHopWrapper(util.DBInterface): DEFAULT_IPV6_NEIGHBOR_ID = "fe80::21a:11ff:fe17:5f80" # tunnel nexthop attribute values - TUNNEL_ACTION = "set_tunnel_encap_nexthop" + TUNNEL_ACTION = "set_p2p_tunnel_encap_nexthop" DEFAULT_TUNNEL_ID = "tunnel-1" def generate_app_db_key(self, nexthop_id): @@ -266,12 +266,10 @@ def create_next_hop( else: neighbor_id = neighbor_id or self.DEFAULT_IPV6_NEIGHBOR_ID nexthop_id = nexthop_id or self.DEFAULT_NEXTHOP_ID - attr_list = [ - (util.prepend_param_field(self.NEIGHBOR_ID_FIELD), neighbor_id), - (self.ACTION_FIELD, action), - ] + attr_list = [(self.ACTION_FIELD, action)] if action == self.DEFAULT_ACTION: attr_list.append((util.prepend_param_field(self.RIF_FIELD), router_interface_id)) + attr_list.append((util.prepend_param_field(self.NEIGHBOR_ID_FIELD), neighbor_id)) if tunnel_id != None: attr_list.append((util.prepend_param_field(self.TUNNEL_ID_FIELD), tunnel_id)) nexthop_key = self.generate_app_db_key(nexthop_id) diff --git a/tests/p4rt/test_l3.py b/tests/p4rt/test_l3.py index 2816f3d4faa6..a16c8d3f03a3 100644 --- a/tests/p4rt/test_l3.py +++ b/tests/p4rt/test_l3.py @@ -2731,7 +2731,24 @@ def test_RemovePrunedWcmpGroupMember(self, dvs, testlog): == self._p4rt_wcmp_group_obj.get_original_asic_db_member_entries_count() ) - # Delete the pruned wcmp group member. + # Attempt to delete the next hop. Expect failure as the pruned WCMP + # group member is still referencing it. + self._p4rt_nexthop_obj.remove_app_db_entry(nexthop_key) + + # Verify that the P4RT key to OID count is same as before in Redis DB. + status, fvs = key_to_oid_helper.get_db_info() + assert status == True + assert len(fvs) == len(original_key_oid_info) + count + + # Verify that the next hop still exists in app state db. + (status, fvs) = util.get_key( + self._p4rt_nexthop_obj.appl_state_db, + self._p4rt_nexthop_obj.APP_DB_TBL_NAME, + nexthop_key, + ) + assert status == True + + # Delete the pruned wcmp group member and try again. self._p4rt_wcmp_group_obj.remove_app_db_entry(wcmp_group_key) # Verify that P4RT key to OID count decremented by 1 in Redis DB.