[dhcpmon] Report persistent DHCPv4 relay loss per VLAN by Xichen96 · Pull Request #104 · sonic-net/sonic-dhcpmon

Xichen96 · 2026-07-26T07:09:14Z

Description of PR

Report DHCPv4 relay disparity from independent per-message pending-window
state instead of one shared aggregate health counter.

Dependencies:

[dhcpmon] Validate DHCPv6 relay transformations #101 disables the invalid DHCPv6 same-type disparity signal.
[dhcpmon] Match DHCPv4 receive and transmit counters across windows #103 tracks unmatched DHCPv4 windows per message type.

Work item tracking

Microsoft ADO (number only): 38615655

Type of change

Bug fix

Approach

Remove the shared DHCPv4 aggregate state entry.
Check each monitored message type's pending-window age.
Preserve the existing threshold and event payload.
Publish/log once when any type enters a persistent VLAN-level outage episode.
Keep one VLAN-level report latch while any message type remains pending.
Re-arm only after every pending message flow recovers.
Preserve signed threshold semantics.

Simultaneous or staggered message types produce one VLAN-level event because
the existing event contract has no message-type field.

Verification

F2 hardware with a real post-VLAN/no-TX outage produced one disparity log/event.
The pre-VLAN min_links loss produced no disparity event.

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

Back port request

None. This targets master only.

mssonicbld · 2026-07-26T07:09:21Z

/azp run

azure-pipelines · 2026-07-26T07:09:22Z

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

azure-pipelines · 2026-07-26T07:09:31Z

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

Copilot

Pull request overview

This PR updates DHCPv4 relay-disparity reporting to use per-message-type pending-window state (instead of a single aggregate counter) while also adding counter-state synchronization to safely snapshot/update counters across multiple socket event-loop threads.

Changes:

Add counter-state locking helpers (counter_state_{read,write}_lock) and use them to coordinate packet handling vs. health checks / DB snapshotting.
Implement per-message-type DHCPv4 “untransmitted window” tracking and emit a single VLAN-level disparity event per outage episode.
Limit per-callback packet processing to a fixed maximum to reduce time spent holding the counter-state lock.

Reviewed changes

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

Show a summary per file

File	Description
src/sock_mgr.h	Adds shared counter-state lock primitives and new cache-counter snapshot APIs.
src/sock_mgr.cpp	Implements counter-state locking and snapshot-based DB counter updates.
src/packet_handler.cpp	Acquires counter-state read lock during packet processing and bounds packets per callback.
src/health_check.cpp	Switches DHCPv4 disparity reporting to per-message pending-window state with latching.
src/dhcp_mon.cpp	Wraps health checks, status printing, and DB sync snapshotting with counter-state write locks; resets DHCPv4 health state at init.
src/dhcp_device.h	Exposes API to compute per-message-type untransitted-window ages and to reset DHCPv4 health state.
src/dhcp_device.cpp	Implements per-message-type relay flow state tracking and integrates it into positive-health evaluation.

mssonicbld · 2026-07-26T07:38:16Z

/azp run

azure-pipelines · 2026-07-26T07:38:25Z

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

Copilot

Pull request overview

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

mssonicbld · 2026-07-26T07:43:58Z

/azp run

azure-pipelines · 2026-07-26T07:44:07Z

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

mssonicbld · 2026-07-26T07:47:32Z

/azp run

azure-pipelines · 2026-07-26T07:47:42Z

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

Copilot

Pull request overview

Copilot reviewed 7 out of 7 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() performs a deep copy of the cache counter maps; this must be done while holding counter_state_write_lock, otherwise concurrent packet-handler updates can mutate (and rehash) the unordered_maps during the copy. Please document this requirement in the header to avoid accidental unsafe use by future call sites.

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

mssonicbld · 2026-07-26T07:52:36Z

/azp run

azure-pipelines · 2026-07-26T07:52:46Z

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

Copilot

Pull request overview

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

Comments suppressed due to low confidence (2)

src/health_check.cpp:63

check_relay_disparity() latches DHCPv4 disparity with one global flag, so it only clears when all message types have no pending windows. This doesn’t match the stated approach of clearing each message type’s latch when its matching TX recovers; consider tracking latches per message type and still emitting only one VLAN-level event.

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/sock_mgr.h:141

