[vslib] Fix VS SAI reporting 0xFFFFFFFF oper speed for virtio NICs

When running on KVM/virtio, /sys/class/net/ethN/speed returns -1
(unknown). vs_get_oper_speed() reads this into uint32_t, which wraps
to 4294967295 (0xFFFFFFFF) and gets reported as the oper speed.

Fix:
- Read sysfs speed as int32_t and check for <= 0 (invalid)
- When invalid, log a warning and return false
- In refresh_port_oper_speed(), fall back to SAI_PORT_ATTR_SPEED
  (configured speed) instead of returning SAI_STATUS_FAILURE

This ensures VS ports show the correct configured speed (e.g. 40G)
rather than garbage values on virtual NICs.

Added unit tests:
- test_refresh_port_oper_speed_configured_speed: verifies oper speed
  equals configured speed when m_useConfiguredSpeedAsOperSpeed=true
- test_refresh_port_oper_speed_down_port: verifies oper speed is 0
  for operationally down ports
- test_refresh_port_oper_speed_fallback_no_tap: verifies fallback to
  configured speed when vs_get_oper_speed fails (no TAP device)

Signed-off-by: Rustiqly <[email protected]>

Author: rustiqly

tests/aspell.en.pws

@@ -490,3 +490,8 @@ IPFix
 ipfix
 SNR
 tparam
+NIC
+NICs
+oper
+sysfs
+virtio

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<SwitchConfig>(0, "");
+    auto signal = std::make_shared<Signal>();
+    auto eventQueue = std::make_shared<EventQueue>(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<SwitchConfigContainer>();
+
+    scc->insert(sc);
+
+    SwitchBCM56850 sw(
+            0x2100000000,
+            std::make_shared<RealObjectIdManager>(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<SwitchConfig>(0, "");
+    auto signal = std::make_shared<Signal>();
+    auto eventQueue = std::make_shared<EventQueue>(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<SwitchConfigContainer>();
+
+    scc->insert(sc);
+
+    SwitchBCM56850 sw(
+            0x2100000000,
+            std::make_shared<RealObjectIdManager>(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
+     * falls back to configured speed instead of failing.
+     */
+
+    auto sc = std::make_shared<SwitchConfig>(0, "");
+    auto signal = std::make_shared<Signal>();
+    auto eventQueue = std::make_shared<EventQueue>(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<SwitchConfigContainer>();
+
+    scc->insert(sc);
+
+    SwitchBCM56850 sw(
+            0x2100000000,
+            std::make_shared<RealObjectIdManager>(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 fall back to configured speed
+    ASSERT_EQ(sw.refresh_port_oper_speed(port_id), SAI_STATUS_SUCCESS);
+
+    // Verify oper speed equals configured speed (fallback)
+    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)100000);
+}

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

vslib/SwitchStateBase.cpp

@@ -2655,7 +2655,11 @@ sai_status_t SwitchStateBase::refresh_port_oper_speed(
         }
         else if (!vs_get_oper_speed(port_id, attr.value.u32))
         {
-            return SAI_STATUS_FAILURE;
+            // Fall back to configured speed when oper speed is unavailable
+            // (e.g. virtio NICs report -1 for speed in sysfs)
+
+            attr.id = SAI_PORT_ATTR_SPEED;
+            CHECK_STATUS(get(SAI_OBJECT_TYPE_PORT, port_id, 1, &attr));
         }
     }
 

vslib/SwitchStateBaseHostif.cpp

@@ -906,9 +906,21 @@ 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());
+
+        return false;
+    }
+
+    speed = static_cast<uint32_t>(speed_value);
+
     return true;
 }