RFC: hostapd: extend module to allow multiple APs. #49171

qolii · 2018-10-26T17:29:01Z

Motivation for this change

It is useful to be able to run multiple APs on a hostapd machine. The traditional ways to do this appear to be:

Run multiple instances of the daemon.
Run one instance of the daemon with multiple separate config files.

This change implements (1). I prefer this a bit over (2), because it gives each AP its own separate log in journald, and also means you can change configuration and restart just one AP at a time. I have found this to be more flexible in use.

I also have a version of this change implementing (2), if people feel strongly about that.

NOTE: This is a breaking change to the configuration format. My approach has been to take the existing single-AP configuration style, define a submodule around it, and modify the nixos module to take a list of this submodule called APs, as follows:

{
  enable = true;
  APs = [
    # 5GHz
    { interface = "wlp7s0";
      driver = "nl80211";
      ssid = "acbdef";
      hwMode = "a";
      wpa = true;
      wpaPassphrase = "12345";
      channel = 0;
      extraConfig = ''
        ...
      '';
    }
    # 2.4GHz
    { interface = "wlp5s0";
      driver = "nl80211";
      ssid = "abcdef";
      hwMode = "g";
      wpa = true;
      wpaPassphrase = "12345";
      channel = 0;
      extraConfig = ''
        ...
      '';
    }
  ];
}

This will result in two systemd service files, namely, hostapd-wlp7s0.service and hostapd-wlp5s0.service.

This works well, but will not accept any working existing configurations. I also could not figure out how to extend the hostapd entry in rename.nix. I expect these things are a big problem for the acceptance of this change, so if anyone has advice on how to achieve this more gently, please let me know.

systemd ordering

Upon making this change, I noticed failures at startup due to systemd ordering, because hostapd would try to start too early. I have thus reworked the ordering a little bit, and observe a completely reliable service on my machine. However, I am not an expert in all the different types of ways to do this, so any advice on this front is also very welcome.

In summary, this change has been very functional for me in testing.

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 nox --run "nox-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)
Fits CONTRIBUTING.md.

qolii · 2018-10-26T18:09:55Z

[Mentioning @wkennington and @Phreedom, as folks in the maintainers list for hostapd.]

qolii · 2018-10-27T20:03:18Z

A thought: given that we are faced with a breaking change anyway, would it be worthwhile to add a bridge field to the submodule, and then have hostapd service unit bindsTo and after the bridge's device unit? I don't know if there is currently an actual dependency on that, but I think there should be.

qolii · 2018-10-31T04:59:23Z

Well, I went for it. This actually lets me simplify the dependency list now that the bridge dependency is explicit. I rebooted my machine a bunch of times an observed stable bring-up of everything.

Since it's a separate commit, it will be easy to back out if there are issues.

flokli · 2019-06-22T20:33:43Z

@qolii Thanks for the PR! Sorry this went unnoticed for so long.

I gave this a try together with @NinjaTrappeur. We rebased your commits on the latest master, a more recent version of the branch can be found at https://github.com/flokli/nixpkgs/commits/hostapd-multiple-ifs.

I incorporated the systemd interface escaping that was done in master since your PR.

I do have more comments about it. Do you still want to merge this in? If yes, could you reset your PR to that version, so I can comment in here?

qolii · 2019-06-24T06:21:41Z

@flokli, thanks for getting back to me! This sounds great. Please give me another day or two, I'm a bit swamped at the moment.

flokli · 2019-06-24T08:04:08Z

sure :-)

qolii · 2019-07-03T05:49:09Z

Hi @flokli, so, what I did was remake the branch on my fork by cherry-picking the two commits from the branch on your fork. So I think those are now what you see here.

Is this what you had in mind?

If so, comment away! What are you thinking?

qolii · 2019-07-03T18:04:58Z

(And just fixed merge conflicts)

…systemd service file (and one instance of hostapd) for each AP.

…vice dependency.

qolii · 2019-07-08T07:09:29Z

(and just reinstated the noScan option that got stomped)

qolii · 2019-08-09T04:50:08Z

@flokli / @NinjaTrappeur, any thoughts?

flokli · 2019-08-09T13:38:06Z

@qolii this doesn't look too bad to me, but for the next two weeks, I won't have access to the hardware available to test it.

I remember @NinjaTrappeur mentioning a lot of hostapd options were still missing. Instead of adding all these, do you think it might make sense to refactor this to something like https://github.com/Infinisil/rfcs/blob/config-option/rfcs/0042-config-option.md describes?

qolii · 2019-08-13T06:03:11Z

@floli, I like the idea of doing a refactor. However, would you be amenable to merging this first (as I think this provides genuinely useful functionality), and doing that in a separate PR?

I'm happy to give the refactor a shot myself.

flokli

After reading through the hostapd.conf manpage once more, some comments.

The changed options also require a changelog entry.

flokli · 2019-08-17T16:28:33Z