sock_mgr_copy_cache_counters() and the snapshot-based sock_mgr_update_db_counters() are now part of the counter-state synchronization story, but their comments don’t state the locking contract. Since these functions iterate/copy mutable cache counter structures, document that callers must hold counter_state_write_lock (or take the lock internally) to avoid races with packet handling.

/** Write a previously locked, immutable cache-counter snapshot */
void sock_mgr_update_db_counters(const socket_counters_t &counters_by_socket);

/** Copy cache counters for all sockets; caller must hold counter_state_write_lock */
socket_counters_t sock_mgr_copy_cache_counters();

Copilot

Pull request overview

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

mssonicbld · 2026-07-26T08:06:25Z

/azp run

mssonicbld · 2026-07-26T10:14:03Z

/azp run

azure-pipelines · 2026-07-26T10:14:13Z

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 1 comment.

mssonicbld · 2026-07-27T02:20:31Z

/azp run

azure-pipelines · 2026-07-27T02:20:41Z

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

mssonicbld · 2026-07-27T02:24:43Z

/azp run

azure-pipelines · 2026-07-27T02:24:52Z

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

Copilot

Pull request overview

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

+    [1] = {
        .check_health = check_agg_health_v6,
        .alert = alert_dhcp_relay_disparity,
-        .log = log_agg_error,
+        .log = log_v6_agg_error,
        .count = 0,
    },


Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

src/health_check.cpp:190

