Conversation
Fix spelling error: "avaailable" -> "available" Signed-off-by: Panchajanya1999 <kernel@panchajanya.dev>
- Rename to Canadian-Shield-Private to reflect the actual service tier - Fix IPs from 149.112.121.10/122.10 to 149.112.121.20/122.20 (previous IPs were duplicates of RethinkDNS entries) Signed-off-by: Panchajanya1999 <kernel@panchajanya.dev>
- Rename to Canadian-Shield-Private to reflect the actual service tier - Fix IPs from 149.112.121.10/122.10 to 149.112.121.20/122.20 (previous IPs were duplicates of RethinkDNS entries) Signed-off-by: Panchajanya1999 <kernel@panchajanya.dev>
Add command-line options for more flexible usage: -4, --ipv4-only Test only IPv4 DNS servers -6, --ipv6-only Test only IPv6 DNS servers -n, --count N Custom number of tests per server -q, --quick Quick mode (3 tests per server) --no-ping Skip ping correlation test --no-color Disable colored output -h, --help Show help message -v, --version Show version Also adds VERSION variable (1.0.0) for tracking releases. Signed-off-by: Panchajanya1999 <kernel@panchajanya.dev>
Add command-line options for more flexible usage: -4, --ipv4-only Test only IPv4 DNS servers -6, --ipv6-only Test only IPv6 DNS servers -n, --count N Custom number of tests per server -q, --quick Quick mode (3 tests per server) --no-ping Skip ping correlation test --no-color Disable colored output -h, --help Show help message -v, --version Show version Also adds VERSION variable (1.0.0) for tracking releases. Signed-off-by: Panchajanya1999 <kernel@panchajanya.dev>
Use process substitution (< <(...)) instead of piping to while loops. This keeps the loop in the main shell so variable modifications persist. Fixes the issue where ipv4_count, ipv6_count, and correlation counts were always 0 due to running in a subshell. Signed-off-by: Panchajanya1999 <kernel@panchajanya.dev>
Write sorted data to temp files and read from them instead of piping to while loops. This keeps the loop in the main shell so variable modifications (ipv4_count, ipv6_count, etc.) persist. ash/dash doesn't support process substitution, so temp files are used. Signed-off-by: Panchajanya1999 <kernel@panchajanya.dev>
Median is more robust than average for latency measurements as it's less affected by outliers (occasional slow queries due to network jitter, etc.). - Add calculate_median() function to both scripts - Update DNS test results to use median - Update table headers and labels to reflect the change Signed-off-by: Panchajanya1999 <kernel@panchajanya.dev> (cherry picked from commit 7d0c68d)
Remove the following DNS servers that were found to be unreachable or consistently failing: IPv4: - OpenBLD (46.151.208.154) - IIJ-Primary/Secondary (103.2.57.5, 103.2.58.5) - Foundation-Applied-Privacy (37.252.185.229, 37.252.185.232) - Restena (158.64.1.29) - Digitale-Gesellschaft-Primary/Secondary (185.95.218.42/43) - Switch-Primary/Secondary (130.59.31.248/251) - DNSPod-Primary (119.29.29.29) - UncensoredDNS-Primary/Secondary (91.239.100.100, 89.233.43.71) - DNS0.EU-Primary/Secondary (193.110.81.0, 185.253.5.0) - puntCAT (109.69.8.51) IPv6: - Digitale-Gesellschaft-Primary/Secondary-v6 - Switch-Primary/Secondary-v6 - UncensoredDNS-Primary/Secondary-v6 - DNS0.EU-Primary/Secondary-v6 - Restena-v6 - IIJ-Primary/Secondary-v6 Signed-off-by: Panchajanya1999 <kernel@panchajanya.dev> (cherry picked from commit 13a3cac)
- Add warmup query before testing to prime DNS server cache - Use single domain (google.com by default) for all tests - Add -d/--domain CLI option to customize test domain - Remove unused TEST_DOMAINS array (15 domains) This measures cached DNS response time rather than recursive resolution time, giving results that correlate with actual network latency to the DNS server. Signed-off-by: Panchajanya1999 <kernel@panchajanya.dev>
- Update server count from 60+ to 50+ (removed failed servers) - Add "How It Works" section explaining cache-aware benchmarking - Add comprehensive "Command Line Options" section with examples - Document -d/--domain flag for custom test domains - Document -4/-6 flags for IPv4/IPv6-only testing - Document -n/--count and -q/--quick flags - Update results table to show "Median" instead of "Avg Time" - Simplify "Advanced Usage" section Signed-off-by: Panchajanya1999 <kernel@panchajanya.dev>
Remove "FOR OPENWRT" from the recommendation header since pingerr.sh is for Linux/macOS systems, not OpenWRT (that's pingerr_ash.sh). Signed-off-by: Panchajanya1999 <kernel@panchajanya.dev>
There was a problem hiding this comment.
Pull request overview
This pull request significantly enhances the pingerr DNS benchmarking tool by adding flexible command-line options, improving the benchmarking methodology to use cache-aware testing with median calculations, and streamlining the DNS server list from 60+ to 50+ servers.
Changes:
- Added comprehensive CLI options (-4/-6 for IP version selection, -n for test count, -d for custom domain, -q for quick mode, --no-ping, --no-color, -h, -v)
- Switched from average to median for response time calculations with a warmup query approach that measures cached DNS response times
- Updated DNS server lists by removing outdated servers and consolidating the list to 50+ active servers
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| README.md | Updated documentation to reflect new CLI options, cache-aware testing methodology, command examples, and reduced server count (60+ to 50+) |
| pingerr.sh | Added CLI argument parsing, median calculation function, warmup query logic, IPv6 option handling, and loop restructuring to fix subshell variable scope issues |
| pingerr_ash.sh | Implemented same features as pingerr.sh with ash/POSIX-compatible syntax, including printf-based help output and file-based median calculation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Function to calculate median (POSIX-compatible, more robust than average) | ||
| calculate_median() { | ||
| local temp_file="/tmp/median_$$" | ||
| local count=0 | ||
| local val | ||
|
|
||
| # Write non-zero values to temp file for sorting | ||
| : > "$temp_file" | ||
| for val in "$@"; do | ||
| if [ "$val" != "0" ] && [ "$val" != "" ]; then | ||
| echo "$val" >> "$temp_file" | ||
| count=$((count + 1)) | ||
| fi | ||
| done | ||
|
|
||
| if [ $count -eq 0 ]; then | ||
| rm -f "$temp_file" | ||
| echo "9999" | ||
| return | ||
| fi | ||
|
|
||
| # Sort and get median | ||
| sort -n "$temp_file" > "${temp_file}.sorted" | ||
| local mid=$((count / 2)) | ||
|
|
||
| if [ $((count % 2)) -eq 0 ]; then | ||
| # Even count: average of two middle values | ||
| local val1=$(sed -n "${mid}p" "${temp_file}.sorted") | ||
| local val2=$(sed -n "$((mid + 1))p" "${temp_file}.sorted") | ||
| echo $(( (val1 + val2) / 2 )) | ||
| else | ||
| # Odd count: middle value | ||
| sed -n "$((mid + 1))p" "${temp_file}.sorted" | ||
| fi | ||
|
|
||
| rm -f "$temp_file" "${temp_file}.sorted" | ||
| } |
There was a problem hiding this comment.
The calculate_median function creates temporary files (/tmp/median_$$ and /tmp/median_$$.sorted) that are cleaned up at the end of the function. However, if the function is interrupted before completion (e.g., script killed), these files might not be cleaned up. Consider adding these patterns to the main trap handler or using a more robust cleanup mechanism.
| # Function to calculate median (POSIX-compatible, more robust than average) | ||
| calculate_median() { | ||
| local temp_file="/tmp/median_$$" | ||
| local count=0 | ||
| local val | ||
|
|
||
| # Write non-zero values to temp file for sorting | ||
| : > "$temp_file" | ||
| for val in "$@"; do | ||
| if [ "$val" != "0" ] && [ "$val" != "" ]; then | ||
| echo "$val" >> "$temp_file" | ||
| count=$((count + 1)) | ||
| fi | ||
| done | ||
|
|
||
| if [ $count -eq 0 ]; then | ||
| rm -f "$temp_file" | ||
| echo "9999" | ||
| return | ||
| fi | ||
|
|
||
| # Sort and get median | ||
| sort -n "$temp_file" > "${temp_file}.sorted" | ||
| local mid=$((count / 2)) | ||
|
|
||
| if [ $((count % 2)) -eq 0 ]; then | ||
| # Even count: average of two middle values | ||
| local val1=$(sed -n "${mid}p" "${temp_file}.sorted") | ||
| local val2=$(sed -n "$((mid + 1))p" "${temp_file}.sorted") | ||
| echo $(( (val1 + val2) / 2 )) | ||
| else | ||
| # Odd count: middle value | ||
| sed -n "$((mid + 1))p" "${temp_file}.sorted" | ||
| fi | ||
|
|
||
| rm -f "$temp_file" "${temp_file}.sorted" | ||
| } |
There was a problem hiding this comment.
The calculate_median function uses /tmp/median_$$ for temporary files. Since $$ is the parent shell's PID and this function is called from parallel background jobs (line 550), multiple concurrent calls could create a race condition where jobs overwrite each other's temp files. Consider using
pingerr.sh
Outdated
| if [ -n "$2" ] && [ "$2" -eq "$2" ] 2>/dev/null; then | ||
| TEST_COUNT=$2 | ||
| shift 2 | ||
| else | ||
| echo "Error: --count requires a numeric argument" |
There was a problem hiding this comment.
The validation for the --count argument only checks if the value is numeric but doesn't validate that it's a positive integer. Users could pass zero or negative values like --count 0 or --count -5, which would cause all DNS tests to fail. Consider adding validation to ensure TEST_COUNT is greater than zero.
| if [ -n "$2" ] && [ "$2" -eq "$2" ] 2>/dev/null; then | |
| TEST_COUNT=$2 | |
| shift 2 | |
| else | |
| echo "Error: --count requires a numeric argument" | |
| if [ -n "$2" ] && [ "$2" -eq "$2" ] 2>/dev/null && [ "$2" -gt 0 ] 2>/dev/null; then | |
| TEST_COUNT=$2 | |
| shift 2 | |
| else | |
| echo "Error: --count requires a positive numeric argument (greater than 0)" |
pingerr_ash.sh
Outdated
| if [ -n "$2" ] && [ "$2" -eq "$2" ] 2>/dev/null; then | ||
| TEST_COUNT=$2 | ||
| shift 2 | ||
| else | ||
| printf 'Error: --count requires a numeric argument\n' |
There was a problem hiding this comment.
The validation for the --count argument only checks if the value is numeric but doesn't validate that it's a positive integer. Users could pass zero or negative values like --count 0 or --count -5, which would cause all DNS tests to fail. Consider adding validation to ensure TEST_COUNT is greater than zero.
| if [ -n "$2" ] && [ "$2" -eq "$2" ] 2>/dev/null; then | |
| TEST_COUNT=$2 | |
| shift 2 | |
| else | |
| printf 'Error: --count requires a numeric argument\n' | |
| if [ -n "$2" ] && [ "$2" -eq "$2" ] 2>/dev/null && [ "$2" -gt 0 ]; then | |
| TEST_COUNT=$2 | |
| shift 2 | |
| else | |
| printf 'Error: --count requires a positive integer argument greater than zero\n' |
The validation for the --count only checks if the value is numeric but doesnt validates if the value is positive or not.
We don't need it anymore since coomit 6737d2f Signed-off-by: Panchajanya1999 <rsk52959@gmail.com>
This pull request introduces significant improvements to both the documentation and the code of
pingerr.sh, focusing on usability, accuracy, and maintainability. The highlights are the addition of flexible command-line options, a switch to cache-aware DNS benchmarking using median response times, and a streamlined DNS server list. The README has been updated to reflect these changes, providing clearer instructions and examples.Usability and CLI improvements:
pingerr.sh, allowing users to select IPv4/IPv6, set test count, choose test domain, enable quick mode, and control output formatting. The help and version flags are also included. [1] [2]--ipv4-onlyand--ipv6-only) and handles IPv6 connectivity checks more robustly. [1] [2]Benchmarking methodology improvements:
DNS server list maintenance:
Documentation and advanced usage:
Other improvements:
--versionflag for easy version tracking. [1] [2]These changes make
pingerr.shmore user-friendly, accurate, and maintainable, and ensure the documentation is up-to-date and helpful for both new and advanced users.