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/network-interfaces-scripted: fix dhcpcd when mac address set #87116

Closed
wants to merge 1 commit into from

Conversation

@goertzenator
Copy link
Contributor

goertzenator commented May 6, 2020

Fixes issue #74471 (dhcpcd is started before mac addresses are set)

Motivation for this change

I want my NixOS system to boot up with working networking.

Things done

I ran this patch against my installation at 20.03 and it corrected the issue. I attempted to verify against master but gave up after several hours of building.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.
- Fixes issue #74471 (dhcpcd is started before mac addresses are set)
Copy link
Contributor

flokli left a comment

eeeh… This is trying to mask a problem which we wouldn't have if we'd apply interface parameters as soon as the interfaces appear, via udev. Even if we'd merge this PR as-is and add this dependency, things could still fail during reconfiguration.

I made the scripts below scripted-networking only in #85170, as running these scripts for networkd definitely was wrong, and I was too lazy to think about how to make scripted networking better, as I mostly use the networkd codepaths,

However, mac address and MTU should be perfectly fine configurable via a systemd.network.links directive even in the non-networkd case (as it's handled by udev, not networkd) since #82941.

The only line in the script that would need to be left would be the line bringing up the interface.

@goertzenator
Copy link
Contributor Author

goertzenator commented May 15, 2020

Thanks, I was finally able to try your suggested work-around but it did not work for me. Possibly because I'm working on a bridge that may not yet exist when systemd wants to set its MAC address.

I should probably attempt a pure networkd setup to see if that works for me, but I have some learning to do before attempting that. (I wasn't even aware of it until your last message).

@flokli
Copy link
Contributor

flokli commented May 17, 2020

Hm - the systemd.link files should trigger every time a new device appears, and not just on a specific point in time - so it should be way more robust.

I'll send a PR shortly that will make use of this for scripted networking as well, feedback appreciated.

flokli added a commit to flokli/nixpkgs that referenced this pull request May 17, 2020
…MTUBytes

The `network-link-${i.name}` units raced with other things trying to
configure the interface, or ran before the interface was available.

Instead of running our own set of shell scripts on boot, and hoping
they're executed at the right time, we can make use of udev to configure
the interface *while they appear*, by providing `.link` files in
/etc/systemd/network/*.link to set MACAddress and MTUBytes.

This doesn't require networkd to be enabled, and is populated properly
on non-networkd systems since
NixOS#82941.

This continues clean-up work done in
NixOS#85170 for the scripted networking
stack.

The only leftover part of the `network-link-${i.name}` unit (bringing
the interface up) is moved to the beginning of the
`network-addresses-${i.name}` unit.

Fixes: NixOS#74471
Closes: NixOS#87116
@flokli
Copy link
Contributor

flokli commented May 17, 2020

@goertzenator PR is at #88032.

flokli added a commit to flokli/nixpkgs that referenced this pull request May 21, 2020
…MTUBytes

The `network-link-${i.name}` units raced with other things trying to
configure the interface, or ran before the interface was available.

Instead of running our own set of shell scripts on boot, and hoping
they're executed at the right time, we can make use of udev to configure
the interface *while they appear*, by providing `.link` files in
/etc/systemd/network/*.link to set MACAddress and MTUBytes.

This doesn't require networkd to be enabled, and is populated properly
on non-networkd systems since
NixOS#82941.

This continues clean-up work done in
NixOS#85170 for the scripted networking
stack.

The only leftover part of the `network-link-${i.name}` unit (bringing
the interface up) is moved to the beginning of the
`network-addresses-${i.name}` unit.

Fixes: NixOS#74471
Closes: NixOS#87116
flokli added a commit to flokli/nixpkgs that referenced this pull request May 22, 2020
…MTUBytes

The `network-link-${i.name}` units raced with other things trying to
configure the interface, or ran before the interface was available.

Instead of running our own set of shell scripts on boot, and hoping
they're executed at the right time, we can make use of udev to configure
the interface *while they appear*, by providing `.link` files in
/etc/systemd/network/*.link to set MACAddress and MTUBytes.

This doesn't require networkd to be enabled, and is populated properly
on non-networkd systems since
NixOS#82941.

This continues clean-up work done in
NixOS#85170 for the scripted networking
stack.

The only leftover part of the `network-link-${i.name}` unit (bringing
the interface up) is moved to the beginning of the
`network-addresses-${i.name}` unit.

Fixes: NixOS#74471
Closes: NixOS#87116
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

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