The DHCPv6 aggregate check is still configured to publish the same "dhcp-relay-disparity" event (and log) when it becomes UNHEALTHY. This contradicts the PR dependency notes (#101 disables the invalid DHCPv6 same-type RX/TX disparity signal) and can also create duplicate/false disparity events because DHCPv6 relay transforms message types at the boundary.

If DHCPv6 disparity is intentionally disabled, the v6 aggregate check should not alert/log the disparity event.

        .alert = alert_dhcp_relay_disparity,
        .log = log_v6_agg_error,

+/**
+ * @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);


+/** Reset DHCPv4 relay-flow watermarks */
+void dhcp_device_reset_health_state(const std::string &ifname);


Track unmatched receive activity by message type so idle windows and adjacent transmit activity do not create false relay-loss state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 <lukelin0907@gmail.com>

Publish one VLAN-level disparity episode from the independent per-message unmatched receive windows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 <lukelin0907@gmail.com>

Copilot

Pull request overview

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

+        int64_t duration_value = static_cast<int64_t>(report_windows) * window_interval_sec;
+        int duration = static_cast<int>(std::min(
+            duration_value, static_cast<int64_t>(std::numeric_limits<int>::max())));


mssonicbld · 2026-07-28T04:49:59Z

/azp run

azure-pipelines · 2026-07-28T04:50:08Z

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

Xichen96 · 2026-07-28T16:32:33Z

Closing because this reporting PR depends entirely on #103 persistent unmatched-window state, which is being replaced by stateless one-packet-in-flight tolerance.

Copilot AI review requested due to automatic review settings July 26, 2026 07:09

Copilot started reviewing on behalf of Xichen96 July 26, 2026 07:09 View session

Copilot AI reviewed Jul 26, 2026

View reviewed changes

Comment thread src/health_check.cpp Outdated

Comment thread src/dhcp_device.cpp

This was referenced Jul 26, 2026

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

Closed

[dhcpmon] Make relay disparity detection resilient #90

Closed

Copilot AI review requested due to automatic review settings July 26, 2026 07:38

Copilot started reviewing on behalf of Xichen96 July 26, 2026 07:38 View session

Copilot AI reviewed Jul 26, 2026

View reviewed changes

Comment thread src/health_check.cpp

Copilot AI review requested due to automatic review settings July 26, 2026 07:43

Copilot started reviewing on behalf of Xichen96 July 26, 2026 07:44 View session

Copilot AI reviewed Jul 26, 2026

View reviewed changes

Comment thread src/sock_mgr.cpp Outdated

Comment thread src/sock_mgr.h Outdated

Copilot AI review requested due to automatic review settings July 26, 2026 07:48

Copilot started reviewing on behalf of Xichen96 July 26, 2026 07:49 View session

Copilot AI reviewed Jul 26, 2026

View reviewed changes

Comment thread src/health_check.cpp

Copilot AI review requested due to automatic review settings July 26, 2026 07:53

Copilot started reviewing on behalf of Xichen96 July 26, 2026 07:53 View session

Copilot AI reviewed Jul 26, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 26, 2026 08:06

Copilot AI review requested due to automatic review settings July 26, 2026 10:13

Copilot started reviewing on behalf of Xichen96 July 26, 2026 10:14 View session

Copilot AI reviewed Jul 26, 2026

View reviewed changes

Comment thread src/packet_handler.cpp Outdated

Xichen96 changed the title ~~[dhcpmon] Report DHCPv4 disparity per message state~~ [dhcpmon] Report persistent DHCPv4 relay loss per VLAN Jul 27, 2026

Copilot AI review requested due to automatic review settings July 27, 2026 02:20

Xichen96 force-pushed the dev/xichenlin/report-v4-disparity-per-message branch from 70808f7 to 0a65eda Compare July 27, 2026 02:20

Copilot started reviewing on behalf of Xichen96 July 27, 2026 02:20 View session

Xichen96 force-pushed the dev/xichenlin/report-v4-disparity-per-message branch from 0a65eda to bbba74f Compare July 27, 2026 02:24

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Comment thread src/health_check.cpp

Comment on lines +187 to 192

[1] = {

.check_health = check_agg_health_v6,

.alert = alert_dhcp_relay_disparity,

.log = log_agg_error,

.log = log_v6_agg_error,

.count = 0,

},

Copilot AI review requested due to automatic review settings July 27, 2026 02:25

Copilot started reviewing on behalf of Xichen96 July 27, 2026 02:26 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Xichen96 added 2 commits July 28, 2026 14:36

Copilot AI review requested due to automatic review settings July 28, 2026 04:36

Xichen96 force-pushed the dev/xichenlin/report-v4-disparity-per-message branch from bbba74f to 9fdb23c Compare July 28, 2026 04:36

Copilot started reviewing on behalf of Xichen96 July 28, 2026 04:37 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

Comment thread src/health_check.cpp

Comment on lines +78 to +80

int64_t duration_value = static_cast<int64_t>(report_windows) * window_interval_sec;

int duration = static_cast<int>(std::min(

duration_value, static_cast<int64_t>(std::numeric_limits<int>::max())));

Xichen96 closed this Jul 28, 2026

Xichen96 deleted the dev/xichenlin/report-v4-disparity-per-message branch July 28, 2026 16:32

Xichen96 mentioned this pull request Jul 28, 2026

[dhcpmon] Tolerate one packet across health windows #109

Draft

1 task

		/** Reset DHCPv4 relay-flow watermarks */
		void dhcp_device_reset_health_state(const std::string &ifname);

Uh oh!

Conversation

Xichen96 commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Work item tracking

Type of change

Approach

Verification

Back port request

Uh oh!

mssonicbld commented Jul 26, 2026

Uh oh!

azure-pipelines Bot commented Jul 26, 2026

Uh oh!

azure-pipelines Bot commented Jul 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Jul 26, 2026

Uh oh!

azure-pipelines Bot commented Jul 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

mssonicbld commented Jul 26, 2026

Uh oh!

azure-pipelines Bot commented Jul 26, 2026

Uh oh!

mssonicbld commented Jul 26, 2026

Uh oh!

azure-pipelines Bot commented Jul 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Jul 26, 2026

Uh oh!

azure-pipelines Bot commented Jul 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

mssonicbld commented Jul 26, 2026

Uh oh!

mssonicbld commented Jul 26, 2026

Uh oh!

azure-pipelines Bot commented Jul 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

mssonicbld commented Jul 27, 2026

Uh oh!

azure-pipelines Bot commented Jul 27, 2026

Uh oh!

mssonicbld commented Jul 27, 2026

Uh oh!

azure-pipelines Bot commented Jul 27, 2026

Uh oh!

Copilot AI left a comment

Xichen96 commented Jul 26, 2026 •

edited

Loading