[caclmgrd] Own the dhcp_server docker0 syslog INPUT exception#412
Conversation
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
64c85b3 to
e8dac04
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Updates caclmgrd (the host-side control-plane ACL manager that flushes/rebuilds iptables) to own the dhcp_server syslog exception rule so it reliably persists across ACL rebuilds, preventing UDP/514 syslog traffic arriving via docker0 from being dropped by the catch-all INPUT DROP.
Changes:
- Add
DhcpServerSyslogAllowedfeature-gated flag (seeded fromFEATURE|dhcp_serverat init) and re-emit thedocker0/UDP 514ACCEPTrule during host-namespace IPv4 ACL rebuilds. - Generalize the FEATURE-table event handler from redfish-only to
handle_feature_state_events, handling bothredfishanddhcp_serverstate transitions. - Add a focused unit test suite for the new
dhcp_serversyslog exception behavior and update the existing redfish test to use the renamed handler.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| scripts/caclmgrd | Seeds/handles FEATURE.dhcp_server state and emits the host-only docker0 UDP/514 ACCEPT exception before any catch-all DROP; generalizes FEATURE event handling for redfish + dhcp_server. |
| tests/caclmgrd/caclmgrd_dhcp_server_syslog_test.py | New unit tests validating seeding, rule presence/absence, ordering before catch-all DROP, host-namespace-only emission, and FEATURE event transitions. |
| tests/caclmgrd/caclmgrd_redfish_acl_test.py | Updates test to call the renamed generalized FEATURE event handler. |
|
@StormLiangMS @yxieca @lguohan could you please review when you get a chance? This is step 2 of 3 (caclmgrd now owns the dhcp_server docker0 syslog INPUT exception; buildimage#28328 removed the old container-side add/del). Thanks! |
|
This PR has backport request for branch(es): 202605. ---Powered by SONiC BuildBot
|
StormLiangMS
left a comment
There was a problem hiding this comment.
@Xichen96 — the master-branch implementation looks good.
caclmgrd now owns and re-emits the feature-gated docker0 UDP/514 exception before the catch-all INPUT DROP, including correct enable/disable rebuild behavior and host-namespace-only scoping. CI is green and I found no source-branch code blocker.
For the 202605 request, the current cherry-pick conflicts because that branch does not contain the master redfish FEATURE-event infrastructure or its test. The backport needs a target-aware adaptation that adds only the dhcp_server handling, plus explicit 202605 test evidence.
|
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
|
dhcp_server is the only bridge-mode container, so its syslog reaches host rsyslog over docker0. caclmgrd flushes and rebuilds the INPUT chain on every control-plane ACL change, appending a catch-all DROP, which drops that syslog traffic. Previously the dhcp_server container script added/removed this exception itself, but caclmgrd wiped it on the next rebuild (sonic-net/sonic-buildimage#27584). Container-to-host syslog forwarding uses RELP over tcp/2514 (since sonic-net/sonic-buildimage#18113), so the exception is for tcp/2514. The connection is initiated by the container, so the inbound SYN is a new connection that the conntrack ESTABLISHED,RELATED accept does not cover -- it needs an explicit ACCEPT or the RELP session never opens. Make caclmgrd own the exception, mirroring the existing REDFISH FEATURE-gated mechanism: - Seed DhcpServerSyslogAllowed from FEATURE|dhcp_server state at init. - Re-emit the docker0/2514 ACCEPT rule on every rebuild while enabled, before any catch-all DROP (host namespace / IPv4 only). On feature-disable the rule is not re-emitted, so the next rebuild removes it. - Generalize the shared FEATURE-table subscriber handler (handle_redfish_feature_events -> handle_feature_state_events) to drain both redfish and dhcp_server keys in the one loop; a second popper would steal events from the redfish handling. The rule string is byte-identical to the container-side -C check so the container can wait on presence/absence (follow-up PR). Step 2 of 3 (buildimage#28328 removed the old container-side add/del; a follow-up adds container-side waits). ADO: 38745011. Signed-off-by: Xichen Lin <lukelin0907@gmail.com> Signed-off-by: Xichen96 <lukelin0907@gmail.com>
e8dac04 to
f913bf9
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
hi @Xichen96 Do we have a SONiC-MGMT test that verifies this functionality? If no, pls open an issue to track it as a test gap, then we can merge this PR. |
|
Hi @StormLiangMS — thanks for the review. There is existing sonic-mgmt coverage: I've opened a tracking issue for it: sonic-net/sonic-mgmt#26470. This has been hardware-validated by manual patch on an |
|
The change is not in 202605 yet. @Xichen96, please manually create the cherry pick PR for branch 202605. ---Powered by SONiC BuildBot
|
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>
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>
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>
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>
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>
Why I did it
dhcp_serveris the only bridge-mode container, so its syslog reaches host rsyslog overdocker0.caclmgrdflushes and rebuilds theINPUTchain on every control-plane ACL change and appends a catch-allDROP, which drops that syslog traffic. Thedhcp_servercontainer script used to add/remove thisdocker0ACCEPTexception itself, butcaclmgrdwiped it on the next rebuild.Container-to-host syslog forwarding uses RELP over tcp/2514 (since sonic-net/sonic-buildimage#18113), so the exception is for tcp/2514. The RELP connection is initiated by the container, so its inbound SYN is a new connection that caclmgrd's
conntrack ESTABLISHED,RELATEDaccept does not cover — it needs an explicit ACCEPT or the RELP session never opens.Fixes sonic-net/sonic-buildimage#27584.
This is step 2 of 3:
caclmgrdown the rule.Work item tracking
How I did it
Mirror the existing REDFISH
FEATURE-gated mechanism:DhcpServerSyslogAllowedfromFEATURE|dhcp_serverstate at init.docker0/2514ACCEPTrule on every rebuild while enabled, before any catch-allDROP(host namespace / IPv4 only). On feature-disable the rule is simply not re-emitted, so the next rebuild removes it.FEATURE-table subscriber handler (handle_redfish_feature_events->handle_feature_state_events) to drain bothredfishanddhcp_serverkeys in the one loop (a second popper would steal events from the redfish handling).The rule string is byte-identical to the container-side
-Ccheck so the container can wait on presence/absence in the follow-up PR.How to verify it
tests/caclmgrd/caclmgrd_dhcp_server_syslog_test.pycovers: init seeding from FEATURE, rule present when enabled / absent when disabled, rule strictly before the catch-all DROP, host-namespace-only, and the FEATURE-event handler transitions.tests/caclmgrd/caclmgrd_redfish_acl_test.pyupdated for the renamed handler and still passes.Hardware validation
Validated by manually hand-patching this
caclmgrdchange onto a running DUT (no image rebuild).bjw-can-720dt-2(720dt,mx),dhcp_serverenabled.rsyslogdlistens on240.127.1.1:2514, thedocker0IP).caclmgrdprograms its catch-all-A INPUT -j DROP.Results:
docker exec dhcp_server logger ...reached host/var/log/syslog; thedocker0ACCEPT rule showed packet hits (2 pkts / 214 B).config feature state dhcp_server disabledremoves the rule;enabledre-adds it. caclmgrd logs both transitions:INPUT), thedocker0/2514 ACCEPT rule is still present at position 3, above the catch-all DROP.Which release branch to backport (provide reason below if selected)
Backported as a coordinated set with sonic-buildimage#28328 (step 1) and #28580 (step 3) so the branch is never left with neither the container nor caclmgrd owning the rule.
Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO): Microsoft ADO 38745011
Failure type: day-one issue (the container-owned docker0 syslog rule never survived a caclmgrd control-plane ACL rebuild)
Tested branch
Test result
bjw-can-720dt-2(720dt,mx); see "Hardware validation" above (rule programmed above the catch-all DROP, end-to-end RELP syslog delivered, feature toggle add/remove, survives an unrelated control-plane ACL re-walk — bug #27584 fix). Unit teststests/caclmgrd/caclmgrd_dhcp_server_syslog_test.pyand the updatedcaclmgrd_redfish_acl_test.pypass.caclmgrdcode path is identical to 202605; a master-image run is the only remaining validation step.Description for the changelog
[caclmgrd] Own the dhcp_server docker0 syslog INPUT exception so it survives the flush-and-rebuild.