[dhcpmon] Track physical members beneath VLAN PortChannels by Xichen96 · Pull Request #99 · sonic-net/sonic-dhcpmon

Xichen96 · 2026-07-26T06:40:33Z

Description of PR

Track a physical member through its VLAN-backed PortChannel:

Ethernet312 -> PortChannel1005 -> Vlan1000

The current implementation records PortChannel1005 -> Vlan1000, but skips
the physical-member relation when the PortChannel is not itself a command-line
context interface.

Work item tracking

Microsoft ADO (number only): 38615656

Type of change

Approach

What is the motivation for this PR?

Fungible F2 places PortChannels beneath the downstream VLAN. During LACP
convergence the kernel can deliver the packet on a bare physical member, which
must still resolve to the monitored VLAN context.

How did you do it?

Keep VLAN and PortChannel membership maps from CONFIG_DB.
Resolve physical member -> PortChannel -> VLAN recursively.
Increment only the immediate-parent aggregate at each observation point.
Preserve direct VLAN-member and DualToR behavior.
Ignore accidental self-referential mappings.

Immediate-parent aggregation is intentional: rolling a pre-VLAN physical or
PortChannel observation all the way into the root VLAN aggregate would recreate
the false relay-disparity signal from the F2 RCA.

How did you verify/test it?

Azure PR CI will validate the restacked head on amd64, arm64, and armhf. No
local compilation is used. Prior regular and F2 hardware validation covered
steady state, bare-member convergence, and PortChannel recreation with a
changed ifindex.

Any platform specific information?

No.

Back port request

None. This targets master only.

Tested branch

master

Test result

Direct and PortChannel-backed VLAN traffic counted correctly. Member, VLAN
membership, and full PortChannel recreation were handled with an unchanged
dhcpmon PID; the recreated PortChannel ifindex changed from 1004 to 1005.

Documentation

Not applicable.

Stack

Depends on #96. Review the final commit only:

1331563 [dhcpmon]: Track physical members beneath VLAN PortChannels

mssonicbld · 2026-07-26T06:40:41Z

/azp run

azure-pipelines · 2026-07-26T06:40:42Z

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

azure-pipelines · 2026-07-26T06:40:50Z

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

Copilot

Pull request overview

This PR updates sonic-dhcpmon’s interface relationship tracking so packets observed on a physical PortChannel member can be attributed through a nested hierarchy (e.g., Ethernet312 -> PortChannel1005 -> Vlan1000), enabling correct downstream VLAN-context monitoring during LACP convergence and PortChannel recreation scenarios.

Changes:

Extend device-context resolution to follow PortChannel membership recursively before VLAN membership.
Introduce “immediate parent” attribution helpers (dhcp_devman_get_parent_ifname, dhcp_devman_get_agg_counter_ifname) and migrate aggregate counter updates to use them.
Adjust PortChannel member mapping rules to allow nested PortChannels under monitored VLANs (with Dual-ToR caveats).

Reviewed changes

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

Show a summary per file

File	Description
src/util.h	Removes the old context-parameter aggregate counter helper (now handled by devman).
src/packet_handler.cpp	Switches per-packet aggregate counting to immediate-parent attribution; fixes a comment typo.
src/dhcp_mon.cpp	Switches aggregate counter recalculation to immediate-parent attribution.
src/dhcp_devman.h	Adds new APIs for parent resolution and aggregate counter naming.
src/dhcp_devman.cpp	Implements recursive context resolution and the new immediate-parent attribution helpers; updates PortChannel mapping rules.

mssonicbld · 2026-07-26T07:26:56Z

/azp run

azure-pipelines · 2026-07-26T07:27:06Z

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.

mssonicbld · 2026-07-27T03:26:47Z

/azp run

azure-pipelines · 2026-07-27T03:26:56Z

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.

mssonicbld · 2026-07-27T03:31:00Z

/azp run

azure-pipelines · 2026-07-27T03:31:09Z

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.

mssonicbld · 2026-07-27T03:57:44Z

/azp run

azure-pipelines · 2026-07-27T03:57:53Z

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

mssonicbld · 2026-07-27T04:00:57Z

/azp run

azure-pipelines · 2026-07-27T04:01:08Z

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

mssonicbld · 2026-07-27T17:06:21Z

/azp run

azure-pipelines · 2026-07-27T17:06:32Z

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.

mssonicbld · 2026-07-28T02:08:12Z

/azp run

azure-pipelines · 2026-07-28T02:08:21Z

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.

mssonicbld · 2026-07-28T05:58:22Z

/azp run

azure-pipelines · 2026-07-28T05:58:32Z

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.

Comments suppressed due to low confidence (1)

src/dhcp_devman.cpp:240

Self-referential PORTCHANNEL_MEMBER entries (e.g., PORTCHANNEL_MEMBER|PortChannelX|PortChannelX) are still inserted into portchan_map/rev_portchan_map. While dhcp_devman_get_parent_ifname() ignores direct self-maps, keeping the self-member in rev_portchan_map can skew rev_portchan_map[portchan].size() and break the AGG_* ratio health checks (and may also create unnecessary counters). Since the PR description explicitly calls out ignoring accidental self-referential mappings, it’s safer to skip these keys here before inserting into the maps.

        auto portchannel = key.substr(first + 1, second - first - 1);
        auto ifname = key.substr(second + 1);
        bool portchannel_is_context = intfs.find(portchannel) != intfs.end();
        bool portchannel_is_vlan_member = vlan_map.find(portchannel) != vlan_map.end();
        // Dual-ToR downlink counters require MUX attribution that is unavailable on a nested PortChannel.

mssonicbld · 2026-07-31T17:58:02Z

/azp run

azure-pipelines · 2026-07-31T17:58:14Z

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

azure-pipelines · 2026-07-31T17:58:23Z

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

Copilot

Pull request overview

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

Bound each raw-socket callback to 64 packets and reset the recvfrom address length for every receive attempt. Rename three file-scope helpers that begin with underscores because C++ reserves those identifiers in the global namespace; static linkage already marks them as internal. Replace four nullptr comparisons with NULL to match the daemon's established style. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 <lukelin0907@gmail.com>

Preserve VLAN and PortChannel membership so a physical member resolves through its immediate PortChannel parent to the monitored VLAN context. Keep direct VLAN-member precedence, bound hierarchy traversal, ignore self-references, and centralize parent, context, and aggregate-name lookup. Aggregate only to the immediate parent so pre-VLAN observations are not rolled into the root VLAN counter and misreported as relay loss. 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-31T19:20:41Z

/azp run

azure-pipelines · 2026-07-31T19:20:51Z

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

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

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

This was referenced Jul 26, 2026

[dhcpmon] Identify parent and aggregate counter disparities #100

Draft

[dhcpmon] Track PortChannels under downstream VLANs #89

Closed

Copilot AI reviewed Jul 26, 2026

View reviewed changes

Comment thread src/dhcp_devman.cpp Outdated

Comment thread src/dhcp_devman.cpp Outdated

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

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

Copilot AI reviewed Jul 26, 2026

View reviewed changes

Xichen96 changed the title ~~[dhcpmon] Track nested PortChannel hierarchy~~ [dhcpmon] Track physical members beneath VLAN PortChannels Jul 27, 2026

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

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

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 27, 2026 03:30

Copilot started reviewing on behalf of Xichen96 July 27, 2026 03:31 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Comment thread src/dhcp_devman.h

Comment thread src/dhcp_devman.h Outdated

Copilot AI review requested due to automatic review settings July 27, 2026 03:57

Copilot started reviewing on behalf of Xichen96 July 27, 2026 03:58 View session

Copilot started reviewing on behalf of Xichen96 July 27, 2026 17:06 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 28, 2026 02:08

Copilot started reviewing on behalf of Xichen96 July 28, 2026 02:08 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 28, 2026 05:58

Copilot started reviewing on behalf of Xichen96 July 28, 2026 05:58 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

Xichen96 force-pushed the dev/xichenlin/nested-portchannel-hierarchy branch from 1659bef to 3128da1 Compare July 31, 2026 17:57

Xichen96 marked this pull request as ready for review July 31, 2026 17:58

Copilot AI review requested due to automatic review settings July 31, 2026 17:58

Copilot started reviewing on behalf of Xichen96 July 31, 2026 17:58 View session

Copilot AI reviewed Jul 31, 2026

View reviewed changes

Xichen96 and others added 2 commits August 1, 2026 05:01

Xichen96 force-pushed the dev/xichenlin/nested-portchannel-hierarchy branch from 3128da1 to 1331563 Compare July 31, 2026 19:20

Copilot AI review requested due to automatic review settings July 31, 2026 19:20

Copilot started reviewing on behalf of Xichen96 July 31, 2026 19:21 View session

Copilot AI reviewed Jul 31, 2026

View reviewed changes

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

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

Stack

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!

mssonicbld commented Jul 27, 2026

Uh oh!

azure-pipelines Bot commented Jul 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

mssonicbld commented Jul 27, 2026

Uh oh!

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

mssonicbld commented Jul 27, 2026

Uh oh!

azure-pipelines Bot commented Jul 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

mssonicbld commented Jul 28, 2026

Uh oh!

azure-pipelines Bot commented Jul 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Xichen96 commented Jul 26, 2026 •

edited

Loading