Skip to content

[dhcp_server] Revert container-side docker0 syslog iptables add/del (#26637); caclmgrd to own the rule#28328

Merged
StormLiangMS merged 1 commit into
sonic-net:masterfrom
Xichen96:dev/xichenlin/revert-dhcp-server-syslog-iptables
Jul 16, 2026
Merged

[dhcp_server] Revert container-side docker0 syslog iptables add/del (#26637); caclmgrd to own the rule#28328
StormLiangMS merged 1 commit into
sonic-net:masterfrom
Xichen96:dev/xichenlin/revert-dhcp-server-syslog-iptables

Conversation

@Xichen96

@Xichen96 Xichen96 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Why I did it

Reverts #26637.

This is step 1 of 3 of moving the dhcp_server docker0 syslog (UDP 514) INPUT exception out of the container start/stop script so that caclmgrd owns it. caclmgrd will re-install the rule on every control-plane ACL flush-and-rebuild, keyed on FEATURE|dhcp_server, so the rebuild never drops it — the root cause of #27584.

Plan (parent ADO 38699922):

  1. (this PR) revert the container-side iptables -I / -D add/del.
  2. caclmgrd installs/removes the rule based on FEATURE|dhcp_server (sonic-host-services#412 — must reach the image before this repo relies on it).
  3. replace the removed add/del with feature-gated bounded waits that synchronize with caclmgrd ([dhcp_server] Wait for caclmgrd-owned docker0 syslog rule on start/stop #28580).
Work item tracking
  • Microsoft ADO (number only): 38745010

How I did it

git revert of 933d908 (#26637). Removes the two dhcp_server-specific blocks in files/build_templates/docker_image_ctl.j2 (the iptables -I in preStartAction and the iptables -D in stop). No other change.

How to verify it

  1. grep dhcp_server_syslog files/build_templates/docker_image_ctl.j2 returns nothing.
  2. The rendered dhcp_server container ctl script no longer adds/removes the docker0 UDP 514 iptables rule on start/stop.
Hardware validation

Validated as part of the 3-PR set (revert here + caclmgrd ownership + container waits) by hand-patching the changes onto a running DUT (no image rebuild).

  • DUT / topology: bjw-can-720dt-2 (720dt, mx), dhcp_server enabled.
  • Transport: RELP tcp/2514 (host rsyslogd listens on 240.127.1.1:2514, the docker0 IP).
  • This PR's effect on the DUT: the dhcp_server container ctl script no longer adds/removes the docker0 syslog iptables rule on start/stop — the rule is instead owned and re-installed by caclmgrd.

Results:

  1. caclmgrd owns the rule, above the catch-all DROP (feature enabled):
    3:-A INPUT -i docker0 -p tcp -m tcp --dport 2514 -m comment --comment dhcp_server_syslog -j ACCEPT
    46:-A INPUT -j DROP
    
  2. End-to-end syslog deliverydocker exec dhcp_server logger ... reached host /var/log/syslog; the docker0 ACCEPT rule showed packet hits (2 pkts / 214 B).
  3. Feature toggleconfig feature state dhcp_server disabled removes the rule; enabled re-adds it (caclmgrd logs both transitions).
  4. Survives control-plane ACL re-walk (the root regression, Bug: CACL updates remove dhcp_server syslog iptables rule #27584) — after modifying an unrelated control-plane ACL rule (forcing caclmgrd to flush + rebuild INPUT), the docker0/2514 ACCEPT rule is still present at position 3, above the catch-all DROP. With the container-side add/del removed by this PR, this only holds because caclmgrd now owns the rule.

Note: this hardware validation was performed on a 202605 image (identical container ctl / caclmgrd flush-rebuild path); the code change under test is the same logic.

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

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

Backporting the revert in isolation would leave the branch with neither the container nor caclmgrd owning the rule; the 202605 backport is handled together with the caclmgrd change (sonic-host-services#412) and the container-waits change (#28580) as a coordinated set.
Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO): Microsoft ADO 38745010
Failure type: day-one issue (the container-owned docker0 syslog rule never survived a caclmgrd control-plane ACL rebuild)

Tested branch

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

Test result

  • 202605: 20260510.07 — hand-patched HW validation on bjw-can-720dt-2 (720dt, mx) as part of the 3-PR set; see "Hardware validation" above (caclmgrd owns the docker0 tcp/2514 rule above the catch-all DROP, end-to-end RELP syslog delivered, feature toggle add/remove, survives an unrelated control-plane ACL re-walk — bug Bug: CACL updates remove dhcp_server syslog iptables rule #27584 fix).
  • master: validation pending — the rendered dhcp_server ctl script and caclmgrd code path are identical to 202605; a master-image run is the only remaining validation step.

Description for the changelog

Revert the dhcp_server container-side docker0 syslog iptables add/del; ownership of that INPUT exception moves to caclmgrd.

…n bridge-mode container (sonic-net#26637)"

This reverts commit 933d908.

Move ownership of the dhcp_server docker0 syslog (UDP 514) INPUT exception
out of the container start/stop script. Going forward caclmgrd owns this
rule and re-installs it on every control-plane ACL flush-and-rebuild, keyed
on FEATURE|dhcp_server, so the rebuild never drops it
(sonic-net#27584).

This revert is step 1 of that plan. The caclmgrd change (sonic-host-services)
and the container-side bounded waits that replace this add/del land in
follow-up PRs.

Signed-off-by: Xichen96 <lukelin0907@gmail.com>
@Xichen96
Xichen96 requested a review from lguohan as a code owner July 9, 2026 14:37
Copilot AI review requested due to automatic review settings July 9, 2026 14:37
@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

This PR reverts the dhcp_server container-side management of the host iptables INPUT exception for bridge-mode syslog (UDP/514 on docker0), as part of moving ownership of that rule to caclmgrd so it persists across CACL flush/rebuild operations.

Changes:

  • Removed the dhcp_server-specific iptables -I ... --dport 514 ... insertion from preStartAction().
  • Removed the matching iptables -D ... --dport 514 ... deletion from stop().
Comments suppressed due to low confidence (1)

files/build_templates/docker_image_ctl.j2:138

  • This revert removes the only place in this repo that ensured UDP/514 syslog from the bridge-mode dhcp_server container is accepted on the host (INPUT on docker0). Unless the image already includes the follow-up change where caclmgrd reinstalls the equivalent rule on every CACL rebuild, this will reintroduce the original failure mode (syslog silently dropped when the catch-all DROP is present). Consider updating the relevant dependency (e.g., sonic-host-services/caclmgrd) in the same PR or otherwise gating/ordering the merge so the image is never built without any component owning this exception.
{%- else %}
    : # nothing
{%- endif %}
    updateSyslogConf
}

@Xichen96

Copy link
Copy Markdown
Contributor Author

Gentle ping for review 🙏 — this is the resolved approach for the caclmgrd / dhcp_server syslog-rule ownership: it reverts the container-side docker0 iptables add/del from #26637 and lets caclmgrd own the rule, replacing the abandoned sonic-host-services#396. CI green. @yxieca @StormLiangMS could you take a look? Thanks!

@dgsudharsan

Copy link
Copy Markdown
Collaborator

@Xichen96 Can you provide ETA to raise PR to make caclmgrd owning the iptable rules?

@StormLiangMS StormLiangMS 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.

LGTM

@StormLiangMS
StormLiangMS merged commit 80a6a0f into sonic-net:master Jul 16, 2026
29 checks passed
@mssonicbld

Copy link
Copy Markdown
Collaborator

The label Approved for 202605 Branch can only be added by a member of the @sonic-net/release-manager-202605 team. Removing the label. Please contact one of the release managers: @vaibhavhd to approve the cherry pick.

@Xichen96

Copy link
Copy Markdown
Contributor Author

@vaibhavhd Hi Vaibhvd, pls help review, I want this in the 202605 branch

@Xichen96 Xichen96 added the Tested for 202605 branch Tested for 202605 branch label Jul 22, 2026
Xichen96 added a commit that referenced this pull request Jul 22, 2026
Step 3 of 3 of moving the dhcp_server docker0 syslog INPUT exception
ownership to caclmgrd (fixes #27584). PR1
(#28328) removed the container-side add/del; PR2 (sonic-net/sonic-host-services#412)
made caclmgrd own the rule. This replaces the removed add/del with
feature-gated bounded waits that synchronize with caclmgrd:

- preStartAction: when FEATURE|dhcp_server is enabled, wait up to 10s for
  the docker0 tcp/2514 ACCEPT rule to be present before the container
  starts, so the RELP client's first connection is not dropped by the
  control-plane catch-all DROP.
- stop(): when FEATURE|dhcp_server is disabled, wait up to 10s for the
  rule to be removed. Plain restarts / shutdown / CONFIG_DB-down skip the
  wait (positive-state gate).

The iptables -C existence check matches the canonical stored form of the
rule caclmgrd installs (identical to `iptables -S INPUT` output), so the
wait observes the rule reliably across iptables variants.

Signed-off-by: Xichen96 <lukelin0907@gmail.com>
Xichen96 added a commit that referenced this pull request Jul 22, 2026
Step 3 of 3 of moving the dhcp_server docker0 syslog INPUT exception
ownership to caclmgrd (fixes #27584). PR1
(#28328) removed the container-side add/del; PR2 (sonic-net/sonic-host-services#412)
made caclmgrd own the rule. This replaces the removed add/del with
feature-gated bounded waits that synchronize with caclmgrd:

- preStartAction: when FEATURE|dhcp_server is enabled, wait up to 10s for
  the docker0 tcp/2514 ACCEPT rule to be present before the container
  starts, so the RELP client's first connection is not dropped by the
  control-plane catch-all DROP.
- stop(): when FEATURE|dhcp_server is disabled, wait up to 10s for the
  rule to be removed. Plain restarts / shutdown / CONFIG_DB-down skip the
  wait (positive-state gate).

The iptables -C existence check matches the canonical stored form of the
rule caclmgrd installs (identical to `iptables -S INPUT` output), so the
wait observes the rule reliably across iptables variants.

Signed-off-by: Xichen96 <lukelin0907@gmail.com>
Xichen96 added a commit to sonic-net/sonic-mgmt that referenced this pull request Jul 22, 2026
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 added a commit to sonic-net/sonic-mgmt that referenced this pull request Jul 22, 2026
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 added a commit to sonic-net/sonic-mgmt that referenced this pull request Jul 22, 2026
Add test_caclmgrd_dhcp_server_syslog verifying the docker0 RELP tcp/2514
INPUT exception that caclmgrd now owns (sonic-net/sonic-buildimage#28328,
sonic-net/sonic-host-services#412, sonic-net/sonic-buildimage#28580;
fixes sonic-net/sonic-buildimage#27584):

  * present above the control-plane catch-all DROP while
    FEATURE|dhcp_server is enabled,
  * survives a control-plane ACL flush-and-rebuild (forced via the
    dummy_acl_rules fixture / acl-loader), and
  * removed on feature-disable and restored on feature-enable.

The test skips when dhcp_server is not enabled on the DUT.

Signed-off-by: Xichen Lin <lukelin0907@gmail.com>
Xichen96 added a commit that referenced this pull request Jul 23, 2026
Step 3 of 3 of moving the dhcp_server docker0 syslog INPUT exception
ownership to caclmgrd (fixes #27584). PR1
(#28328) removed the container-side add/del; PR2 (sonic-net/sonic-host-services#412)
made caclmgrd own the rule. This replaces the removed add/del with
feature-gated bounded waits that synchronize with caclmgrd:

- preStartAction: when FEATURE|dhcp_server is enabled, wait up to 10s for
  the docker0 tcp/2514 ACCEPT rule to be present before the container
  starts, so the RELP client's first connection is not dropped by the
  control-plane catch-all DROP.
- stop(): when FEATURE|dhcp_server is disabled, wait up to 10s for the
  rule to be removed. Plain restarts / shutdown / CONFIG_DB-down skip the
  wait (positive-state gate).

The iptables -C existence check matches the canonical stored form of the
rule caclmgrd installs (identical to `iptables -S INPUT` output), so the
wait observes the rule reliably across iptables variants.

Signed-off-by: Xichen96 <lukelin0907@gmail.com>
Xichen96 added a commit to sonic-net/sonic-mgmt that referenced this pull request Jul 23, 2026
Add test_caclmgrd_dhcp_server_syslog verifying the docker0 RELP tcp/2514
INPUT exception that caclmgrd now owns (sonic-net/sonic-buildimage#28328,
sonic-net/sonic-host-services#412, sonic-net/sonic-buildimage#28580;
fixes sonic-net/sonic-buildimage#27584):

  * present above the control-plane catch-all DROP while
    FEATURE|dhcp_server is enabled,
  * survives a control-plane ACL flush-and-rebuild (forced via the
    dummy_acl_rules fixture / acl-loader), and
  * removed on feature-disable and restored on feature-enable.

The test skips when dhcp_server is not enabled on the DUT.

Signed-off-by: Xichen Lin <lukelin0907@gmail.com>
Xichen96 added a commit that referenced this pull request Jul 23, 2026
Step 3 of 3 of moving the dhcp_server docker0 syslog INPUT exception
ownership to caclmgrd (fixes #27584). PR1
(#28328) removed the container-side add/del; PR2 (sonic-net/sonic-host-services#412)
made caclmgrd own the rule. This replaces the removed add/del with
feature-gated bounded waits that synchronize with caclmgrd:

- preStartAction: when FEATURE|dhcp_server is enabled, wait up to 10s for
  the docker0 tcp/2514 ACCEPT rule to be present before the container
  starts, so the RELP client's first connection is not dropped by the
  control-plane catch-all DROP.
- stop(): when FEATURE|dhcp_server is disabled, wait up to 10s for the
  rule to be removed. Plain restarts / shutdown / CONFIG_DB-down skip the
  wait (positive-state gate).

The iptables -C existence check matches the canonical stored form of the
rule caclmgrd installs (identical to `iptables -S INPUT` output), so the
wait observes the rule reliably across iptables variants.

Signed-off-by: Xichen96 <lukelin0907@gmail.com>
Xichen96 added a commit to sonic-net/sonic-mgmt that referenced this pull request Jul 23, 2026
Add test_caclmgrd_dhcp_server_syslog verifying the docker0 RELP tcp/2514
INPUT exception that caclmgrd now owns (sonic-net/sonic-buildimage#28328,
sonic-net/sonic-host-services#412, sonic-net/sonic-buildimage#28580;
fixes sonic-net/sonic-buildimage#27584):

  * present above the control-plane catch-all DROP while
    FEATURE|dhcp_server is enabled,
  * survives a control-plane ACL flush-and-rebuild (forced via the
    dummy_acl_rules fixture / acl-loader), and
  * removed on feature-disable and restored on feature-enable.

The test skips when dhcp_server is not enabled on the DUT.

Signed-off-by: Xichen Lin <lukelin0907@gmail.com>
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.

5 participants