Add dual-stack mDNS and Samba link handling#139
Conversation
Inventory IPv4 and IPv6 links for Samba and mDNS, keeping Samba bind tokens explicit while preserving IPv4 link-local support. Add UDP6 mDNS participation with per-link ff02::fb joins, scoped IPv6 sends, and link-valid A/AAAA advertising. Update watchdog, doctor probes, tests, manifests, and rebuilt NetBSD mdns/nbns artifacts.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces dual-stack (IPv4 and IPv6) support for Samba and mDNS services. Key changes include new C utility functions for IPv6 address manipulation and interface enumeration in auto-ip-common.inc, updated shell scripts for network discovery and service monitoring, and enhanced device probing logic to verify both IPv4 and IPv6 bindings. Feedback highlights a critical buffer over-read and limitations in the legacy ioctl-based interface enumeration, suggesting a move to getifaddrs(3). Additionally, the reviewer recommended avoiding hardcoded prefix fallbacks in IPv6 mask calculations and refactoring duplicated process binding check logic in the shell scripts.
Parse SIOCGIFCONF entries through a checked raw sockaddr view so NetBSD 4 IPv6 addresses are not truncated or overread. Use explicit IPv6 netmask ioctls, keep unknown prefixes out of Samba/AAAA output, normalize IPv4 link-local masks, and share fstat socket checks. Rebuild mdns/nbns artifacts and extend tests for raw ifconf entries, invalid masks, and socket scanning.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces dual-stack (IPv4 and IPv6) support for Samba and mDNS advertising. Key changes include a refactored interface enumeration system in C that supports IPv6 and link-local addresses, updated shell scripts for protocol-agnostic bind discovery and normalization, and enhanced probing logic to verify service listeners on both address families. Additionally, the manifest is updated with new binary hashes, and the test suite is expanded with dual-stack validation cases. I have no feedback to provide.
Fix Ubuntu CI portability by marking BSD-only ifreq parameters as intentionally unused on platforms that do not expose sockaddr lengths. Improve mdns-advertiser's runtime socket update path with rollback for partial multicast joins, IPv4 fallback when IPv6 transport setup fails, capacity truncation reporting, bounded stack-only log truncation, and known-answer suppression that does not hide other link-valid A/AAAA records. Rebuild mdns and nbns NetBSD artifacts and refresh the artifact manifest.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive dual-stack IPv4 and IPv6 support across the networking, service management, and diagnostic components. Key changes include refactoring interface enumeration and address validation in the shared C headers, updating boot scripts to manage IPv6 CIDRs and socket families, and enhancing the watchdog and device probing logic to monitor both address families. The advertiser binaries have been updated, and the test suite has been extensively expanded to verify the new functionality. I have no feedback to provide.
Plan mDNS responses from the active link inventory before applying known-answer suppression and packet writing. Keep broken per-link socket failures scoped to that link/family, preserve IPv4-only watchdog checks, and rebuild the affected NetBSD artifacts.
Handle service type enumeration, ANY-class questions, legacy unicast question sections, multicast response delays, and unicast hop limits in the planned RRset response path. Rebuild the NetBSD 6 and NetBSD 4 mDNS artifacts and expand tests for mDNS query routing, service enumeration, startup spacing, socket options, and NBSTAT response shape.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc422c0a13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces IPv6 support for Samba and mDNS services. Key changes include the addition of IPv6 address enumeration and filtering logic in the common C header, updates to shell scripts for handling IPv6 CIDRs and socket family probing, and expanded watchdog health checks to verify service bindings across both IPv4 and IPv6. Additionally, binary artifacts have been updated, and the test suite has been adjusted to reflect these improvements. I have no feedback to provide as there were no review comments to assess.
1 participant
Summary
Validation
.venv/bin/pytest(1062 passed, 9 skipped)./build/mdns.sh && ./build/mdnsoldle.sh && ./build/mdnsoldbe.sh && ./build/nbns.sh && ./build/nbnsoldle.sh && ./build/nbnsoldbe.shcp .env.backup6 .env && .venv/bin/tcapsule deploy --debug-logging --yes && .venv/bin/tcapsule doctordns-sd -G v4v6 jamess-airport-time-capsule.local: ULA AAAA, 192.168 A, and 169.254 A; no fe80 AAAA advertised.