Skip to content

[dhcpmon] Classify nested VLAN members as downlinks - #28625

Closed
Xichen96 wants to merge 2 commits into
sonic-net:masterfrom
Xichen96:dev/xichenlin/classify-nested-dhcpmon-downlinks
Closed

[dhcpmon] Classify nested VLAN members as downlinks#28625
Xichen96 wants to merge 2 commits into
sonic-net:masterfrom
Xichen96:dev/xichenlin/classify-nested-dhcpmon-downlinks

Conversation

@Xichen96

Copy link
Copy Markdown
Contributor

Why I did it

sonic-dhcpmon PR sonic-net/sonic-dhcpmon#89 adds
counter rows for physical members beneath a PortChannel that belongs to the
downstream VLAN:

Ethernet312 -> PortChannel1005 -> Vlan1000

The current show dhcp_relay ipv4/ipv6 counters implementation classifies
downlinks from direct VLAN_MEMBER entries only. The nested Ethernet members
exist in PORTCHANNEL_MEMBER, so the CLI would incorrectly display their new
counter rows as Uplink.

Work item tracking
  • Microsoft ADO (number only): 38615656

How I did it

  • Read all PORTCHANNEL_MEMBER keys once and build a parent-to-children map.
  • When a direct VLAN member is a PortChannel, add its physical children to the
    existing VLAN downlink set.
  • Preserve the current command, columns, counter keys, and classification of
    direct VLAN members, uplinks, and management interfaces.
  • Extend the focused CLI unit test with a VLAN-backed PortChannel and an
    unrelated PortChannel to verify that only the correct children are included.

How to verify it

  1. Let this PR's Azure CI run the existing docker-dhcp-relay CLI plugin tests.
  2. With sonic-dhcpmon PR Split script: push_docker.sh #89 installed, run:
    • show dhcp_relay ipv4 counters Vlan1000
    • show dhcp_relay ipv6 counters Vlan1000
  3. Confirm direct VLAN members, the VLAN-backed PortChannel, and its physical
    members display as Downlink, while unrelated interfaces remain Uplink.

Which release branch to backport (provide reason below if selected)

  • 202305
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 202605
  • 202608

No backport is requested. This repair targets master only.

Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO):
Failure type:

Tested branch

  • master
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 202605
  • 202608
  • N/A

Test result

Pending this PR's Azure CI and combined master hardware validation with
sonic-dhcpmon PR #89.

Description for the changelog

Classify physical members of a VLAN-backed PortChannel as downlinks in dhcpmon
counter output.

Link to config_db schema for YANG module changes

N/A - no YANG or CONFIG_DB schema changes.

A picture of a cute animal (not mandatory but encouraged)

N/A.

Expand VLAN-backed PortChannels to their physical members when displaying dhcpmon counters so the new hierarchy is not mislabeled as uplink traffic.

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 17:07
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

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

@azure-pipelines

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the docker-dhcp-relay CLI plugin so that show dhcp_relay ipv4/ipv6 counters correctly classifies physical interfaces beneath a VLAN-backed PortChannel as Downlink (e.g., Ethernet -> PortChannel -> Vlan), aligning the CLI output with new nested counter rows produced by dhcpmon.

Changes:

  • Add parsing of PORTCHANNEL_MEMBER entries from CONFIG_DB to expand VLAN downlink membership to include physical PortChannel children.
  • Extend the focused CLI unit test to cover a VLAN-backed PortChannel and ensure only the correct physical children are included.

Reviewed changes

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

File Description
dockers/docker-dhcp-relay/cli/show/plugins/show_dhcp_relay.py Expands VLAN downlink set to include physical members of VLAN-member PortChannels by reading PORTCHANNEL_MEMBER keys.
dockers/docker-dhcp-relay/cli-plugin-tests/test_show_dhcpmon_counters.py Updates unit test expectations to include PortChannel physical children as VLAN downlinks.

Comment thread dockers/docker-dhcp-relay/cli/show/plugins/show_dhcp_relay.py
@Xichen96

Copy link
Copy Markdown
Contributor Author

/azpw run

@mssonicbld

Copy link
Copy Markdown
Collaborator

⚠️ Notice: /azpw run only runs failed jobs now. If you want to trigger a whole pipline run, please rebase your branch or close and reopen the PR.
💡 Tip: You can also use /azpw retry to retry failed jobs directly.

Retrying failed(or canceled) jobs...

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) stages in build 1174830:

✅Stage Build:

  • Job marvell_prestera_arm64: retried.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Ignore malformed VLAN_MEMBER keys before classifying counter interfaces.

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:17
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@Xichen96

Copy link
Copy Markdown
Contributor Author

Closing per maintainer direction: keep the Fungible F2 repair entirely in dhcpmon/dhcp6relay and preserve the existing CLI unchanged.

@Xichen96 Xichen96 closed this Jul 26, 2026
@Xichen96
Xichen96 deleted the dev/xichenlin/classify-nested-dhcpmon-downlinks branch July 26, 2026 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants