[dhcpmon] Reset relay-loss tracking when counters are cleared

[Xichen96]

Description of PR

Reset relay-disparity watermarks and report latches atomically after counter
clear replaces all cache counters.

Dependencies:

• [dhcpmon] Emit health alerts once per outage #102 generic health alert latching.
• [dhcpmon] Match DHCPv4 receive and transmit counters across windows #103 unmatched DHCPv4 window tracking.
• [dhcpmon] Report persistent DHCPv4 relay loss per VLAN #104 per-message disparity reporting.

Work item tracking

• Microsoft ADO (number only): 38615655

Type of change

• Bug fix

Approach

• Seed relay-flow watermarks immediately after counter initialization.
• Capture the v4 RX and TX root baselines as each cleared cache is applied.
• When the final cache-update callback completes, reset report state and flow
  watermarks from those saved baselines while the writer lock is still held.
• Serialize health checks with counter-clear callbacks through the existing
  DB-sync mutex.
• Clear generic and per-message episode latches together.
• Accept one recovery trigger per clear transaction and reject stale callbacks.

Using per-socket saved baselines keeps packets arriving between cache updates
above the new watermarks instead of absorbing them into the reset baseline.

Verification

F2 hardware counter clear during traffic and membership churn preserved the
dhcpmon PID and produced no false disparity event.

Counter-clear callbacks and health sampling use the existing DB-sync
serialization; no global packet-counter lock is introduced.

Fresh exact-head Azure PR CI is pending. No local compilation is used.

Back port request

None. This targets master only.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

This PR improves the correctness of DHCP relay disparity monitoring across counter-clear operations by atomically resetting disparity watermarks and report latches while counter-state updates are serialized. It introduces a counter-state locking scheme to prevent packet callbacks and health evaluation from observing partially-updated counter state, and adds per-message DHCPv4 disparity window tracking with episode-style reporting behavior.

Changes:

• Add counter-state read/write locks to serialize packet callbacks, counter snapshotting, and health checks around counter clears.
• Snapshot cache counters and pass snapshots into DB updates to avoid races while writing counters out.
• Add/reset DHCPv4 relay-flow per-message “untransmitted window” tracking and clear related report latches after counter replacement.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/sock_mgr.h	Adds counter-state lock primitives and APIs for copying cache counters / updating DB from a snapshot.
src/sock_mgr.cpp	Implements counter-state locks and introduces snapshot-based DB update flow for cache counters.
src/packet_handler.cpp	Serializes packet processing against counter-state updates and bounds per-callback packet processing.
src/health_check.h	Extends health state with a per-episode reported latch and adds a reset API.
src/health_check.cpp	Adds relay disparity checking/latching and resets latches + flow watermarks on counter replacement.
src/dhcp_mon.cpp	Wraps key callbacks with counter-state write locking; resets health/disparity state after counter clear.
src/dhcp_device.h	Adds APIs for per-message unmatched-window tracking and resetting relay-flow state.
src/dhcp_device.cpp	Implements per-message relay-flow state tracking and reset-on-clear behavior for DHCPv4 disparity.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

src/health_check.cpp:65

• Current logic can publish multiple identical v4 disparity events in one sustained episode (one per message type the first time it exceeds the threshold). If the intent is “once per VLAN-level unhealthy episode” (since the event has no message-type field), latch/report based on the max pending-window across all monitored types and clear the latch only when all types are healthy.

static void check_relay_disparity()
{
    auto windows_by_type = dhcp_device_get_untransmitted_windows(agg_dev_all);
    uint32_t report_windows = 0;
    bool has_pending = false;

src/health_check.cpp:273

• If v4 disparity is latched at the episode level, the reset path should clear that latch together with the other health-report state so post-clear traffic cannot be absorbed into a later baseline.

void reset_dhcp_relay_health_state(const std::string &ifname)
{
    std::lock_guard<std::mutex> lock(health_state_mutex);
    reported_disparity_v4 = false;
    for (auto &state : state_data) {

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

src/sock_mgr.h:142

• sock_mgr_copy_cache_counters() reads info.all_counters while packet threads may be mutating those maps. It should only be called while holding counter_state_write_lock (or otherwise guaranteeing packet callbacks are quiesced); documenting that requirement here helps prevent accidental data races.

socket_counters_t sock_mgr_copy_cache_counters();

src/sock_mgr.cpp:755

• The no-arg sock_mgr_update_db_counters() wrapper takes a snapshot without holding the counter-state write lock, which would be a data race if it were ever called concurrently with packet callbacks. Removing this wrapper forces callers to take an explicit snapshot under the proper lock before updating DB state.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

src/health_check.h:43

• reset_dhcp_relay_health_state() can be called with or without explicit baselines; the no-baseline overload reads current socket counters via dhcp_device_reset_health_state(). To prevent unsafe future call sites, the header should document the expected calling context (startup before socket threads, or during the counter-clear replacement transaction).

/** Reset relay state after counter replacement */

src/dhcp_device.h:251

• dhcp_device_reset_health_state() seeds internal relay-flow watermarks from counters (either current socket counters or provided baselines). The header should document when it’s safe to call the no-baseline overload (e.g., during startup or as part of an atomic counter-replacement transaction), to avoid accidental use while counters are actively changing.

/** Reset DHCPv4 relay-flow watermarks */

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/dhcp_mon.cpp:239

• If SIGUSR1 triggers counter-clear before the first periodic DB update runs, last_update_time is still default. In that case db_update_callback() will not recognize the clear-in-progress state (it gates on last_update_time != default_time_point) and can still call sock_mgr_update_db_counters(), defeating the intent to stop writing COUNTERS_DB during a clear transaction. Starting the clear timeout window when pausing also makes the timeout semantics more accurate.

        sock_mgr_pause_write_cache_to_db();
        clear_rx_baseline_valid = false;
        clear_tx_baseline_valid = false;
        clear_update_triggered = false;

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

src/dhcp_device.h:249

• The new dhcp_device_get_untransmitted_windows() API updates internal relay-flow watermark state (see relay_flow_states usage in dhcp_device.cpp) but the header comment doesn’t document the required serialization/thread-safety contract. Without that, future callers may invoke it concurrently with resets/health checks and introduce data races or incorrect watermark updates. Please document that callers must serialize calls with dhcp_device_reset_health_state() (and any other callers) or add internal synchronization.

/**
 * @brief Update and return unmatched DHCPv4 relay RX age in health windows per message type.
 */
std::unordered_map<int, uint32_t> dhcp_device_get_untransmitted_windows(const std::string &ifname);

src/dhcp_device.h:255

• dhcp_device_reset_health_state() also mutates the internal relay-flow watermark state used by dhcp_device_get_untransmitted_windows(), but the declaration comment doesn’t mention any thread-safety/serialization requirement. Please document that callers must serialize reset calls with window tracking updates to avoid races and inconsistent episode state.

/** Reset DHCPv4 relay-flow watermarks */
void dhcp_device_reset_health_state(const std::string &ifname);
void dhcp_device_reset_health_state(
    const std::string &ifname,
    const std::unordered_map<uint8_t, uint64_t> &rx_counters,

src/health_check.h:47

• reset_dhcp_relay_health_state() resets global report/episode state and per-message relay-flow watermarks (via dhcp_device_reset_health_state), but the header doesn’t state any required serialization/thread-safety guarantees. Since reset is invoked from counter-clear callbacks and the periodic health timer, documenting the expectation that callers serialize reset with check_dhcp_relay_health() will help prevent future races and inconsistent latch behavior.

/** Reset relay state after counter replacement */
void reset_dhcp_relay_health_state(const std::string &ifname);
void reset_dhcp_relay_health_state(
    const std::string &ifname,
    const std::unordered_map<uint8_t, uint64_t> &rx_counters,

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Xichen96]

Closing because this reset integration depends on the #103/#104 persistent watermark and latch model, which is being removed.