Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/networkd: reimplement useDHCP in a sensible way #167327

Merged
merged 10 commits into from May 6, 2022

Conversation

lheckemann
Copy link
Member

@lheckemann lheckemann commented Apr 5, 2022

Description of changes

cc people I've talked about this with: @fpletz @Ma27 @globin @WilliButz

This seems like a decent default, with the caveat that the wait-for-online service will not succeed. This can be fixed by setting useDHCP on a specific interface, and doing so is still recommended for this reason.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable 👍

I guess we should also revert the changes in nixos-generate-config then, correct?

Also not sure about the state of networkd-docs in NixOS, but if there are any, this should be noted there as well.

@lheckemann
Copy link
Member Author

revert the changes in nixos-generate-config

I think it still makes sense to generate config for known interfaces in order to reduce the impact of the wait-online issue.

@Ma27
Copy link
Member

Ma27 commented Apr 25, 2022

I think it still makes sense to generate config for known interfaces in order to reduce the impact of the wait-online issue.

I haven't used scripted networking in quite a while, but doesn't that hang if an interface from networking.interfaces is not available? THis is e.g. an issue if I have plugged in a smartphone for USB tethering, run nixos-generate-config, the tethering-eth iface is part of hardware-configuration.nix. When I plug it off and reboot, it potentially hangs, correct? See for instance #107908.

If we already go down that route, networking.useDHCP = true; in nixos-generate-config is IMHO the more sensible default. If somebody knows their network well-enough they can still apply some customizations.

@lheckemann
Copy link
Member Author

The problem with networking.useDHCP = true in the general case is that a device with multiple matching interfaces (e.g. a laptop with WiFi and Ethernet, which is fairly common) will never reach an "online" state if only one of the interfaces is used (which is also fairly common), even though it's perfectly usable and "online" from a user's perspective. I'd say that useDHCP should indeed default to true, but we should also generate per-interface config and comment that.

@Ma27
Copy link
Member

Ma27 commented Apr 25, 2022

I'd say that useDHCP should indeed default to true, but we should also generate per-interface config and comment that.

Would be OK with that. Also, my concerns only apply to the use of scripted networking, how nixos-generate-config should behave with networkd being the default can be discussed when it's time :)

@Ma27
Copy link
Member

Ma27 commented Apr 29, 2022

I'd like to see this in 22.05 as this is an important step towards networkd adoption.

@Ma27 Ma27 added this to the 22.05 milestone Apr 29, 2022
Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I'd like to see this implemented:

I'd say that useDHCP should indeed default to true, but we should also generate per-interface config and comment that.

@lheckemann do you want to apply the remaining tasks or do you want me to take over? :)

lheckemann and others added 8 commits April 30, 2022 00:30
…DHCP = true;`

Previously this wasn't done in the `forEach`-expression for
`cfg.interfaces` and thus `networking.useDHCP` didn't have any effect if
no further interface was statically configured.
According to `systemd.network(5)` of systemd v249 this is a valid
option.

Fixes evaluation of the wildcard network definitions.
Currently we're still using scripted networking by default. A problem
with scripted networking is that having `useDHCP` on potentially
non-existing interfaces (e.g. an ethernet interface for USB tethering)
can cause the boot to hang.

Closes NixOS#107908
@lheckemann
Copy link
Member Author

lheckemann commented Apr 30, 2022

I think we need to have another look at the test, because in spite of some glaring issues (as you've pointed out), it was passing!?

EDIT: Oh, I see, you fixed that too :D

# Enables DHCP on each ethernet and wireless interface. In case of scripted networking
# (the default) this is the recommended approach. When using systemd-networkd it's
# still possible to use this option, but it's recommended to explicitly set it
# per-interface using `networking.interfaces.<interface>.useDHCP`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say that the optimal configuration is to have both when using networkd: explicit useDHCP per interface for the ones that should be configured for network-online.target, because they're the ones usually used for internet access, and global useDHCP for any extra interfaces that are used occasionally. This is what nixos-generate-config should generate IMHO.

@Ma27 Ma27 merged commit f0bb39d into NixOS:master May 6, 2022
@lheckemann lheckemann deleted the networkd-usedhcp branch May 7, 2022 08:44
@vcunat
Copy link
Member

vcunat commented May 23, 2022

FYI, I see:

dhcpcd[38576]: read_config: /etc/dhcpcd.conf: No such file or directory

I think the only relevant option I have is auto-generated:

  networking.useDHCP = lib.mkDefault true;

(in particular, no networkd)

EDIT: I'm also suspicious of some lesser regression in behavior since 21.11 (in this config), but I haven't had time to look deeper so far.

Comment on lines +92 to +96
# We set RequiredForOnline to false, because it's fairly
# common for such devices to have multiple interfaces and
# only one of them to be connected (e.g. a laptop with
# ethernet and WiFi interfaces). Maybe one day networkd will
# support "any"-style RequiredForOnline...
Copy link
Member

@ncfavier ncfavier Jun 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does now! systemd.network.wait-online.anyInterface = true;

#178046

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/plans-for-the-networkd-infrastructure/23086/3

ivanbrennan added a commit to ivanbrennan/nixbox that referenced this pull request May 16, 2023
Configuring dhcp on a per-interface basis is problematic if an interface
isn't present at boot time.

NixOS/nixpkgs#167327
ivanbrennan added a commit to ivanbrennan/nixbox that referenced this pull request May 18, 2023
Configuring dhcp on a per-interface basis is problematic if an interface
isn't present at boot time.

NixOS/nixpkgs#167327
@jfly
Copy link
Contributor

jfly commented Jan 3, 2024

with the caveat that the wait-for-online service will not succeed

What service is this referring to? Is it systemd-networkd-wait-online.service?

jfly added a commit to jfly/snow that referenced this pull request Jan 13, 2024
Make aragorn act more like a dumb ap should (disable dhcp for ipv4 and
ipv6). As a nice side effect, this seems to unbreak ipv6 browsing.

Also remove legacy commands about `networking.useDHCP` being
deprecacted. It no longer is:
NixOS/nixpkgs#167327.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants