[dhcpmon] Match DHCPv4 receive and transmit counters across windows

[Xichen96]

Description of PR

Track unmatched DHCPv4 RX activity per message type with absolute counter
watermarks and adjacent-window TX credit.

Work item tracking

• Microsoft ADO (number only): 38615655

Type of change

• Bug fix

Approach

• Maintain RX/TX watermarks independently for Discover, Offer, Request, and
  Ack.
• Preserve an unmatched RX across idle windows.
• Clear it when matching TX activity appears in the current or credited
  adjacent window.
• Preserve one-window TX-before-RX credit.
• Seed watermarks before the first monitoring window.
• Reset safely when counters decrease or are cleared.
• Keep an idle flow INDETERMINATE, not positively healthy.

This PR changes health-state calculation only. Event/report policy is handled
separately.

Stack

Depends on #101. Review the final commit only:

6041e24 [dhcpmon]: Match DHCPv4 counters across health windows

Verification

Fresh exact-head Azure PR CI is pending. No local compilation is used.
The previously validated behavior avoided the pre-VLAN false positive,
detected a real post-VLAN outage, and handled counter clear during traffic.

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 updates sonic-dhcpmon’s DHCPv4 relay health calculation to track “unmatched” RX activity across monitoring windows (per message type) using absolute counter watermarks and a one-window TX-before-RX credit model, while also tightening counter-state synchronization so health sampling and DB writes operate on coherent snapshots.

Changes:

• Add counter-state read/write locking primitives and use them to serialize health sampling/snapshot/DB-sync operations against packet callbacks.
• Implement DHCPv4 per-message-type unmatched-window tracking (watermarks + adjacent-window TX credit) and update positive health evaluation to return INDETERMINATE for idle flows.
• Write COUNTERS_DB from a stable in-memory snapshot (copy under lock, write outside lock).

Reviewed changes

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

Show a summary per file

File	Description
src/sock_mgr.h	Introduces counter-state lock APIs and snapshot counter copy/update interfaces.
src/sock_mgr.cpp	Implements counter-state locking and snapshot-based DB update plumbing.
src/packet_handler.cpp	Binds packet callback work and wraps packet handling with counter-state read locking.
src/dhcp_mon.cpp	Wraps health checks / cache sync / DB update paths with counter-state write locking and uses snapshot DB updates.
src/dhcp_device.h	Adds APIs for DHCPv4 unmatched-window tracking and health-state reset.
src/dhcp_device.cpp	Implements per-message-type unmatched-window tracking and updates positive health logic accordingly.

[mssonicbld]

/azp run

[azure-pipelines]

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/sock_mgr.h:142

• sock_mgr_copy_cache_counters() reads all live per-socket counter maps and must be called under an exclusive counter-state lock to avoid races with packet callbacks incrementing counters. The header comment currently doesn’t document this requirement, which makes accidental unsafe use more likely.

/** Copy cache counters for all sockets */
socket_counters_t sock_mgr_copy_cache_counters();

[mssonicbld]

/azp run

[azure-pipelines]

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

[Copilot]

Pull request overview

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

[mssonicbld]

/azp run

[azure-pipelines]

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

[Copilot]

Pull request overview

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

[mssonicbld]

/azp run

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/dhcp_device.cpp:120

• The new health algorithm reads sock_info.all_counters directly while packet-handler threads concurrently increment those counters. Since the counters are stored in std::unordered_map<uint8_t, uint64_t>, these read/write operations are not synchronized and constitute undefined behavior (data races and potentially torn 64-bit reads), which can lead to incorrect rx_delta/tx_delta calculations or crashes under load. Consider making counter values atomic or introducing a lock/snapshot mechanism so health evaluation reads a stable view of counters while packet threads update them.

    const sock_info_t &rx_sock_info = sock_mgr_get_sock_info(rx_sock);
    const counter_t &rx_counters = rx_sock_info.all_counters.at(ifname);
    const sock_info_t &tx_sock_info = sock_mgr_get_sock_info(tx_sock);
    const counter_t &tx_counters = tx_sock_info.all_counters.at(ifname);

[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 9 out of 9 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/dhcp_device.cpp:213

• check_counter_not_transmitted() is now unused (only its definition remains). Leaving dead code here makes future refactors harder and can trigger -Wunused-function warnings under common build flags.

Remove the function if it’s no longer needed (or reintroduce a call site if it’s meant to back a health check).

static bool check_counter_not_transmitted(const std::string &ifname, int rx_sock, int tx_sock,
                                          const int *monitored_msgs, size_t monitored_msg_cnt)
{
    const sock_info_t &rx_sock_info = sock_mgr_get_sock_info(rx_sock);
    const counter_t &rx_counters = rx_sock_info.all_counters.at(ifname);
    const counter_t &rx_snapshot = rx_sock_info.all_counters_snapshot.at(ifname);
    const sock_info_t &tx_sock_info = sock_mgr_get_sock_info(tx_sock);
    const counter_t &tx_counters = tx_sock_info.all_counters.at(ifname);
    const counter_t &tx_snapshot = tx_sock_info.all_counters_snapshot.at(ifname);

    for (size_t i = 0; i < monitored_msg_cnt; i++) {
        int msg_type = monitored_msgs[i];
        if (rx_counters.at(msg_type) > rx_snapshot.at(msg_type) &&
            tx_counters.at(msg_type) <= tx_snapshot.at(msg_type)) {
            return true;
        }
    }
    return false;
}

[Xichen96]

Closing because the persistent cross-window watermark model does not match the intended point-in-time health semantics. It will be replaced after the final relay transformation, downstream fan-out, and configured-server ratio rules.