Skip to content

[dhcp_server] Wait for caclmgrd-owned docker0 syslog rule on start/stop#28580

Open
Xichen96 wants to merge 2 commits into
masterfrom
dev/xichenlin/dhcp-server-syslog-waits
Open

[dhcp_server] Wait for caclmgrd-owned docker0 syslog rule on start/stop#28580
Xichen96 wants to merge 2 commits into
masterfrom
dev/xichenlin/dhcp-server-syslog-waits

Conversation

@Xichen96

@Xichen96 Xichen96 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Why I did it

dhcp_server is the only container that forwards its rsyslog to the host over docker0 (other containers, including bridge-mode redfish, log to 127.0.0.1); that forwarded traffic uses RELP (tcp/2514). caclmgrd flushes and rebuilds the INPUT chain on every control-plane ACL change and appends a catch-all DROP. Ownership of the docker0/2514 ACCEPT exception has moved from the container start/stop script to caclmgrd, which re-installs it on every rebuild keyed on FEATURE|dhcp_server (fixes #27584).

This is step 3 of 3:

  1. [dhcp_server] Revert container-side docker0 syslog iptables add/del (#26637); caclmgrd to own the rule #28328 (merged) removed the old container-side add/del of the rule.
  2. [caclmgrd] Own the dhcp_server docker0 syslog INPUT exception sonic-host-services#412 (merged) made caclmgrd own the rule.
  3. This PR replaces the removed container-side add/del with feature-gated bounded waits that synchronize with caclmgrd.
Work item tracking
  • Microsoft ADO (number only): 38745012

How I did it

Re-add the dhcp_server branches in files/build_templates/docker_image_ctl.j2 at the same spots #28328 reverted, but as bounded waits instead of add/del:

  • 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 (state NEW) 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 container restarts, shutdown, and CONFIG_DB-unreachable all skip the wait (positive-state gate, so a restart with the feature still enabled does not block).

The canonical iptables -C command is defined once and reused by both waits. Each wait checks at seconds 0 through 10 without a duplicate final command: status 0 means present, status 1 means absent, and status 4 (xtables resource/lock contention) is retried. Other statuses produce a warning. The rule specification matches the stored form caclmgrd installs and the live iptables -S INPUT output.

How to verify it

The wait behavior was hardware-validated as part of the 3-PR set by hand-patching the rendered dhcp_server ctl script and caclmgrd change onto a running DUT (no image rebuild).

  • DUT / topology: bjw-can-720dt-2 (720dt, mx), dhcp_server enabled; RELP tcp/2514 to host rsyslogd on 240.127.1.1:2514 (the docker0 IP); a control-plane ACL rule present so caclmgrd programs its catch-all -A INPUT -j DROP.
  • preStartAction wait (present): with the feature enabled, the container start proceeds once caclmgrd has the docker0/2514 ACCEPT rule installed (INPUT pos 3, above the catch-all DROP); end-to-end docker exec dhcp_server logger ... reached host /var/log/syslog.
  • stop() wait (absent): config feature state dhcp_server disabled -> caclmgrd removes the rule and the container reaches Exited(0); re-enabled re-adds the rule and the container starts.
  • Restart does not block (by design; not separately hardware-exercised): the stop wait is gated on FEATURE|dhcp_server being disabled, so a plain container restart with the feature still enabled skips it.
  • Final polling implementation: rendered the dhcp_server specialization and passed bash -n; a controlled shell harness passed present, absent, lock-busy, unexpected-error, and timeout cases; the exact iptables -C command returned status 0 against the live rule on the 202605 DUT.

Note: this hardware validation was performed on a 202605 image (identical container ctl / caclmgrd path); the 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

Coordinated with the caclmgrd change (step 2); the buildimage backport is handled together with #28328 as a set so the branch is never left with neither party owning the rule.
Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO): Microsoft ADO 38745012
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); see "How to verify it" above. The docker0 tcp/2514 ACCEPT rule is programmed above the catch-all DROP, end-to-end RELP syslog is delivered, the feature toggle adds/removes the rule cleanly, and the rule 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

[dhcp_server] Replace the container-side docker0 syslog iptables add/del with feature-gated bounded waits on the caclmgrd-owned tcp/2514 rule.

@Xichen96
Xichen96 requested a review from lguohan as a code owner July 22, 2026 16:13
Copilot AI review requested due to automatic review settings July 22, 2026 16:13
@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 rendered dhcp_server container control script template to synchronize start/stop with the iptables rule that caclmgrd now owns (docker0 → host rsyslog RELP tcp/2514), using bounded waits instead of container-side rule add/delete. This helps prevent syslog connectivity drops during control-plane ACL rebuilds by ensuring the exception rule is present before start, and (on explicit feature disable) removed before stop returns.

Changes:

  • Add a preStartAction wait (up to 10s) for the docker0 tcp/2514 ACCEPT rule when FEATURE|dhcp_server is enabled.
  • Add a stop() wait (up to 10s) for the rule to be removed when FEATURE|dhcp_server is disabled.

Comment thread files/build_templates/docker_image_ctl.j2 Outdated
Comment thread files/build_templates/docker_image_ctl.j2 Outdated
Copilot AI review requested due to automatic review settings July 22, 2026 16:20
@Xichen96
Xichen96 force-pushed the dev/xichenlin/dhcp-server-syslog-waits branch from 68faa04 to a8046d7 Compare July 22, 2026 16:20
@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 1 out of 1 changed files in this pull request and generated 2 comments.

Comment thread files/build_templates/docker_image_ctl.j2 Outdated
Comment thread files/build_templates/docker_image_ctl.j2 Outdated
Copilot AI review requested due to automatic review settings July 22, 2026 16:26
@Xichen96
Xichen96 force-pushed the dev/xichenlin/dhcp-server-syslog-waits branch from a8046d7 to b3ae12d Compare July 22, 2026 16:26
@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 1 out of 1 changed files in this pull request and generated no new comments.

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
Xichen96 requested a review from Copilot July 22, 2026 17:21

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 2 comments.

Comment thread files/build_templates/docker_image_ctl.j2 Outdated
Comment thread files/build_templates/docker_image_ctl.j2 Outdated
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
Xichen96 force-pushed the dev/xichenlin/dhcp-server-syslog-waits branch from b3ae12d to a10a5ac Compare July 23, 2026 07:46
@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).

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
Xichen96 requested a review from Copilot July 23, 2026 10:10

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.

Comments suppressed due to low confidence (1)

files/build_templates/docker_image_ctl.j2:930

  • Same bounding issue in stop(): the loop intends to wait ~10s, but iptables -w 5 -C can block up to 5s per probe under xtables lock contention, so this path may take far longer than 10s (and the warning text becomes misleading). Tightening -w and retry count keeps the stop path’s worst-case delay aligned with the intended bound.
    if [ "$($SONIC_DB_CLI CONFIG_DB HGET 'FEATURE|dhcp_server' state 2>/dev/null)" = "disabled" ]; then
        for i in $(seq 1 10); do
            iptables -w 5 -C INPUT -i docker0 -p tcp -m tcp --dport 2514 -m comment --comment "dhcp_server_syslog" -j ACCEPT 2>/dev/null || break
            sleep 1
        done
        iptables -w 5 -C INPUT -i docker0 -p tcp -m tcp --dport 2514 -m comment --comment "dhcp_server_syslog" -j ACCEPT 2>/dev/null && \
            echo "Warning: caclmgrd-owned dhcp_server docker0 syslog ACCEPT rule still present after 10s despite dhcp_server feature disabled."
    fi

Comment thread files/build_templates/docker_image_ctl.j2
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>
Copilot AI review requested due to automatic review settings July 23, 2026 12:42
@Xichen96
Xichen96 force-pushed the dev/xichenlin/dhcp-server-syslog-waits branch from a10a5ac to 78384be Compare July 23, 2026 12:42
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>
@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 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 19:08
@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 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

files/build_templates/docker_image_ctl.j2:45

  • Consider adding a short xtables-lock wait to the iptables existence probe. Without -w, iptables -C returns immediately with status 4 when caclmgrd is holding the lock, which forces the loop to sleep 1s even for sub-second contention (delaying start/stop and possibly producing spurious warnings).
DHCP_SERVER_SYSLOG_CHECK=(iptables -C INPUT -i docker0 -p tcp -m tcp --dport 2514 -m comment --comment dhcp_server_syslog -j ACCEPT)

Define the canonical iptables check once and reuse it in the start and
stop waits. Poll the rule state without a duplicate final check, and
retry explicit xtables lock contention by exit status.

Signed-off-by: Xichen96 <lukelin0907@gmail.com>
@Xichen96
Xichen96 force-pushed the dev/xichenlin/dhcp-server-syslog-waits branch from 86b1de5 to ece7594 Compare July 24, 2026 20:31
@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).

@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

No failed(or canceled) jobs found in completed stages. Only failed(or canceled) jobs in completed stages can be retried.

Stages in progress: BuildVS, Build. Please wait for the stages to complete and then retry again.

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.

Bug: CACL updates remove dhcp_server syslog iptables rule

3 participants