Skip to content

kernelPatches: ath driver: allow setting regulatory domain - #108725

Merged
symphorien merged 1 commit into
NixOS:masterfrom
veehaitch:ath_regd_optional
Jan 30, 2021
Merged

kernelPatches: ath driver: allow setting regulatory domain#108725
symphorien merged 1 commit into
NixOS:masterfrom
veehaitch:ath_regd_optional

Conversation

@veehaitch

@veehaitch veehaitch commented Jan 7, 2021

Copy link
Copy Markdown
Member

Ports an OpenWRT patch for Atheros wireless drivers (ath*) which allows
the user to change the regulatory domain code to the one which actually
applies.

All Atheros devices have a regulatory domain burned into their EEPROM.
When using a device as AP, this domain is frequently overly restrictive
when compared to the regulation which applies in the country the device
actually operates in; often, this restriction disallows IR on all
channels making it impossible to use the device as an AP at all.

This commit introduces the NixOS config option
networking.wireless.athUserRegulatoryDomain which, if enabled, applies
the patch and sets the kernel config option ATH_USER_REGD.

The original OpenWRT patch targets Linux 5.8.

Tested with a Compex WLE900VX (Qualcomm Atheros QCA986x/988x 802.11ac)
on NixOS master running Linux 5.10.5.

Motivation for this change
Things done
  • 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.

@ofborg ofborg Bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Jan 7, 2021
Comment on lines 8 to 10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you use structuredExtraConfig instead? Like this:

structuredExtraConfig = with lib.kernel; {
PREEMPT_RT = yes;
# Fix error: unused option: PREEMPT_RT.
EXPERT = yes; # PREEMPT_RT depends on it (in kernel/Kconfig.preempt)
# Fix error: option not set correctly: PREEMPT_VOLUNTARY (wanted 'y', got 'n').
PREEMPT_VOLUNTARY = lib.mkForce no; # PREEMPT_RT deselects it.
# Fix error: unused option: RT_GROUP_SCHED.
RT_GROUP_SCHED = lib.mkForce (option no); # Removed by sched-disable-rt-group-sched-on-rt.patch.
} // structuredExtraConfig;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for your comment! Your example, however, refers to a derivation of an entire kernel rather than an implementation of a mere patch. Therefore, I'm not sure how to adopt this approach. At least, according to the documentation, a kernelPatch doesn't have an attribute structuredExtraConfig. Could you elaborate please?

Ports an OpenWRT patch for Atheros wireless drivers (ath*) which allows
the user to change the regulatory domain code to the one which actually
applies.

All Atheros devices have a regulatory domain burned into their EEPROM.
When using a device as AP, this domain is frequently overly restrictive
when compared to the regulation which applies in the country the device
actually operates in; often, this restriction disallows IR on all
channels making it impossible to use the device as an AP at all.

This commit introduces the NixOS config option
networking.wireless.athUserRegulatoryDomain which, if enabled, applies
the patch and sets the kernel config option ATH_USER_REGD.

The original OpenWRT patch targets Linux 5.8.
@SuperSandro2000

Copy link
Copy Markdown
Member

This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch)
If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 108725 run on x86_64-linux 1

1 package blacklisted:
  • tests.nixos-functions.nixos-test

@veehaitch

Copy link
Copy Markdown
Member Author

This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch)
If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 108725 run on x86_64-linux 1
1 package blacklisted:

I'm not sure what this output means for this PR. Please elaborate.

@symphorien

Copy link
Copy Markdown
Member

I wonder if adding an option is the right thing to do. pkgs already contains a number of kernel patches as packages. It looks like it's possible to create such a package and instruct the user in meta.longDescription to add it to boot.kernelPatch.

I have no strong opinion on the question, I'm just raising the idea.

@veehaitch

Copy link
Copy Markdown
Member Author

I wonder if adding an option is the right thing to do. pkgs already contains a number of kernel patches as packages. It looks like it's possible to create such a package and instruct the user in meta.longDescription to add it to boot.kernelPatch.

I have no strong opinion on the question, I'm just raising the idea.

Thanks for your comment! Actually, I was just wondering what's the most straight-forward way to enable this patch from a user's perspective. I see your point though. Do you have an example of a package that follows your proposal?

@symphorien

symphorien commented Jan 9, 2021

Copy link
Copy Markdown
Member

They are in the kernelPatches attribute. However you would have to add recurseIntoAttrs on this attribute to make the patches visible in the package search.

@veehaitch

Copy link
Copy Markdown
Member Author

In fact, the patch of this PR is already added to the kernelPatches attribute. I'm happy to adapt this PR to expose those kernel patches in the package search, if this is what the community wants.

@SuperSandro2000

Copy link
Copy Markdown
Member

I'm not sure what this output means for this PR. Please elaborate.

nixpkgs-review does not build tests.

@symphorien

Copy link
Copy Markdown
Member

I wonder if adding an option is the right thing to do. pkgs already contains a number of kernel patches as packages. It looks like it's possible to create such a package and instruct the user in meta.longDescription to add it to boot.kernelPatch.

After some thought, it will probably not work because a patch is not a derivation and will not be listed in the package search. So let's forget about this.


A vm built with

{ config, pkgs, lib, ...}: {
  config = {
    users.users.root.initialPassword = "root";
    console = {
      font = "Lat2-Terminus16";
      keyMap = "fr";
    };
    boot.kernelParams = [ "systemd.log_level=debug" ];
    networking.wireless.athUserRegulatoryDomain =true;
    boot.kernelPackages = pkgs.linuxPackages_latest;
  };
}

compiles and boots, so it looks good to me.

cc @NeQuissimus for a second, more informed, opinion.

If noone complains I'll merge next week or the week after.

@NeQuissimus

Copy link
Copy Markdown
Member

One thing that is going to be tricky here is that sometimes kernel patches break with minor versions, sometimes with patch level releases.
So when you say this patch needs at least 5.8, I hope we can keep up with maintaining this patch :D
What if it continues to work with 5.8.x or 5.9.x (EOL'd, so no more updates here) but not an upcoming 5.10.x ?

@veehaitch

Copy link
Copy Markdown
Member Author

Good point! Actually, this patch has been part of OpenWRT long before 5.8 and I'd assume it also works for versions <5.8. I just introduced the "at least 5.8" requirement as this is the kernel version of OpenWRT for the commit the patch relates to and I didn't test any version before. I didn't consider it's worth it for the reasons you mentioned.

Regarding the compatibility with 5.10.x: No worries, it works as a charm so far :)

Tested with a Compex WLE900VX (Qualcomm Atheros QCA986x/988x 802.11ac) on NixOS master running Linux 5.10.5.

I depend on this patch (my wifi router runs NixOS). Therefore, I can commit to maintain this patch (best-effort, of course).

@NeQuissimus

Copy link
Copy Markdown
Member

Then I shall have no objections :D
I assume there is no way to write a test here without actual hardware...

@veehaitch

Copy link
Copy Markdown
Member Author

The best test I can think of is compiling the latest kernel with this patch applied, boot it, and execute zcat /proc/config.gz | grep -q CONFIG_ATH_USER_REGD=y

@symphorien

Copy link
Copy Markdown
Member

The patch is really short in fact, and only makes some functions return true. A conflict is not impossible but I this this explains that "this patch has been part of OpenWRT long before 5.8".

@symphorien
symphorien merged commit 04af7c0 into NixOS:master Jan 30, 2021
NeQuissimus pushed a commit that referenced this pull request Jan 31, 2021
Test for the option `networking.wireless.athUserRegulatoryDomain` to
make sure the underlying patch still applies for the latest kernel.

Also see #108725.
@johnjameswhitman

Copy link
Copy Markdown
Contributor

@veehaitch just wanted to say thank you for pulling in this patch. I spent about a week trying to track down why the ath10k card in a router I'm setting up wouldn't allow an AP on 5GHz despite setting the regdomain. I was about to try making my own patch and then saw you already got to it. This made the difference between getting 50-60mb/s on the 2.4GHz band to 700mb/s on 5GHz.

@PaulGrandperrin

Copy link
Copy Markdown
Contributor

@veehaitch thanks too! It's great to be able to use NixOS with the nice patches from OpenWRT!

Unfortunatly, I found about this after I created a custom package for patching the module (and not the whole kernel, which would take forever on my machine: https://github.com/PaulGrandperrin/nix-systems/blob/main/packages/kernel-module-ath-patched.nix)

Anyway, since this patch is basically trivial and already part of the NixOS tests (and therefor hydra's build closure), why not include it by default?

  • that's one less kernel build for hydra and in the cache
  • things will work by default for users without needing to discover this option first

CC @RaitoBezarius because he already changed some kernel conf to make ath easier to use by default in #239780.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants