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

initrd.network: support predictable interface names #47664

Closed

Conversation

erikarvstedt
Copy link
Member

Copy of main commit msg:

When initrd.network.enable and networking.usePredictableInterfaceNames
are enabled, apply predictable interface naming in initrd.

This also fixes a bug:
Previously, setting initrd.network.enable effectively disabled option
networking.usePredictableInterfaceNames.
Reason: Interfaces brought up in initrd are not renamed by udev in boot
stage 2. So the unpredictable names assigned in initrd remained
unchanged after booting.

Implementation notes:
udhcpc uses eth0 as the default interface.
When predictable names are enabled, use the lexicographically
first Ethernet interface (en*) instead.

Things done:
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)

When `initrd.network.enable` and `networking.usePredictableInterfaceNames`
are enabled, apply predictable interface naming in initrd.

This also fixes a bug:
Previously, setting `initrd.network.enable` effectively disabled option
`networking.usePredictableInterfaceNames`.
Reason: Interfaces brought up in initrd are not renamed by udev in boot
stage 2. So the unpredictable names assigned in initrd remained
unchanged after booting.

Implementation notes:
udhcpc uses eth0 as the default interface.
When predictable names are enabled, use the lexicographically
first Ethernet interface (en*) instead.
@xeji
Copy link
Contributor

xeji commented Oct 2, 2018

I did that in #39329 but there was no consensus on how to avoid breaking existing configurations, so it wasn't merged yet.

@erikarvstedt
Copy link
Member Author

erikarvstedt commented Oct 2, 2018

Great, thanks for the heads-up.

I think some aspects of this PR are worth copying to your proposal:

  1. This tweak to make single-interface initrd DHCP work the same way regardless of usePredictableInterfaceNames being enabled or disabled.
    Most importantly this avoids forcing users to explicitly set the DHCP interface when predictable names are enabled and thus avoids breakage for upgrading users.
  2. Emphasize that this fixes another overt bug, namely initrd.network.enable unconditionally breaking networking.usePredictableInterfaceNames.
  3. Only copy the extra udev rules on initrd.network.enable. This keeps the initrd minimal and minimizes the chance of breaking existing configs. Edit: Ah, I see, we also need this for networkd.
  4. Maybe DRY up the predictable-interface-names test names like so.

@@ -76,6 +76,11 @@ in

boot.initrd.kernelModules = [ "af_packet" ];

boot.initrd.extraUdevRulesCommands = mkIf config.networking.usePredictableInterfaceNames ''
Copy link
Member

Choose a reason for hiding this comment

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

Can you write release notes for that?

Copy link
Member

Choose a reason for hiding this comment

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

This aligns more with the semantics of the option, but requires users to migrate.

Copy link
Contributor

Choose a reason for hiding this comment

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

#39329 contains some draft release notes, feel free to copy/adapt.

@xeji
Copy link
Contributor

xeji commented Oct 2, 2018

cc @samueldr @oxij who were involved in the previous discussion of #39329.

@mmahut
Copy link
Member

mmahut commented Aug 9, 2019

Any updates on this pull request, please?

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/predictable-network-interface-names-in-initrd/4055/1

@erikarvstedt
Copy link
Member Author

Superseded by #68953.

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

6 participants