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/boot: Make sure interfaces are renamed in stage-1 #68953

Closed
wants to merge 2 commits into from

Conversation

@arianvp
Copy link
Member

@arianvp arianvp commented Sep 17, 2019

Otherwise we run into issues that udev cannot rename the interface in
stage-2 because the device is busy.

See :

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

Otherwise we run into issues that udev cannot rename the interface in
stage-2 because the device is busy.

See :
* #39329
* https://discourse.nixos.org/t/predictable-network-interface-names-in-initrd/4055
* 1f03f6f
@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Sep 17, 2019

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/4

renaming

this will respect the kernel parameter that disables predictible naming
I think, so I think the nixos Option for disabling predictible naming
should still work
@arianvp
Copy link
Member Author

@arianvp arianvp commented Sep 17, 2019

This needs a thorough review and testing. But I think we should try to get this into 19.09 as now the network setup stuff is a bit flakey

@arianvp
Copy link
Member Author

@arianvp arianvp commented Sep 17, 2019

Also not sure if this is the right approach. I'm still very confused how all this udev magic comes together. Because currently if networking.useNetworkd = false then stage-2 doesn't have an /etc/systemd/network/99-default.link file , but from what I read from the manpages from systemd, we need this file present regardless if we're using networkd or not, as it is a file that is read by udev, not networkd. (See https://www.freedesktop.org/software/systemd/man/systemd.link.html for more info.

So I think we should also copy this file over to the stage-2 /etc directory otherwise no renaming happens in stage-2 either.

But that begs the question, in the current scenario, why is stage-2 udev trying to rename if there is no 99-default.link to read the config from?

Edit:

I think udev will also look in $out/systemd/network for link files, at least in stage-2. Though this is a hunch. I'm building a version of master with debug logging enabled for udev to investigate a bit more.

@@ -209,6 +216,8 @@ let
cp -v ${udev}/lib/udev/rules.d/60-cdrom_id.rules $out/
cp -v ${udev}/lib/udev/rules.d/60-persistent-storage.rules $out/
cp -v ${udev}/lib/udev/rules.d/80-drivers.rules $out/
# Make sure interface is renamed in stage-1
cp -v ${udev}/lib/udev/rules.d/80-net-link-setup-link.rules $out/

This comment has been minimized.

@emilazy

emilazy Sep 17, 2019
Member

should be 80-net-setup-link.rules

@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Sep 17, 2019

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/5

@wizeman
Copy link
Member

@wizeman wizeman commented Oct 11, 2019

@arianvp With this PR, even after fixing the typo mentioned by @emilazy I was experiencing the problems you mentioned in the NixOS Discourse thread, namely:

  1. in stage-1 the network is still not renamed
  2. in stage-2 the renaming fails because the interface is busy

However, after I applied the changes in the following commit on top of your PR, everything seems to be finally fixed for me on NixOS 19.09:
wizeman@5d9d8fa

I don't even want to pretend to understand how udev works -- the idea about adding 75-net-descriptions.rules was taken from PR #47664.

Note that I have only done limited testing on 2 machines: one using a static network config with networking enabled in the initrd and another one using NetworkManager (with networking disabled in the initrd). AFAIK I'm not using systemd-networkd.

@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Oct 11, 2019

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/9

@arianvp
Copy link
Member Author

@arianvp arianvp commented Oct 11, 2019

@fpletz has a branch lying around that is a better implementation of this one. We worked on it at All Systems Go. @fpletz did you push this work somewhere?

@wizeman
Copy link
Member

@wizeman wizeman commented Oct 11, 2019

It looks like even with my changes, one of my machines still failed to rename the network interface, even in stage 2. I don't know what's different about this machine that causes this to fail...

@emilazy
Copy link
Member

@emilazy emilazy commented Oct 27, 2019

@fpletz ping?

Would love to see this fixed for 20.03.

@flokli
Copy link
Contributor

@flokli flokli commented Oct 29, 2019

@arianvp this seems somewhat redundant to #47664.

@erikarvstedt, can we close the other one, or should more be moved into here?

@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Dec 8, 2019

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

https://discourse.nixos.org/t/disk-encryption-on-nixos-servers-how-when-to-unlock/5030/8

@ju1m
Copy link
Contributor

@ju1m ju1m commented Jan 12, 2020

For nixos-19.09pre-git (Loris), I've come up with this module config (importable in a configuration.nix) with explanations inlined (beware that I'm no udev's nor systemd's well informed user — I've only run a few tests on a single machine (APU2) and read a few docs and comments, instead of going to bed — so I may have missed or misunderstood things, but it works for me™ to decrypt my ZFS root over SSH):

# Use predictable interface names in stage-1 and stage-2.
# DOC: https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/
#
# Tip: names that can be given using ID_NET_NAME_* envvars
# can be checked before hand with:
# udevadm test-builtin net_id /sys/class/net/*

{ pkgs, lib, config, ... }:
let udevNetSetupLinkRules = pkgs.writeTextFile {
    name = "80-net-setup-link.rules";
    destination = "/etc/udev/rules.d/80-net-setup-link.rules";
    text = ''
      SUBSYSTEM!="net", GOTO="net_setup_link_end"

      IMPORT{builtin}="path_id"

      ACTION!="add", GOTO="net_setup_link_end"

      # Load net_setup_link to setup the ID_NET_NAME_* envvars
      IMPORT{builtin}="net_setup_link"

      # Rename eth* using the "path" name policy (eg. enp1s0),
      # Note that in stage-1 the envvar ID_NET_NAME is not set,
      # hence not usable as in $${pkgs.systemd}/lib/udev/rules.d/80-net-setup-link.rules
      # Because in stage-1 there is no /etc/systemd/network/*.link
      # nor **/systemd/network/99-default.link
      # to set NamePolicy= which is responsible to set ID_NET_NAME.
      # Not sure if ATTR{type}=="1" and KERNEL=="eth*" are equivalent or not.
      ATTR{type}=="1", KERNEL=="eth*", NAME="$env{ID_NET_NAME_PATH}"

      LABEL="net_setup_link_end"
    '';
  };
in
{
  networking = {
    # Currently no-op.
    # false would set boot.kernelParams = [ "net.ifnames=0" ];
    # to disable NamePolicy= in *.link.
    usePredictableInterfaceNames = true;
  };

  boot.initrd = {
    extraUdevRulesCommands = ''
      # Query hwdb to set some more ID_* in case someone need them for their rules.
      cp -v ${pkgs.systemd}/lib/udev/rules.d/75-net-description.rules $out/

      # The name set here in stage-1 by 80-net-setup-link.rules
      # will stay in stage-2 (at least until the device is removed/added).
      cp -v ${udevNetSetupLinkRules}/etc/udev/rules.d/80-net-setup-link.rules $out/
    '';
  };

  services.udev.packages = [
    # Only useful here in stage-2 if the device is removed and re-added
    # (eg. the network module is rmmod-ed then modprobe-d).
    # The stage-1 (or initrd) is only a pivot_root after all,
    # it does not reload the kernel, hence passing to stage-2
    # does not trigger ACTION=="add" for the net devices.
    udevNetSetupLinkRules
  ];

  /* Useless block, only here for explanations.

  # NixOS put this .link only in the root filesystem, not in the initrd
  # hence it's only active in stage-2, not stage-1.
  # And even in stage-2, the 80-net-setup-link.rules has priority.
  # DOC: https://www.freedesktop.org/software/systemd/man/systemd.link.html
  environment.etc."systemd/network/79-net-setup.link".text = ''
    [Match]
    OriginalName=*

    [Link]
    #NamePolicy=keep kernel database onboard slot path
    NamePolicy=mac
    MACAddressPolicy=persistent
  '';
  */
}
@arianvp
Copy link
Member Author

@arianvp arianvp commented Jan 13, 2020

This PR is unfinished and probably not correct.

I and @fpletz briefly hacked on this during All-systems-go conf and our work is in this branch https://github.com/NixOS/nixpkgs/tree/fix-predictable-ifnames-in-initrd

@emilazy
Copy link
Member

@emilazy emilazy commented Jan 17, 2020

@ju1m This works perfectly for me, thank you! I'd love to see this cleaned up and merged upstream -- presumably some part of NixOS's existing udev setup is incorrect?

@emilazy emilazy mentioned this pull request Feb 8, 2020
6 of 10 tasks complete
@flokli
Copy link
Contributor

@flokli flokli commented Feb 13, 2020

Can this be closed in favor of #79532?

@arianvp arianvp closed this Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.