chore: optimize localdns provisioning polling and iptables setup#8401
Open
jingwenw15 wants to merge 5 commits intomainfrom
Open
chore: optimize localdns provisioning polling and iptables setup#8401jingwenw15 wants to merge 5 commits intomainfrom
jingwenw15 wants to merge 5 commits intomainfrom
Conversation
Reduce polling intervals from 1s to 0.1s in start_localdns() and wait_for_localdns_ready() since CoreDNS typically starts in <100ms. Batch iptables rules using iptables-restore instead of individual calls to avoid repeated xtables lock acquisition. Estimated total savings: ~1.3–2.7s during node provisioning. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jingwenw15
changed the title
jingwenw15
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets node provisioning latency in the Linux localdns bootstrap path by tightening readiness polling and batching iptables rule installation to reduce xtables lock contention.
Changes:
- Reduced polling sleep interval in
start_localdns()andwait_for_localdns_ready()from 1s to 0.1s, adjusting attempt counts accordingly. - Switched pod-traffic conntrack-skip rule installation to a single
iptables-restorecall instead of multipleiptablesinvocations. - Removed the previously used
IPTABLEScommand wrapper variable.
Update add_iptable_rules_to_skip_conntrack_from_pods tests to mock iptables-restore via PATH instead of the old IPTABLES variable, and assert on the new iptables-restore input format (*raw, -A rules, COMMIT). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
iptables requires -j (jump target) to be last in the rule. The comment match module must come before it, otherwise iptables-restore rejects the rule as invalid syntax. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Place -m comment immediately after chain name in iptables-restore input so that iptables -S displays comment before protocol match, matching the Cilium eBPF host routing regex. The previous ordering placed comment after --dport which caused nft backend to display it after the protocol match extension. Add ValidateLocalDNSIptablesRules e2e validator that checks: - localdns.sh uses iptables-restore (batched rules) - NOTRACK rules exist in both OUTPUT and PREROUTING chains - Comment tag is present for cleanup logic - NOTRACK is functional (no conntrack entries for localdns DNS traffic) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jingwenw15
requested review from
AbelHu,
Devinwong,
awesomenix,
cameronmeissner,
djsly,
ganeshkumarashok,
junjiezhang1997,
lilypan26,
mxj220,
phealy,
r2k1,
timmy-wright and
zachary-bailey
as code owners
jingwenw15
requested review from
calvin197,
pdamianov-dev,
sulixu and
surajssd
as code owners
Drop NOTRACK rules temporarily, do a DNS lookup, and verify conntrack entries appear. This proves the conntrack check is actually capable of detecting entries and isn't silently passing. Rules are restored after. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines
+1548
to
+1568
| saved_rules=$(sudo iptables -w -t raw -S | grep "localdns: skip conntrack") | ||
| sudo iptables -w -t raw -S | grep "localdns: skip conntrack" | while IFS= read -r rule; do | ||
| # Convert -A to -D to delete the rule | ||
| sudo iptables -w -t raw $(echo "$rule" | sed 's/^-A/-D/') 2>/dev/null || true | ||
| done | ||
|
|
||
| # Flush any leftover conntrack entries before the negative test | ||
| for ip in 169.254.10.10 169.254.10.11; do | ||
| sudo conntrack -D -d "$ip" -p udp --dport 53 2>/dev/null || true | ||
| done | ||
|
|
||
| # Do a DNS lookup without NOTRACK — this SHOULD create conntrack entries | ||
| dig bing.com @169.254.10.10 +short +timeout=2 +tries=1 > /dev/null 2>&1 || true | ||
|
|
||
| ct_dns_neg=$(sudo conntrack -L -d 169.254.10.10 -p udp --dport 53 2>/dev/null | wc -l) | ||
| echo "Conntrack entries for 169.254.10.10:53 without NOTRACK: $ct_dns_neg" | ||
|
|
||
| # Restore NOTRACK rules | ||
| echo "$saved_rules" | while IFS= read -r rule; do | ||
| sudo iptables -w -t raw $rule 2>/dev/null || true | ||
| done |
There was a problem hiding this comment.
The negative-test restore path builds iptables commands by expanding the iptables -S output into $rule unquoted (sudo iptables ... $rule). Since iptables -S prints the comment as --comment "localdns: skip conntrack", unquoted expansion will split that into multiple argv tokens (quotes become literal characters), so both deletion and restoration are likely to fail and make this validator flaky (and may leave rules removed). Consider deleting by rule number (using iptables -L --line-numbers like localdns.sh cleanup does) and restoring rules via a known-safe mechanism (e.g., iptables-restore payload or re-adding rules with properly quoted args), and don’t ignore restore errors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
start_localdns()andwait_for_localdns_ready()since CoreDNS typically starts in <100msiptables-restoreinstead of individual calls to avoid repeated xtables lock acquisitionIPTABLESvariableEstimated total savings: ~1.3–2.7s during node provisioning.
Telescope perf runs:
