operating-system icon indicating copy to clipboard operation
operating-system copied to clipboard

add dnsmasq for NetworkManager shared network

Open jkt628 opened this issue 9 months ago • 14 comments
trafficstars

required by https://github.com/home-assistant/supervisor/pull/5577

Summary by CodeRabbit

  • New Features
    • Updated configuration settings to enable a lightweight DNS forwarder and DHCP server across supported platforms.
    • Disabled the TFTP functionality within the DNS service to streamline network operations and enhance security.

jkt628 avatar Feb 08 '25 11:02 jkt628

📝 Walkthrough

Walkthrough

This pull request introduces changes to two build configuration files in the external buildroot repository. In both the generic_x86_64_defconfig and rpi4_64_defconfig files, the DNSMasq package is enabled with the configuration option BR2_PACKAGE_DNSMASQ=y, while the TFTP feature is explicitly disabled with BR2_PACKAGE_DNSMASQ_TFTP=n. These modifications adjust the build configurations to include DNSMasq as a lightweight DNS forwarder and DHCP server without TFTP support.

Changes

File(s) Change Summary
buildroot…/generic_x86_64_defconfig, buildroot…/rpi4_64_defconfig Added new configuration options: BR2_PACKAGE_DNSMASQ=y and BR2_PACKAGE_DNSMASQ_TFTP=n

📜 Recent review details

Configuration used: CodeRabbit UI Review profile: CHILL Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 98efed87370b1c9c58a3c9c6f7b4358fa4bfd02b and 3e8484fd50d0917de2c5dc8b884938a1ae8a3be4.

📒 Files selected for processing (2)
  • buildroot-external/configs/generic_x86_64_defconfig (1 hunks)
  • buildroot-external/configs/rpi4_64_defconfig (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • buildroot-external/configs/rpi4_64_defconfig
  • buildroot-external/configs/generic_x86_64_defconfig

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Feb 08 '25 11:02 coderabbitai[bot]

Hm, ok so there is more than it than just Supervisor changes :cry:. It should be fairly low-risk to add dnsmasq. What is a bit concern is bloating. We already now struggle to fit things for generic-x86-64 :cry: .

We certainly should check that there is no init scripts or systemd units deployed. In a quick look the dhcpd package excludes init scripts when NetworkManager is selected, but I don't see this for dnsmasq :thinking:

agners avatar Feb 11 '25 09:02 agners

Hm, ok so there is more than it than just Supervisor changes 😢. It should be fairly low-risk to add dnsmasq. What is a bit concern is bloating. We already now struggle to fit things for generic-x86-64 😢 .

there are also https://github.com/home-assistant/cli/pull/533 and https://github.com/home-assistant/home-assistant.io/pull/37140. (i started on frontend yesterday but, even though i've coded for 50 years, i have never done a front end so i'm having to learn some new tricks. at least retirement is not being wasted!)

can you provide some parameters around "fit" for generic_x86_64? i have a system arriving today for testing custom HAOS so i can hopefully address this concern.

We certainly should check that there is no init scripts or systemd units deployed. In a quick look the dhcpd package excludes init scripts when NetworkManager is selected, but I don't see this for dnsmasq 🤔

on rpi4_64 the only references to dnsmasq are apparmor, dbus and NetworkManager. it runs only by NetworkManager, this taken from ps axf:

    506 ?        Ssl    0:59 /usr/sbin/NetworkManager --no-daemon
   9279 ?        S      0:00  \_ /usr/sbin/dnsmasq --conf-file=/dev/null --no-hosts --keep-in-foreground --bind-interfac

i should know more about generic_x86_64 soon.

jkt628 avatar Feb 11 '25 11:02 jkt628

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

home-assistant[bot] avatar Feb 11 '25 15:02 home-assistant[bot]

I guess you added only the platforms you were able to test,

correct, that is in the commit message.

however, the options should be added to all the board configs pre-emptively

i wondered about that. i'll update all the platforms and commit message, too.

jkt628 avatar Feb 11 '25 15:02 jkt628

What I wonder (but would have to be probably tested), would the DNS resolving, mDNS and whatnot work properly also for the clients?

i have connected my phone and tablet to the shared network and they seem to work fine. the only problem i can remember is frigate-nvr could not connect my cameras due to iptables / netfilter configuration, but that was with home-assistant/supervised-installer and easily fixed, i will have to test again with HAOS.

jkt628 avatar Feb 11 '25 17:02 jkt628

i should know more about generic_x86_64 soon.

initial report: not working, 😢 . i see the AP on my desktop and phone but cannot connect. i found this in the journal:

Feb 12 13:53:26 haos-x86-64 NetworkManager[423]: <warn>  [1739368406.3078] iptables: command /usr/sbin/iptables failed: Child process exited with code 1
Feb 12 13:53:26 haos-x86-64 NetworkManager[423]: <warn>  [1739368406.3132] iptables: command /usr/sbin/iptables failed: Child process exited with code 1
Feb 12 13:53:26 haos-x86-64 NetworkManager[423]: <warn>  [1739368406.3189] iptables: command /usr/sbin/iptables failed: Child process exited with code 1

but do not see similar on rpi4_64.

digging in....

jkt628 avatar Feb 12 '25 13:02 jkt628

turns out WPA3 is advertised but not working with two different WiFi adapters, and a third adapter is not supported at all. if i switch my clients to WPA2 they connect easily. my phone never connects because i cannot find how to disable WPA3.

i have looked to disable advertising WPA3 but have no luck so far.

jkt628 avatar Feb 23 '25 16:02 jkt628

i upgraded wpa_supplicant and my linux laptop and android phone connect properly.

jkt628 avatar Feb 26 '25 15:02 jkt628

@agners NetworkManager creates a rule that prevents addons running on docker, like frigate-nvr, from connecting to cameras on the private network.

table ip nm-shared-wlp3s0 { # handle 10
        chain nat_postrouting { # handle 1
                type nat hook postrouting priority srcnat; policy accept;
                ip saddr 10.42.0.0/24 ip daddr != 10.42.0.0/24 masquerade # handle 2
        }

        chain filter_forward { # handle 3
                type filter hook forward priority filter; policy accept;
                ip daddr 10.42.0.0/24 oifname "wlp3s0" ct state { established, related } accept # handle 5
                ip saddr 10.42.0.0/24 iifname "wlp3s0" accept # handle 6
                iifname "wlp3s0" oifname "wlp3s0" accept # handle 7
                iifname "wlp3s0" reject # handle 8
                oifname "wlp3s0" reject # handle 9
#             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^  this
        }
}

it is easily eliminated with nft, see below, but

i don't mind doing the work to import additional configuration via ha os import but i wonder if this is the best approach.

#!/usr/bin/env bash

INTERFACE="$1"
EVENT="$2"

case "$EVENT" in
	up)
		iptables -P FORWARD ACCEPT
		nft -a list ruleset |& sed -n '/oifname "$INTERFACE" reject/{s/.*#//;p;q}' | xargs -n1 nft delete rule "nm-shared-$INTERFACE" filter_forward
		;;
esac

jkt628 avatar Feb 26 '25 15:02 jkt628

with a necessary upgrade to wpa_supplicant this Practical Note is spot on.

jkt628 avatar Feb 27 '25 11:02 jkt628

@agners NetworkManager creates a rule that prevents addons running on docker, like frigate-nvr, from connecting to cameras on the private network.

Ugh yeah that sucks. I wonder if the firewalling can be influenced? I think rather than add nft and scripts which change the firewall settings, I'd rather just patch NetworkManager to do what we want (e.g. change how it inserts firewall rules).

From the NetworkManager's manual it seems that there are different firewall backends: https://networkmanager.pages.freedesktop.org/NetworkManager/NetworkManager/NetworkManager.conf.html#id-1.2.3.7

Maybe just setting it to none works for our use case? I guess it will route things transparently, even from the external interface, but maybe that is ok? :thinking:

agners avatar Mar 04 '25 10:03 agners

@agners i tried firewall-backend iptables and see a new table created with a similar problematic rule, but it fails, probably because it was not actually linked into PREROUTING.

jkt628 avatar Mar 04 '25 17:03 jkt628

@agners i tried firewall-backend iptables and see a new table created with a similar problematic rule, but it fails, probably because it was not actually linked into PREROUTING.

Did you also try none? I think routing should be enabled already, so that should work without any NM being involved.

agners avatar Mar 04 '25 20:03 agners

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

github-actions[bot] avatar Jun 24 '25 05:06 github-actions[bot]