diff --git a/tests/aspell.en.pws b/tests/aspell.en.pws index 6ef4d0c9b..c6aec3b49 100644 --- a/tests/aspell.en.pws +++ b/tests/aspell.en.pws @@ -495,4 +495,9 @@ SNR tparam ICMP UDP +NIC +NICs SIGSEGV +oper +sysfs +virtio diff --git a/unittest/vslib/TestSwitchBCM56850.cpp b/unittest/vslib/TestSwitchBCM56850.cpp index fba0e9059..ea891a9a4 100644 --- a/unittest/vslib/TestSwitchBCM56850.cpp +++ b/unittest/vslib/TestSwitchBCM56850.cpp @@ -561,3 +561,206 @@ TEST(SwitchBCM56850, test_port_autoneg_fec_override_support) EXPECT_EQ(attr_capability.set_implemented, false); EXPECT_EQ(attr_capability.get_implemented, false); } + +TEST(SwitchBCM56850, test_refresh_port_oper_speed_configured_speed) +{ + /* + * Test that refresh_port_oper_speed returns the administratively + * configured port speed when m_useConfiguredSpeedAsOperSpeed is true, + * i.e., it propagates SAI_PORT_ATTR_SPEED to SAI_PORT_ATTR_OPER_SPEED + * without consulting sysfs. + */ + + auto sc = std::make_shared(0, ""); + auto signal = std::make_shared(); + auto eventQueue = std::make_shared(signal); + + sc->m_saiSwitchType = SAI_SWITCH_TYPE_NPU; + sc->m_switchType = SAI_VS_SWITCH_TYPE_BCM56850; + sc->m_bootType = SAI_VS_BOOT_TYPE_COLD; + sc->m_useTapDevice = false; + sc->m_laneMap = LaneMap::getDefaultLaneMap(0); + sc->m_eventQueue = eventQueue; + sc->m_useConfiguredSpeedAsOperSpeed = true; + + auto scc = std::make_shared(); + + scc->insert(sc); + + SwitchBCM56850 sw( + 0x2100000000, + std::make_shared(0, scc), + sc); + + sai_attribute_t attr; + + attr.id = SAI_SWITCH_ATTR_INIT_SWITCH; + attr.value.booldata = true; + + ASSERT_EQ(sw.initialize_default_objects(1, &attr), SAI_STATUS_SUCCESS); + + // Get the port list + sai_object_id_t port_list[64]; + + attr.id = SAI_SWITCH_ATTR_PORT_LIST; + attr.value.objlist.count = 64; + attr.value.objlist.list = port_list; + + ASSERT_EQ(sw.get(SAI_OBJECT_TYPE_SWITCH, "oid:0x2100000000", 1, &attr), SAI_STATUS_SUCCESS); + ASSERT_GT(attr.value.objlist.count, (uint32_t)0); + + auto port_id = port_list[0]; + auto sport = sai_serialize_object_id(port_id); + + // Set port oper status to UP so refresh_port_oper_speed uses configured speed path + attr.id = SAI_PORT_ATTR_OPER_STATUS; + attr.value.s32 = SAI_PORT_OPER_STATUS_UP; + ASSERT_EQ(sw.set(SAI_OBJECT_TYPE_PORT, sport, &attr), SAI_STATUS_SUCCESS); + + // Set configured speed to 40000 + attr.id = SAI_PORT_ATTR_SPEED; + attr.value.u32 = 40000; + ASSERT_EQ(sw.set(SAI_OBJECT_TYPE_PORT, sport, &attr), SAI_STATUS_SUCCESS); + + // Call refresh_port_oper_speed — with m_useConfiguredSpeedAsOperSpeed=true, + // it should use configured speed (40000) instead of querying sysfs + ASSERT_EQ(sw.refresh_port_oper_speed(port_id), SAI_STATUS_SUCCESS); + + // Verify oper speed matches configured speed + attr.id = SAI_PORT_ATTR_OPER_SPEED; + ASSERT_EQ(sw.get(SAI_OBJECT_TYPE_PORT, sport, 1, &attr), SAI_STATUS_SUCCESS); + EXPECT_EQ(attr.value.u32, (uint32_t)40000); +} + +TEST(SwitchBCM56850, test_refresh_port_oper_speed_down_port) +{ + /* + * Test that refresh_port_oper_speed returns 0 for ports that are + * operationally down, regardless of configured speed. + */ + + auto sc = std::make_shared(0, ""); + auto signal = std::make_shared(); + auto eventQueue = std::make_shared(signal); + + sc->m_saiSwitchType = SAI_SWITCH_TYPE_NPU; + sc->m_switchType = SAI_VS_SWITCH_TYPE_BCM56850; + sc->m_bootType = SAI_VS_BOOT_TYPE_COLD; + sc->m_useTapDevice = false; + sc->m_laneMap = LaneMap::getDefaultLaneMap(0); + sc->m_eventQueue = eventQueue; + + auto scc = std::make_shared(); + + scc->insert(sc); + + SwitchBCM56850 sw( + 0x2100000000, + std::make_shared(0, scc), + sc); + + sai_attribute_t attr; + + attr.id = SAI_SWITCH_ATTR_INIT_SWITCH; + attr.value.booldata = true; + + ASSERT_EQ(sw.initialize_default_objects(1, &attr), SAI_STATUS_SUCCESS); + + // Get a port + sai_object_id_t port_list[64]; + + attr.id = SAI_SWITCH_ATTR_PORT_LIST; + attr.value.objlist.count = 64; + attr.value.objlist.list = port_list; + + ASSERT_EQ(sw.get(SAI_OBJECT_TYPE_SWITCH, "oid:0x2100000000", 1, &attr), SAI_STATUS_SUCCESS); + ASSERT_GT(attr.value.objlist.count, (uint32_t)0); + + auto port_id = port_list[0]; + auto sport = sai_serialize_object_id(port_id); + + // Port is down by default — refresh should set oper speed to 0 + attr.id = SAI_PORT_ATTR_OPER_STATUS; + attr.value.s32 = SAI_PORT_OPER_STATUS_DOWN; + ASSERT_EQ(sw.set(SAI_OBJECT_TYPE_PORT, sport, &attr), SAI_STATUS_SUCCESS); + + // Set configured speed to 40000 (should be ignored for down ports) + attr.id = SAI_PORT_ATTR_SPEED; + attr.value.u32 = 40000; + ASSERT_EQ(sw.set(SAI_OBJECT_TYPE_PORT, sport, &attr), SAI_STATUS_SUCCESS); + + ASSERT_EQ(sw.refresh_port_oper_speed(port_id), SAI_STATUS_SUCCESS); + + attr.id = SAI_PORT_ATTR_OPER_SPEED; + ASSERT_EQ(sw.get(SAI_OBJECT_TYPE_PORT, sport, 1, &attr), SAI_STATUS_SUCCESS); + EXPECT_EQ(attr.value.u32, (uint32_t)0); +} + +TEST(SwitchBCM56850, test_refresh_port_oper_speed_fallback_no_tap) +{ + /* + * Test that when m_useConfiguredSpeedAsOperSpeed is false and no TAP + * device exists (vs_get_oper_speed fails), refresh_port_oper_speed + * reports 0 instead of a bogus value. + */ + + auto sc = std::make_shared(0, ""); + auto signal = std::make_shared(); + auto eventQueue = std::make_shared(signal); + + sc->m_saiSwitchType = SAI_SWITCH_TYPE_NPU; + sc->m_switchType = SAI_VS_SWITCH_TYPE_BCM56850; + sc->m_bootType = SAI_VS_BOOT_TYPE_COLD; + sc->m_useTapDevice = false; + sc->m_laneMap = LaneMap::getDefaultLaneMap(0); + sc->m_eventQueue = eventQueue; + sc->m_useConfiguredSpeedAsOperSpeed = false; + + auto scc = std::make_shared(); + + scc->insert(sc); + + SwitchBCM56850 sw( + 0x2100000000, + std::make_shared(0, scc), + sc); + + sai_attribute_t attr; + + attr.id = SAI_SWITCH_ATTR_INIT_SWITCH; + attr.value.booldata = true; + + ASSERT_EQ(sw.initialize_default_objects(1, &attr), SAI_STATUS_SUCCESS); + + // Get a port + sai_object_id_t port_list[64]; + + attr.id = SAI_SWITCH_ATTR_PORT_LIST; + attr.value.objlist.count = 64; + attr.value.objlist.list = port_list; + + ASSERT_EQ(sw.get(SAI_OBJECT_TYPE_SWITCH, "oid:0x2100000000", 1, &attr), SAI_STATUS_SUCCESS); + ASSERT_GT(attr.value.objlist.count, (uint32_t)0); + + auto port_id = port_list[0]; + auto sport = sai_serialize_object_id(port_id); + + // Set port to UP + attr.id = SAI_PORT_ATTR_OPER_STATUS; + attr.value.s32 = SAI_PORT_OPER_STATUS_UP; + ASSERT_EQ(sw.set(SAI_OBJECT_TYPE_PORT, sport, &attr), SAI_STATUS_SUCCESS); + + // Set configured speed + attr.id = SAI_PORT_ATTR_SPEED; + attr.value.u32 = 100000; + ASSERT_EQ(sw.set(SAI_OBJECT_TYPE_PORT, sport, &attr), SAI_STATUS_SUCCESS); + + // Without TAP devices, vs_get_oper_speed will fail (no host interface) + // refresh_port_oper_speed should report 0 when sysfs is unavailable + ASSERT_EQ(sw.refresh_port_oper_speed(port_id), SAI_STATUS_SUCCESS); + + // Verify oper speed is 0 (no valid sysfs speed, no fallback to configured) + attr.id = SAI_PORT_ATTR_OPER_SPEED; + ASSERT_EQ(sw.get(SAI_OBJECT_TYPE_PORT, sport, 1, &attr), SAI_STATUS_SUCCESS); + EXPECT_EQ(attr.value.u32, (uint32_t)0); +} diff --git a/unittest/vslib/TestSwitchBCM81724.cpp b/unittest/vslib/TestSwitchBCM81724.cpp index 44aa3268f..b27bc6735 100644 --- a/unittest/vslib/TestSwitchBCM81724.cpp +++ b/unittest/vslib/TestSwitchBCM81724.cpp @@ -147,7 +147,10 @@ TEST(SwitchBCM81724, refresh_read_only) attr.id = SAI_PORT_ATTR_OPER_SPEED; - EXPECT_NE(sw.get(SAI_OBJECT_TYPE_PORT, strPortId, 1, &attr), SAI_STATUS_SUCCESS); + // After clearing hostif_info_map, vs_get_oper_speed() fails (no TAP device), + // but refresh_port_oper_speed() now falls back to configured speed instead + // of returning SAI_STATUS_FAILURE (see SwitchStateBase.cpp fix for virtio NICs) + EXPECT_EQ(sw.get(SAI_OBJECT_TYPE_PORT, strPortId, 1, &attr), SAI_STATUS_SUCCESS); //std::cout << sw.dump_switch_database_for_warm_restart(); } diff --git a/vslib/SwitchStateBase.cpp b/vslib/SwitchStateBase.cpp index 01cddc2aa..592c070dd 100644 --- a/vslib/SwitchStateBase.cpp +++ b/vslib/SwitchStateBase.cpp @@ -2659,7 +2659,10 @@ sai_status_t SwitchStateBase::refresh_port_oper_speed( } else if (!vs_get_oper_speed(port_id, attr.value.u32)) { - return SAI_STATUS_FAILURE; + // Report 0 when sysfs speed is unavailable/invalid + // (e.g. virtio NICs report -1 for speed in sysfs) + + attr.value.u32 = 0; } } diff --git a/vslib/SwitchStateBaseHostif.cpp b/vslib/SwitchStateBaseHostif.cpp index e86d30b05..12e3fc63f 100644 --- a/vslib/SwitchStateBaseHostif.cpp +++ b/vslib/SwitchStateBaseHostif.cpp @@ -906,9 +906,23 @@ bool SwitchStateBase::vs_get_oper_speed( return false; } - ifs >> speed; + int32_t speed_value = 0; + + ifs >> speed_value; ifs.close(); + if (speed_value <= 0) + { + SWSS_LOG_WARN("Invalid speed %d read from %s, will use configured speed", + speed_value, veth_speed_filename.c_str()); + + speed = 0; + + return false; + } + + speed = static_cast(speed_value); + return true; }