diff --git a/orchagent/intfsorch.cpp b/orchagent/intfsorch.cpp index 2921393d7c..54bdf45521 100644 --- a/orchagent/intfsorch.cpp +++ b/orchagent/intfsorch.cpp @@ -76,6 +76,7 @@ IntfsOrch::IntfsOrch(DBConnector *db, vector tableNames, m_vidToRidTable = unique_ptr(new Table(m_asic_db.get(), "VIDTORID")); } + auto intervT = timespec { .tv_sec = UPDATE_MAPS_SEC , .tv_nsec = 0 }; m_updateMapsTimer = new SelectableTimer(intervT); auto executorT = new ExecutableTimer(m_updateMapsTimer, this, "UPDATE_MAPS_TIMER"); @@ -436,7 +437,7 @@ bool IntfsOrch::setIntfLoopbackAction(const Port &port, string actionStr) if (!getSaiLoopbackAction(actionStr, action)) { - return false; + return true; } attr.id = SAI_ROUTER_INTERFACE_ATTR_LOOPBACK_PACKET_ACTION; @@ -1066,7 +1067,11 @@ void IntfsOrch::doTask(Consumer &consumer) /* Set loopback action */ if (!loopbackAction.empty()) { - setIntfLoopbackAction(port, loopbackAction); + if (!setIntfLoopbackAction(port, loopbackAction)) + { + it++; + continue; + } } } } @@ -1975,4 +1980,3 @@ void IntfsOrch::voqSyncIntfState(string &alias, bool isUp) } } - diff --git a/tests/mock_tests/intfsorch_ut.cpp b/tests/mock_tests/intfsorch_ut.cpp index 5e50e73183..93970e4fb2 100644 --- a/tests/mock_tests/intfsorch_ut.cpp +++ b/tests/mock_tests/intfsorch_ut.cpp @@ -16,6 +16,10 @@ namespace intfsorch_test int create_rif_count = 0; int remove_rif_count = 0; + bool saw_loopback_action = false; + bool fail_next_rif_set = false; + int loopback_action_set_count = 0; + sai_packet_action_t last_loopback_action = SAI_PACKET_ACTION_FORWARD; sai_router_interface_api_t *pold_sai_rif_api; sai_router_interface_api_t ut_sai_rif_api; @@ -26,14 +30,35 @@ namespace intfsorch_test _In_ const sai_attribute_t *attr_list) { ++create_rif_count; - return SAI_STATUS_SUCCESS; + return pold_sai_rif_api->create_router_interface( + router_interface_id, switch_id, attr_count, attr_list); } sai_status_t _ut_remove_router_interface( _In_ sai_object_id_t router_interface_id) { ++remove_rif_count; - return SAI_STATUS_SUCCESS; + return pold_sai_rif_api->remove_router_interface(router_interface_id); + } + + sai_status_t _ut_set_router_interface_attribute( + _In_ sai_object_id_t router_interface_id, + _In_ const sai_attribute_t *attr) + { + if (attr->id == SAI_ROUTER_INTERFACE_ATTR_LOOPBACK_PACKET_ACTION) + { + ++loopback_action_set_count; + last_loopback_action = static_cast(attr->value.s32); + if (fail_next_rif_set) + { + fail_next_rif_set = false; + return SAI_STATUS_INSUFFICIENT_RESOURCES; + } + saw_loopback_action = true; + return SAI_STATUS_SUCCESS; + } + return pold_sai_rif_api->set_router_interface_attribute( + router_interface_id, attr); } struct IntfsOrchTest : public ::testing::Test @@ -61,6 +86,11 @@ namespace intfsorch_test sai_router_intfs_api->create_router_interface = _ut_create_router_interface; sai_router_intfs_api->remove_router_interface = _ut_remove_router_interface; + sai_router_intfs_api->set_router_interface_attribute = _ut_set_router_interface_attribute; + saw_loopback_action = false; + fail_next_rif_set = false; + loopback_action_set_count = 0; + last_loopback_action = SAI_PACKET_ACTION_FORWARD; m_app_db = make_shared("APPL_DB", 0); m_config_db = make_shared("CONFIG_DB", 0); @@ -489,4 +519,61 @@ namespace intfsorch_test ASSERT_EQ(syncd["Loopback6"].vrf_id, gVrfOrch->getVRFid("Vrf-Blue")); ASSERT_EQ(gVrfOrch->getVrfRefCount("Vrf-Blue"), base_vrf_ref + 1); } + + TEST_F(IntfsOrchTest, IntfsOrchRetriesLoopbackActionSetFailure) + { + std::deque entries{ + {"Ethernet0", "SET", {{"mtu", "9100"}}} + }; + auto consumer = dynamic_cast(gIntfsOrch->getExecutor(APP_INTF_TABLE_NAME)); + ASSERT_NE(consumer, nullptr); + consumer->addToSync(entries); + static_cast(gIntfsOrch)->doTask(); + + fail_next_rif_set = true; + entries = { + {"Ethernet0", "SET", {{"loopback_action", "drop"}}} + }; + consumer->addToSync(entries); + static_cast(gIntfsOrch)->doTask(); + + ASSERT_EQ(consumer->m_toSync.size(), 1u); + ASSERT_EQ(loopback_action_set_count, 1); + ASSERT_FALSE(saw_loopback_action); + + static_cast(gIntfsOrch)->doTask(); + + ASSERT_TRUE(consumer->m_toSync.empty()); + ASSERT_EQ(loopback_action_set_count, 2); + ASSERT_TRUE(saw_loopback_action); + ASSERT_EQ(last_loopback_action, SAI_PACKET_ACTION_DROP); + } + + TEST_F(IntfsOrchTest, IntfsOrchIgnoresInvalidLoopbackActionField) + { + std::deque entries{ + {"Ethernet0", "SET", {{"mtu", "9100"}}} + }; + auto consumer = dynamic_cast(gIntfsOrch->getExecutor(APP_INTF_TABLE_NAME)); + ASSERT_NE(consumer, nullptr); + consumer->addToSync(entries); + static_cast(gIntfsOrch)->doTask(); + + entries = { + {"Ethernet0", "SET", { + {"loopback_action", "invalid"}, + {"nat_zone", "7"} + }} + }; + consumer->addToSync(entries); + static_cast(gIntfsOrch)->doTask(); + + ASSERT_TRUE(consumer->m_toSync.empty()); + ASSERT_EQ(loopback_action_set_count, 0); + ASSERT_FALSE(saw_loopback_action); + + Port port; + ASSERT_TRUE(gPortsOrch->getPort("Ethernet0", port)); + ASSERT_EQ(port.m_nat_zone_id, 7u); + } }