-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
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
networkd: disallow useDHCP #69302
networkd: disallow useDHCP #69302
Conversation
6616d2c
to
bead600
Compare
@volth Thanks for the hint. I would prefer to add a Note that this module already invokes udhcpc for interfaces that have Also note that initrd-network does neither use our scripted networking nor dhcpcd as we do later in the running system anyway. So for now I see no immediate reason to change the initrd-network to networkd even if we switch to it by default in stage2. |
@globin @andir Regarding 2b605e9, with this change we also can remove the As mentioned elsewhere this functionality is inherently broken without manually poking around some other network units anyway; for example to make stuff like |
Added release notes and information to the option docs. |
@@ -903,6 +903,11 @@ in | |||
Whether to use DHCP to obtain an IP address and other | |||
configuration for all network interfaces that are not manually | |||
configured. | |||
|
|||
Using this option is highly discouraged and also incompatible with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be a bit far fetched but how do we plan to proceed with installer images if we discourage usage of this? Wouldn't it be best practice to ship the installer with an encouraged configuration? We should be a good example to users and not just tell them to not use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently working on this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does networking.interfaces."en*".useDHCP = true
work? reason I ask is because setting a wildcard for matchConfig
is supported for networkd.
I guess then for the installer image we can then set networking.interfaces."en*".useDHCP = true
to get an IP address on all wired connections (seems like a sane default), but it won't try to ask for DHCP leases on bridges, veths etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO we shouldn't solve this in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andir I understood your comment as changing nixos-generate-config, from the conversation I've just had with @fpletz he misunderstood this, so I've added a commit to change nixos-generate-config. It adds networking.useDHCP = false;
and adds networking.interfaces.<name>.useDHCP = true;
for all interfaces is /sys/class/net
except for lo
. Virtual interfaces should not be present in our install media anyway.
This setting will be removed with the switch to systemd-networkd. The use of per interface config is encouraged instead.
This sets networking.useDHCP to false and for all interfaces found the per-interface useDHCP to true. This replicates the current default behaviour and prepares for the switch to networkd.
6aa0e98
to
5ee383e
Compare
@@ -72,6 +72,7 @@ let | |||
testCases = { | |||
loopback = { | |||
name = "Loopback"; | |||
machine.networking.useDHCP = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we just strip all lines setting networking.useDHCP = false;
in the tests, if we set it for each interface?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We now assert useDHCP
to be false in conjunction with networkd, I didn't want to change the whole test suite that for the most part doesn't use networkd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair - I misread and assumed these change networkd-based tests…
I meant to refer to any networkd-based tests hardcoding machine.networking.useDHCP
.
@GrahamcOfBorg test networking ipv6 |
This addresses a new assertion to the way NixOS handles networks with networkd [1]. As a result this disables `networking.useDHCP` globally on all my machines. Explicit interface configuration, as requested by [1] will be handled in separate commits due to separate machines, not all of which I have access to right now. [1] NixOS/nixpkgs#69302
Explicitly enable DHCP on interfaces as requested in [1]. [1]: NixOS/nixpkgs#69302
Looks like I was a bit hasty with merging this — it broke evaluation for the networkd variant of the |
The usage of DHCP is no longer global since a while[1] and we now have to explicitly enable it for the interfaces in question. This actually is a good thing and makes it far less problematic if we use tunnel interfaces and other more complicated networking configuration. I added the definitions for all machines where I actually know which interfaces are in use and disabled useNetworkd for shakti, because I don't know the interface names for that machine and the machine currently isn't in use anyway, so we can add it later if needed. [1]: NixOS/nixpkgs#69302 Signed-off-by: aszlig <aszlig@nix.build>
@lheckemann does that mean you reverted those changes for |
They are included. The test never ran with networkd, I simply added the |
I think this makes hotplugging network devices difficult. The interface name is different depending on which port you have. It takes a long time to boot up if it is missing too (1 min 30 sec by default). |
Not sure if I misunderstand you, but can't you use wildcards for that purpose in |
Concretely for the networkd case: {
systemd.network.networks."99-en-dhcp" = {
matchConfig.Name = "en*";
networkConfig.DHCP = "yes";
};
} to get DHCP on all ethernet devices. |
@lheckemann Thanks! That helps. Does nixos-generate-config (https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/installer/tools/nixos-generate-config.pl#L570-L586) need to hardcode these interfaces at all then? If one of those interfaces no longer exists, you get stuck waiting for "network-addresses-en*" or "network-addresses-wl*" to finish (or timeout at 1m 30s) as part of network-setup.service. This could happen frequently if you're switching between ethernet and wifi but have a USB adapter. |
A wildcard sounds fine to me instead of a fixed interface |
The problem with that approach is that it doesn't work with scripted networking, which is still our default. |
Opened #107908 to track this. |
Motivation for this change
This is preparation to switch to networkd by default in 20.03
Also intended for 19.09, does not change anything in the default config.
Things done
Notify maintainers
cc @fpletz @flokli @arianvp @disassembler @lheckemann