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

[Backport release-21.05] nixos/wireless: make wireless.interfaces mandatory #125917

Merged
merged 1 commit into from
Jun 6, 2021

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jun 6, 2021

Description

Backport of #125288 to release-21.05.

This is the only way to solve issue #101963, for now.

(cherry picked from commit 030a521)
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jun 6, 2021
@rnhmjoj rnhmjoj merged commit 313fd79 into release-21.05 Jun 6, 2021
@dasJ
Copy link
Member

dasJ commented Jun 6, 2021

Was this absolutely necessary? For me it broke eval for a bunch of systems that were on a supposedly stable channel

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Jun 6, 2021

Unfortunately yes, without specifying an interface it's possible for the wpa_supplicant service to fail to start at boot, leaving the system without network. I failed to fix this before the 21.05 release date, sorry.

@dasJ
Copy link
Member

dasJ commented Jun 6, 2021

What about https://hydra.nixos.org/jobset/nixos/release-21.05-small#tabs-errors ? This blocks all channels

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Jun 6, 2021

It was reported in #125288 too. I don't know what's going on: the assertions should only ever be evaluated if networking.wireless is enabled.

@dasJ
Copy link
Member

dasJ commented Jun 6, 2021

It is enabled because it's an installation device? You said so yourself in #125288 (comment)

@jonringer jonringer deleted the backport-125288-to-release-21.05 branch June 6, 2021 23:50
@jonringer
Copy link
Contributor

maybe we can revert this, and just make it a warning?

Or add networking.wireless.enable = lib.mkForce false; suggestion to error.

cc @grahamc curious your opinion

@grahamc
Copy link
Member

grahamc commented Jun 7, 2021 via email

@grahamc
Copy link
Member

grahamc commented Jun 7, 2021 via email

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Jun 7, 2021

I opened #126075 to revert the change and make the assertion a warning.

I don’t necessarily know about or want to specify the interfaces I want wireless on.

Me too, the problem is wpa_supplicant isn't designed to work this way. This requires a hack (looping over /sys/class/net devices) , but it fails if the cards are not yet initialised. This wouldn't be much of an issue if we could automatically retry once the card is ready, however we can't restart the service from a udev rule because of a deadlock with systemd-udev-settle.

I think there really is no way around this. The proper solution would be to poll udev for a wireless card, either by patching wpa_supplicant or from the script preStart (I don't know if it's possible using udevadm only), but it's a lot of work.

I don’t think it is such a good idea to casually remove the feature of detecting interfaces.

It has a high chance of failure that leaves users without a network. On my machines wpa_supplicant failed to start consistently.
This is the motivation for hard failure.

@jonringer
Copy link
Contributor

Hydra is currently unable to finish an eval: https://hydra.nixos.org/jobset/nixos/release-21.05#tabs-evaluations so this should be changed (or hydra jobs need to be changed)

@ajs124
Copy link
Member

ajs124 commented Jun 7, 2021

@jonringer that's what @dasJ meant by "This blocks all channels". Currently at least unstable{,-small} and release-21.05{,-small} cannot advance because of this.

@jonringer
Copy link
Contributor

Yea, I just hadn't checked myself until recently.

rnhmjoj added a commit to rnhmjoj/nixpkgs that referenced this pull request Aug 11, 2021
I may have finally found a clean solution to the issues[1][2][3] with
the automatic discovery of wireless network interfaces.

[1]: NixOS#101963
[2]: NixOS#23196
[3]: NixOS#125917 (comment)

Currently the start script fails right away if no interface is available
by the time it's running, possibly leaving the system without network.
This happens when running a little early in the boot. A solution is to
instead wait for at least one interface to appear before scanning the
/sys/class/net/ directory. This is done here by listening for the right
udev events (from the net/wlan subsystem) using the `udevadm monitor`
command and grep to match its output.

This methods guarantees the availability of at least one interface to
wpa_supplicant, but won't add additional interfaces once it has started.
However, if the current interface is lost, say unplugged, the service is
automatically stopped and will be restarted as soon as a one (not
necessarily the same) is detected. It would be possible make this fully
dynamic by running another service that continously listen for udev
events and manages the main wpa_supplicant daemon, but this is probably
overkill.

I tested the following cases:

  - one interface, starting at boot, w/o predictable naming scheme
  - two interfaces, starting at boot (intel wireless and a usb adapter),
    w/o predictable naming scheme
  - one interface after the system booted, w/o predictable naming scheme
  - two interfaces after the system booted, w/o predictable naming scheme
  - unplugging and plugging back the current interface
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants