Skip to content

[cacl][test] Expect dhcp_server docker0 syslog rule on tcp/2514 - #26477

Open
Xichen96 wants to merge 3 commits into
masterfrom
dev/xichenlin/cacl-test-dhcp-server-tcp2514
Open

[cacl][test] Expect dhcp_server docker0 syslog rule on tcp/2514#26477
Xichen96 wants to merge 3 commits into
masterfrom
dev/xichenlin/cacl-test-dhcp-server-tcp2514

Conversation

@Xichen96

@Xichen96 Xichen96 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Description of PR

Summary:
caclmgrd's control-plane ACL rebuild appends a catch-all INPUT ... -j DROP. The dhcp_server container forwards its rsyslog to the host over docker0 (it is the only container that does so; other containers, including bridge-mode redfish, log to 127.0.0.1), and that traffic now uses RELP over tcp/2514 (not the old udp/514). Ownership of the docker0 ACCEPT exception moved from the container start/stop script to caclmgrd, which re-installs it on every rebuild keyed on FEATURE|dhcp_server — so it survives the flush-and-rebuild (root cause of sonic-net/sonic-buildimage#27584).

This updates generate_expected_rules() in tests/cacl/test_cacl_application.py to assert the new tcp/2514 exception instead of the stale udp/514 rule and to gate that expectation on the FEATURE|dhcp_server state (what caclmgrd keys on) rather than container presence, so the CACL test matches the shipped behavior.

Product changes this test requires (merged on master):

Complementary follow-up sonic-net/sonic-buildimage#28580 adds container-side feature-gated waits; it does not change the rule asserted by this test.

Fixes # (issue)
ADO: 38902699 (parent PBI 38699922). Underlying bug: sonic-net/sonic-buildimage#27584.

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
  • Test case improvement

Back port request

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

Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO): Microsoft ADO 38902699
Failure type: day-one issue (the container-owned docker0 syslog rule never survived a caclmgrd control-plane ACL rebuild; the test expectation must move to tcp/2514 in lockstep with the product change on each branch it is backported to)

Tested branch

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

Test result

202605 hardware validation passed on bjw-can-720dt-2 (720dt, mx) running SONiC.20260510.07. The complete PR series was cherry-picked onto internal-202605 and run with:

run_tests.sh -n testbed-bjw-can-720dt-2 -c cacl/test_cacl_application.py -i ../ansible/veos,../ansible/bjw -u -x -a False

Result: 5 passed, 4 topology skips in 25:51. Post-teardown, FEATURE|dhcp_server remained enabled, the container was running, and the canonical tcp/2514 rule remained at INPUT position 3 above the catch-all DROP at position 46.

Approach

What is the motivation for this PR?

Keep the CACL regression test aligned with the product: the dhcp_server docker0 syslog exception is now a caclmgrd-owned tcp/2514 rule, not a container-owned udp/514 rule.

How did you do it?

In generate_expected_rules() (host / asic_index is None branch), changed the expected dhcp_server_syslog rule from -p udp -m udp --dport 514 to -p tcp -m tcp --dport 2514, updated the comment to describe the new caclmgrd ownership, and changed the gate from mere container presence to the FEATURE|dhcp_server state returned by duthost.get_feature_status() that caclmgrd itself keys on.

How did you verify/test it?

python3 -m py_compile + repo pre-commit (flake8) pass. The full 202605 hardware run passed as recorded in Test result; the expected rule matches the live iptables -S INPUT output after test teardown.

Any platform specific information?

None — host-namespace IPv4 INPUT rule; applies to all platforms running the dhcp_server feature.

Supported testbed topology if it's a new test case?

N/A (existing test).

Documentation

N/A.

Copilot AI review requested due to automatic review settings July 22, 2026 16:46
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@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

Updates the CACL application test expectations to match the current caclmgrd behavior for the dhcp_server container’s syslog traffic over docker0, ensuring the regression test aligns with the control-plane ACL rebuild behavior.

Changes:

  • Update the expected dhcp_server_syslog iptables exception from UDP/514 to TCP/2514.
  • Refresh the in-code rationale comment describing why/when this exception should be present.

Comment thread tests/cacl/test_cacl_application.py Outdated
Copilot AI review requested due to automatic review settings July 22, 2026 16:50
@Xichen96
Xichen96 force-pushed the dev/xichenlin/cacl-test-dhcp-server-tcp2514 branch from 5f4f9fb to 7c8e31d Compare July 22, 2026 16:50
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp 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

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

The dhcp_server docker0 syslog INPUT exception changed from UDP 514 to
RELP over tcp/2514, and its ownership moved from the container
start/stop script to caclmgrd, which re-installs it on every
control-plane ACL flush-and-rebuild while FEATURE|dhcp_server is enabled
(sonic-net/sonic-buildimage#28328, sonic-net/sonic-host-services#412,
sonic-net/sonic-buildimage#28580; fixes sonic-net/sonic-buildimage#27584).

Update generate_expected_rules() to assert the tcp/2514 exception
instead of the old udp/514 rule, and gate the expectation on the
FEATURE|dhcp_server state that caclmgrd keys on (read via sonic-db-cli),
rather than on mere container presence.

Signed-off-by: Xichen Lin <lukelin0907@gmail.com>
@Xichen96
Xichen96 force-pushed the dev/xichenlin/cacl-test-dhcp-server-tcp2514 branch from 7c8e31d to 94dffbd Compare July 22, 2026 17:34
Copilot AI review requested due to automatic review settings July 22, 2026 17:34
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@github-actions
github-actions Bot requested a review from ZhaohuiS July 22, 2026 17:34
@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 1 out of 1 changed files in this pull request and generated no new comments.

@Xichen96
Xichen96 requested a review from Copilot July 22, 2026 17:47

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

@mssonicbld

Copy link
Copy Markdown
Collaborator

This PR has backport request label(s) for branch(es): 202605, but is missing required test information. Please make sure you tick the tested branch(es) in the Tested branch section and provide test evidence (e.g., 202605: <test result>) in the Test result section as well in your PR description.

---Powered by SONiC BuildBot

Replace the raw sonic-db-cli CONFIG_DB HGET with the mgmt-native
duthost.get_feature_status() helper, and shorten the comment.

Signed-off-by: Xichen Lin <lukelin0907@gmail.com>
Copilot AI review requested due to automatic review settings July 23, 2026 16:39
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp 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

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

Comment thread tests/cacl/test_cacl_application.py
Copilot AI review requested due to automatic review settings July 23, 2026 17:40
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp 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

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

Copilot AI review requested due to automatic review settings July 24, 2026 05:40
@Xichen96
Xichen96 force-pushed the dev/xichenlin/cacl-test-dhcp-server-tcp2514 branch from 96f1df3 to 1776ee5 Compare July 24, 2026 05:40
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp 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

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

Signed-off-by: Xichen Lin <lukelin0907@gmail.com>
Copilot AI review requested due to automatic review settings July 24, 2026 05:47
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp 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

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

@Xichen96 Xichen96 added the Tested for 202605 branch Tested for 202605 branch label Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants