[dhcpmon] Track PortChannels under downstream VLANs by Xichen96 · Pull Request #89 · sonic-net/sonic-dhcpmon

Xichen96 · 2026-07-25T16:52:54Z

Description of PR

Summary:

Fungible F2 uses PortChannels as members of the downstream VLAN. dhcpmon
currently records PortChannel1005 -> Vlan1000, but skips
Ethernet312 -> PortChannel1005 because the PortChannel is not a command-line
context interface. A packet delivered on the bare physical member during LAG
convergence is therefore not attributed through the full downstream hierarchy.

This change recognizes physical members of a PortChannel that belongs to the
monitored downstream VLAN on single-ToR systems. It also aggregates each
interface into its immediate parent:

Ethernet -> PortChannel -> VLAN -> root aggregate

Existing direct VLAN-member, upstream PortChannel, and Dual-ToR attribution
behavior is unchanged.

Dependencies:

This draft will be rebased after [dhcp_device] Skip address initialization for management interface #87
and [dhcpmon] Clean up source include dependencies #88 merge to avoid overlapping
source-file churn.
Nested counter display classification is tracked in a separate
sonic-buildimage PR.

Work item tracking

Microsoft ADO (number only): 38615656

Type of change

Approach

What is the motivation for this PR?

The existing one-level mapping cannot resolve a physical packet ingress through
a PortChannel that is itself a member of the downstream VLAN. It also collapses
aggregate counters directly into the final context, so the PortChannel-to-VLAN
edge cannot be distinguished from the physical-to-PortChannel edge.

How did you do it?

Build VLAN mappings before PortChannel-member mappings.
Track a PortChannel's physical members when the PortChannel is either a
command-line context or a member of the monitored VLAN.
Preserve existing Dual-ToR behavior because nested physical-member
attribution requires separate MUX-aware handling.
Resolve packet-validation context recursively through
Ethernet -> PortChannel -> VLAN.
Select aggregate counters from the immediate parent instead of the final
command-line context.
Use the same immediate-parent helper for live packet increments and aggregate
recalculation after counter synchronization.
Preserve existing COUNTERS_DB key/field shapes and the top-level
Agg-<Vlan> debug output.

How did you verify/test it?

Pending this PR's Azure CI and master hardware validation.

Planned hardware validation:

Traditional physical VLAN-member topology: existing DHCPv4, DHCPv6, and
dhcpmon counter tests.
Fungible F2 single-ToR topology:
Ethernet312/314 -> PortChannel1005 -> Vlan1000.
Steady state, member flap, PortChannel flap, and bare-member convergence
counter attribution.

Any platform specific information?

The new nested physical-member attribution is limited to single-ToR. Existing
Dual-ToR counting remains unchanged because its downstream RX counters are
derived from MUX state.

Back port request

None. This repair targets master only.

Tested branch

Pending.

Test result

Pending this PR's Azure CI and master hardware validation.

Documentation

Not applicable; existing counter schemas and command output fields are
unchanged.

Resolve nested Ethernet-to-PortChannel-to-VLAN membership and aggregate counters at each immediate parent without changing existing Dual-ToR attribution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 <lukelin0907@gmail.com>

mssonicbld · 2026-07-25T16:53:01Z

/azp run

azure-pipelines · 2026-07-25T16:53:02Z

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

azure-pipelines · 2026-07-25T16:53:10Z

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

Copilot

Pull request overview

This PR updates dhcpmon’s interface attribution model to support nested downstream hierarchies where a monitored VLAN contains a PortChannel, and the PortChannel contains physical members. The goal is to correctly classify/attribute packets that may arrive on a bare physical member during LAG convergence, and to expose intermediate aggregation edges.

Changes:

Build VLAN mappings before PortChannel-member mappings so PortChannels that are VLAN members can have their physical members tracked.
Add device-manager helpers to resolve an interface’s immediate parent and the aggregate-counter name for that parent.
Switch live counter increments and aggregate-counter recomputation to use the new immediate-parent aggregate selection helper.

Reviewed changes

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

Show a summary per file

File	Description
src/util.h	Removes the previous inline aggregate-counter helper in favor of device-manager helpers.
src/packet_handler.cpp	Updates live per-packet aggregate-counter selection to use device-manager aggregation logic.
src/dhcp_mon.cpp	Updates aggregate-counter recomputation to use device-manager aggregation logic.
src/dhcp_devman.h	Adds public APIs for parent resolution and aggregate-counter name derivation.
src/dhcp_devman.cpp	Implements nested PortChannel-member mapping for downstream VLANs and adds parent/aggregate helper implementations.

Copilot

Pull request overview

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

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 <lukelin0907@gmail.com>

mssonicbld · 2026-07-25T23:04:36Z

/azp run

azure-pipelines · 2026-07-25T23:04:45Z

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 no new comments.

Copilot

Pull request overview

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

Treat a self-mapped VLAN or PortChannel member as a root interface instead of creating a self aggregate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 <lukelin0907@gmail.com>

mssonicbld · 2026-07-26T01:03:30Z

/azp run

azure-pipelines · 2026-07-26T01:03:39Z

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 no new comments.

Include family, direction, parent and child-aggregate deltas, expected ratio, and message type in persistent hierarchy mismatch logs for precise loss localization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 <lukelin0907@gmail.com>

mssonicbld · 2026-07-26T05:17:28Z

/azp run

azure-pipelines · 2026-07-26T05:17:38Z

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 no new comments.

Evaluate both an interface and its immediate-child aggregate before declaring a quiet window, so child-only packet observations expose the exact forwarding edge that did not advance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 <lukelin0907@gmail.com>

mssonicbld · 2026-07-26T06:07:48Z

/azp run

azure-pipelines · 2026-07-26T06:07:57Z

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 (1)

src/dhcp_device.cpp:240

check_aggregate_health() also takes uint8_t ratio, so the member-count expression passed by dhcp_device_check_agg_multiple_*() will truncate before the comparison even runs. Update this parameter to a wider type to match the (widened) check_counters_delta_expected() ratio.

static dhcp_mon_status_t check_aggregate_health(const std::string &ifname, int sock, uint8_t ratio,
                                                const int *monitored_msgs, size_t monitored_msg_cnt)

        uint64_t delta = counters.at(monitored_msgs[i]) - counters_snapshot.at(monitored_msgs[i]);
        uint64_t other_delta = other_counters.at(monitored_msgs[i]) - other_counters_snapshot.at(monitored_msgs[i]);
        if (delta * ratio != other_delta) {
+            const std::string *message_names = sock_info.is_v6 ? db_counter_name_v6 : db_counter_name;
+            last_counter_mismatch =


Xichen96 · 2026-07-26T06:42:42Z

Closing after informal scope review. Replaced by smaller PRs: #99 handles nested PortChannel hierarchy and immediate-parent aggregation; #100 adds exact interface-edge mismatch diagnostics.

Copilot AI review requested due to automatic review settings July 25, 2026 16:52

Copilot started reviewing on behalf of Xichen96 July 25, 2026 16:53 View session

Copilot AI reviewed Jul 25, 2026

View reviewed changes

Comment thread src/packet_handler.cpp

Comment thread src/dhcp_mon.cpp

Comment thread src/packet_handler.cpp Outdated

This was referenced Jul 25, 2026

[dhcpmon] Classify nested VLAN members as downlinks sonic-net/sonic-buildimage#28625

Closed

[dhcpmon] Make relay disparity detection resilient #90

Closed

[TEMP][dhcpmon] Validate Fungible F2 repair stack #94

Closed

Xichen96 requested a review from Copilot July 25, 2026 22:57

Copilot started reviewing on behalf of Xichen96 July 25, 2026 22:57 View session

Copilot AI reviewed Jul 25, 2026

View reviewed changes

[dhcpmon]: Fix aggregate comment typo

9f467e5

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 AI review requested due to automatic review settings July 25, 2026 23:04

Copilot started reviewing on behalf of Xichen96 July 25, 2026 23:04 View session

Copilot AI reviewed Jul 25, 2026

View reviewed changes

Xichen96 requested a review from Copilot July 25, 2026 23:42

Copilot started reviewing on behalf of Xichen96 July 25, 2026 23:43 View session

Copilot AI reviewed Jul 25, 2026

View reviewed changes

Comment thread src/dhcp_devman.cpp

Copilot AI review requested due to automatic review settings July 26, 2026 01:03

Copilot started reviewing on behalf of Xichen96 July 26, 2026 01:03 View session

Copilot AI reviewed Jul 26, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 26, 2026 05:17

Copilot started reviewing on behalf of Xichen96 July 26, 2026 05:17 View session

Copilot AI reviewed Jul 26, 2026

View reviewed changes

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

Copilot started reviewing on behalf of Xichen96 July 26, 2026 06:08 View session

Copilot AI reviewed Jul 26, 2026

View reviewed changes

Xichen96 closed this Jul 26, 2026

Xichen96 deleted the dev/xichenlin/fix-portchannel-vlan-hierarchy branch July 26, 2026 06:43

Uh oh!

Conversation

Xichen96 commented Jul 25, 2026

Description of PR

Work item tracking

Type of change

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Back port request

Tested branch

Test result

Documentation

Uh oh!

mssonicbld commented Jul 25, 2026

Uh oh!

azure-pipelines Bot commented Jul 25, 2026

Uh oh!

azure-pipelines Bot commented Jul 25, 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!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

mssonicbld commented Jul 25, 2026

Uh oh!

azure-pipelines Bot commented Jul 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

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!

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!

Xichen96 commented Jul 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!