Shrikrishna (Shri) Khare

This commit introduces a **performance enhancement** to the **`fboss2` CLI's command execution and output handling**. By relocating the `executor.join()` call after print operations within `CmdHandler::runHelper()`, the CLI now **streams tabular output as individual host queries complete**, instead of blocking until all parallel operations are finished. This change dramatically improves user experience by providing **faster initial feedback and reducing overall command execution times**, with observed improvements of up to 35x for first output and 40% faster total execution on large-scale device deployments.

This commit introduces a **new feature** to the **`fboss` test runner script**, `run_test.py`, by adding the `--list-tests-for-features` command-line flag. This flag enables users to **filter and list `AgentHwTests`** based on a comma-separated list of production feature tags. The filtering logic ensures that only tests whose *all* tagged features are contained within the supplied list are returned, enhancing the **test management and execution workflow** by allowing more precise targeting of tests relevant to specific feature sets.

This commit implements a **bug fix** within the **test runner script** (`run_test.py`) to prevent redundant warning messages. Previously, the `_get_unsupported_test_regexes()` and `_get_known_bad_test_regexes()` functions were called inside a per-test loop within `_get_tests_to_run`, causing "skip-known-bad-tests" warnings to print for every applicable test. The change now **caches these regex lists once before the loop**, ensuring the warning is displayed only a single time. This significantly **reduces log noise** and improves the clarity of the **test execution output** for users.

This commit **enhances documentation** and **organizes** the **`HwAsic` interface** by grouping five existing **MPLS-related features** within the `HwAsic::Feature` enum in `fboss/agent/hw/switch_asics/HwAsic.h`. It introduces a dedicated section with a conceptual overview and detailed per-feature documentation for `MPLS`, `SAI_MPLS_QOS`, `SAI_MPLS_TTL_1_TRAP`, `SAI_MPLS_LABEL_LOOKUP_FAIL_COUNTER`, and `SAI_MPLS_INSEGMENT`. This **documentation enhancement** and **code organization** effort improves the clarity and discoverability of hardware ASIC capabilities, making it easier for developers to understand and utilize these **MPLS functionalities** without altering any runtime behavior.

This commit introduces **multi-NPU awareness** to packet sending functionalities within the **`MultiHwSwitchHandler`** and **`SwSwitch`** components. Previously, methods like `sendPacketSwitchedSync` and `sendPacketSwitchedAsync` incorrectly targeted only the first NPU on multi-NPU platforms, leading to incorrect behavior. This **feature enhancement** adds an optional `SwitchID` parameter to these methods, enabling callers to explicitly target a specific NPU for packet transmission. Additionally, `sendPacketOutOfPortSyncForPktType` is updated to utilize `ScopeResolver`, aligning its NPU selection logic with asynchronous methods. This change is crucial for **correct packet handling and robust operation on multi-NPU systems**.

This commit **reformats comments** within the `AgentCoppTests.cpp` file, specifically targeting various test cases related to **Control Plane Policing (CoPP) agent hardware tests**. The primary nature of this work is **maintenance** and **code style improvement**, aiming to enhance the **readability** and clarity of the test suite. This change has no functional impact on the `fboss` agent or its tests, but it improves the developer experience when reviewing or modifying these specific hardware tests.

This commit introduces a **robustness improvement** to the **FBOSS agent's packet transmission path** by replacing a critical assertion with a more graceful error handling mechanism. Specifically, within `fboss/agent/MultiHwSwitchHandler.cpp`, the `CHECK_GE` macro in functions like `sendPacketSwitchedSync` and `sendPacketSwitchedAsync` has been replaced with an `if` condition and `XLOG_EVERY_MS`. This change ensures that the system will now log a warning and continue execution instead of crashing if hardware switch syncers are unexpectedly absent during packet transmission. This **maintenance** work prevents hard failures, leading to more stable and resilient packet forwarding operations within the agent.

This commit **fixes a bug** in the **`AgentDscpMarkingTest`** suite, specifically addressing incorrect packet dispatching in **multi-NPU environments**. Previously, the `sendPacket` method would call `sendPacketSwitchedAsync` without a `switchId` for non-frontPanel paths, causing test packets to always be routed to NPU 0. To rectify this, a new helper `sendPacketSwitchedAsync` is introduced in the **`AgentHwTest`** framework, which now correctly passes `getSwitchIdUnderTest()` to ensure packets are sent to the **intended NPU under test**. This **improves the accuracy and reliability** of DSCP marking tests on hardware with multiple NPUs by ensuring proper packet routing.

This commit introduces a **bug fix** within the **`CoppTestUtils`** module to ensure accurate retrieval of CPU port statistics. Previously, functions like `getLatestCpuStats` and `getCpuPortStats` incorrectly used `SwitchID` as a key for `SwSwitch::hwSwitchStats_`, which expects a `switchIndex`, leading to lookup failures on multi-NPU platforms where these identifiers can differ. The change now correctly maps the `SwitchID` to its corresponding `switchIndex` using `getSwitchIndexFromSwitchId()` before accessing the statistics map, thereby **resolving data retrieval issues** in test utilities. This ensures the reliability of CPU port statistics collection in diverse hardware configurations.

This commit **adds a new test case** to `fboss/agent/test/SwSwitchTest.cpp` to verify the robustness of the **`fillFsdbStats()` function** within the **FBOSS agent**. Specifically, it ensures that `fillFsdbStats()` does not crash on **NPU (non-VOQ) switch configurations** where `shelManager_` is null. This **test addition** validates a recent bug fix that introduced a null check, preventing a previously observed crash during **FBOSS agent statistics collection** on these devices. The new test improves the stability of the **FBOSS agent** by confirming the fix for a critical runtime error related to uninitialized pointers.

This commit **fixes a file descriptor leak** within the `touchFile()` function in the **`fboss/lib/CommonFileUtils` library**. Previously, `touchFile()` would call `createFile()` to obtain a file descriptor but would fail to close it, leading to a resource leak on every invocation. This **bug fix** ensures proper resource management by explicitly closing the file descriptor after its use, preventing potential system instability or resource exhaustion caused by accumulated open file descriptors.

This commit provides a **bug fix** to prevent **undefined behavior** within the **`fillFsdbStats` function** in the `fboss/agent` module. It addresses a scenario where `begin()->second` was called on potentially empty `std::map` objects, which is an invalid operation. By adding explicit `empty()` checks before these accesses, the change ensures the **robustness and stability** of the statistics collection process. This prevents potential crashes or unpredictable behavior when populating FSDB statistics from vacant maps.

This commit **enhances test coverage** for the **`Bsp::emergencyShutdown` function** within the `fboss/platform/fan_service` component. It introduces new test cases to verify that calling `emergencyShutdown(true)` with an undefined command correctly throws an `FbossError`, confirming a previous bug fix. Additionally, tests are added to ensure that `emergencyShutdown(false)` behaves idempotently as a no-op. This **test coverage enhancement** **closes a critical test gap** for this emergency code path, improving the reliability of the system's shutdown mechanisms.

This commit **fixes a critical bug** in the **Fan Service** platform's `Bsp.cpp` module, specifically within the `emergencyShutdown()` function. Previously, when an undefined shutdown command was encountered, a `FbossError` was constructed but silently discarded due to a missing `throw` keyword, leading to the system incorrectly reporting a successful emergency shutdown. This **bug fix** ensures that the `FbossError` is now properly propagated, preventing false positives and significantly improving the reliability and error handling of the **emergency shutdown mechanism**. The change ensures that critical failures during an emergency shutdown are no longer silently ignored.

This commit **fixes a latent null dereference** within the `fillFsdbStats()` function of the **FBOSS agent**, preventing crashes on NPU switches. Previously, `fillFsdbStats()` would attempt to access `shelManager_` without a null check when collecting **SHEL feature statistics**, which is only initialized for VOQ ASICs. This **critical bug fix** introduces a null check before calling `ecmpOverShelDisabledPort()`, ensuring the agent's stability and preventing crashes as NPU fleet deployments catch up to newer agent versions. The change specifically impacts the **`SwSwitch` module's FSDB statistics collection** by making it robust to different ASIC types.

This commit introduces a **performance optimization** within the **`SwSwitch`** module by **refactoring** the state observer notification mechanism. Specifically, it modifies the `notifyStateObservers` function to iterate over `stateObservers_` using a `const auto&` reference instead of copying by value. This change prevents approximately 15 unnecessary heap allocations that previously occurred on every state update, significantly improving the efficiency and reducing memory overhead during state transitions. The fix ensures that the state update path is more performant, especially in scenarios with a large number of registered observers.

This commit **refactors** the `handlePacket` method within the **`SwSwitch`** component to **improve both consistency and efficiency**. Previously, `handlePacket` would call `getState()` multiple times, leading to redundant `stateLock_` acquisitions and `shared_ptr` refcount bumps. Now, the switch state is **captured once** at the beginning of the `handlePacket` function, significantly **reducing overhead** during packet processing. This change also **enhances consistency** by ensuring all operations within `handlePacket` use a single, immutable snapshot of the state, preventing potential issues from state updates occurring mid-processing.

This commit **fixes a null pointer dereference** within the `linkAdminStateChangedByFw` function in the **`fboss/agent`** module's `SwSwitch.cpp`. A **null check** has been added for the `port` object to ensure it exists before being dereferenced. This **bug fix** prevents potential crashes or undefined behavior that could occur if a port is not found when its administrative state is updated by firmware. The change improves the robustness of the **network switch agent** by safeguarding against invalid port references during state transitions.

This commit delivers a **bug fix** for the **`SwSwitch` agent** by correcting a copy-paste error within the `accumulateGlobalCpuStats` function. Previously, the **CPU statistics accumulation** logic incorrectly populated `queueDiscardPackets_` with `queueInPackets_` data, leading to silent data corruption. This fix ensures that **CPU queue discard statistics** are accurately collected and reported, specifically addressing an issue that would provide misleading metrics on **multi-NPU switches**. The change prevents incorrect performance monitoring by ensuring the proper counter is incremented for discarded packets.

This commit **optimizes concurrency** within the **FBOSS agent's statistics collection** by correcting the locking mechanism for `hwSwitchStats_` access. Specifically, the `fillFsdbStats()` and `getBufferPoolStatsFromSwitchWatermarkStats()` functions in `fboss/agent/SwSwitch.cpp` were incorrectly acquiring a write-lock (`wlock()`) for read-only operations. This **refactoring** changes these to use a read-lock (`rlock()`), aligning with other callers and **preventing unnecessary blocking** of concurrent readers. The change **improves the responsiveness** and **throughput of statistics gathering** by allowing multiple readers to access hardware switch statistics simultaneously.