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

Allow setting the regulatory domain (LP: #1951586) #281

Merged
merged 6 commits into from
Jul 4, 2022
Merged

Conversation

slyon
Copy link
Collaborator

@slyon slyon commented Jun 1, 2022

Description

Allow setting the regulatory domain, using a new regulatory-domain setting.
It is implemented in two ways:

  • country=XX setting in wpa_supplicant.conf (networkd/wpasupplicant only)
  • oneshot /run/systemd/system/netplan-regdom.service calling iw reg set XX (works globally)

Example:

network:
  version: 2
  renderer: networkd
  wifis:
    wlp2s0b1:
      regulatory-domain: "GB"
      dhcp4: no
      dhcp6: no
      addresses: [192.168.0.21/24]
      access-points:
        "MYSSID":
          password: 12345678

Checklist

  • Runs make check successfully.
  • Retains 100% code coverage (make check-coverage).
  • New/changed keys in YAML format are documented.
  • (Optional) Adds example YAML for new feature.
  • (Optional) Closes an open bug in Launchpad. LP#1951586

@slyon slyon changed the title Allow the regulatory domain Allow setting the regulatory domain (LP: #1951586) Jun 1, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jun 1, 2022

Codecov Report

Merging #281 (02abaf0) into main (6a47c70) will decrease coverage by 0.11%.
The diff coverage is 100.00%.

❗ Current head 02abaf0 differs from pull request most recent head c4768fb. Consider uploading reports for the commit c4768fb to get more accurate results

@@            Coverage Diff             @@
##             main     #281      +/-   ##
==========================================
- Coverage   99.19%   99.08%   -0.12%     
==========================================
  Files          61       61              
  Lines       11206    11267      +61     
==========================================
+ Hits        11116    11164      +48     
- Misses         90      103      +13     
Impacted Files Coverage Δ
netplan/cli/commands/apply.py 100.00% <ø> (ø)
src/netplan.c 100.00% <ø> (ø)
src/networkd.c 100.00% <100.00%> (ø)
src/parse.c 99.84% <100.00%> (+<0.01%) ⬆️
src/types.c 100.00% <100.00%> (ø)
tests/generator/test_wifis.py 100.00% <100.00%> (ø)
src/abi_compat.c 79.03% <0.00%> (-20.97%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a1ba54...c4768fb. Read the comment docs.

@slyon slyon added schema change This PR includes a change to netplan's YAML schema, which needs sign-off ABI-compatible ABI changed in a compatible way labels Jun 1, 2022
@slyon slyon force-pushed the slyon/regdom branch 2 times, most recently from 80aa57c to f2e5d42 Compare June 2, 2022 13:25
@slyon
Copy link
Collaborator Author

slyon commented Jun 2, 2022

@slyon slyon marked this pull request as ready for review June 2, 2022 13:42
@slyon slyon requested a review from waveform80 June 2, 2022 13:44
Copy link
Member

@waveform80 waveform80 left a comment

Choose a reason for hiding this comment

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

Customized an ubuntu 22.04 server arm64 image for pi with the packages from @slyon's PPA and it worked very nicely out of the box with regulatory-domain set in the network-config YAML on the boot partition (which becomes the netplan configuration via cloud-init).

The wpa-supplicant configuration wound up with country=GB within it, wifi came up happily on first boot, and an iw reg get after logging in confirmed that the regulatory domain was set correctly. Checking the boot logs showed that wpa-supplicant had set the regulatory domain prior to bringing up the interface and associating with the pre-configured AP.

All looks good to me!

@waveform80
Copy link
Member

(should add I also tested the reboot case: no issues to report)

@slyon slyon requested a review from murraybd June 7, 2022 14:00
doc/netplan.md Outdated

: This can be used to define the radio's regulatory domain, to make use of
additional WiFi channels outside the `00` "world domain". Takes an ISO /
IEC 3166 country code, such as `GB`.

Choose a reason for hiding this comment

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

Are all possible country codes supported as a value for the regulatory domain?

In terms of the schema, this looks fine. Expressing this in terms of country codes is sensible as that's what the backend uses and regulatory domains map to geopolitical entities. And the name 'regulatory-domain' is also sensible. If there are limits on which country codes are recognized, we should provide a pointer for that. (For my part, I wasn't able to even locate the database file described by regulatory.bin(5) on my system.)

Would we support '00' as a domain, to override a previous declaration and reset to the default?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch! There are 273 IEC 3166 country codes officially assigned (according to Wikipedia). wireless-regdb lists only 173 (+ "00") domains. So it's not all possible country code. I've added a reference to wireless-regdb in the documentation.

g_string_append(s, "After=network.target\n");
g_string_append(s, "ConditionFileIsExecutable="SBINDIR"/iw\n");
g_string_append(s, "\n[Service]\nType=oneshot\n");
g_string_append_printf(s, "ExecStart="SBINDIR"/iw reg set %s\n", def->regulatory_domain);

Choose a reason for hiding this comment

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

why do the changes to this source file show both a change to the output to the wpa supplicant config, and the addition of a systemd unit that invokes iw? (If we need iw, that appears to be a new runtime dependency; not ideal.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed, this was implemented in two ways:

  • wpa supplicant config (basic case): works only on the sd-networkd backend; works without a new iw runtime dependency
  • systemd service unit calling iw directly (extended case): works on all backends (incl NetworkManager); requires iw as a new runtime dependency

=> iw will be added as a Suggests: dependency. We don't need it necessarily, as the basic case (sd-systemd/wpa_supplicant) works as-is and the new systemd service will fail gracefully if iw is not available (ConditionFileIsExecutable=SBINDIR/iw).

I've updated the documentation accordingly.

@murraybd
Copy link

A test build for this is available in ppa:slyon/testing: https://launchpad.net/~slyon/+archive/ubuntu/netplan/+sourcepub/13655747/+listing-archive-extra

The ppa was actually slyon/netplan not slyon/testing.

@slyon
Copy link
Collaborator Author

slyon commented Jun 30, 2022

Brian confirmed out-of-band that using renderer: NetworkManager this is working as expected, too. Also, re-setting to regulatory-domain: 00 shows up in iw reg get as "global country 00", but is switching back automatically to "global country US" in his setup, most probably due to the raspi wifi firmware reacting to the AP's beacon advertising the country as US. The "switching back" behavior cannot be observed inside a mac80211_hwsim based qemu test.

Thank you for testing, Brian! This is all looking good and expected.
I rebased the branch and think it should be good for merging, if there are no other objections.

@slyon slyon merged commit e748158 into main Jul 4, 2022
@slyon slyon deleted the slyon/regdom branch July 4, 2022 08:39
maxnet added a commit to raspberrypi/rpi-imager that referenced this pull request Jul 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ABI-compatible ABI changed in a compatible way schema change This PR includes a change to netplan's YAML schema, which needs sign-off schema ok
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants