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

modules/avahi: Enable IPv6 by default #361016

Merged
merged 1 commit into from
Dec 7, 2024
Merged

Conversation

frederictobiasc
Copy link
Contributor

Avahi's default for use-ipv6 is yes as well. I see no reason why we should do this differently.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions 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/` labels Dec 2, 2024
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Dec 2, 2024
@frederictobiasc
Copy link
Contributor Author

frederictobiasc commented Dec 2, 2024

Does anyone know why the options nssmdns4 and nssmdns6 default to false as well?

IMO the whole point of enabling avahi is being able to resolve *.local domains and being resolveable as well. The currently required configuration to do that is big:

services.avahi = {
  enable = true;
  nssmdns6 = true;
  nssmdns4 = true;
  publish.addresses = true;
  publish.enable = true;
  ipv6 = true;
};

On other distributions like arch this just works™
Does anyone see any issue with making that the default apart from avahi.enable?

@fpletz
Copy link
Member

fpletz commented Dec 2, 2024

@frederictobiasc I agree that these options should probably also be turned on for a better user experience. Avahi is also being used to announce and discover the presence of services on the network via dbus. For instance, pipewire/pulseaudio is able to detect other instances/sinks over the network and make them available automatically. The nss module is not needed for this to work.

@fpletz
Copy link
Member

fpletz commented Dec 2, 2024

We also have to add release notes if we change the defaults.

@frederictobiasc
Copy link
Contributor Author

The nss module is not needed for this to work.

@fpletz But do you support enabling nss modules by default as well? I'll update the PR accordingly.

@fpletz
Copy link
Member

fpletz commented Dec 2, 2024

I just discovered #258424 where IPv6 was explicitly disabled. This doesn't make sense and IMHO shouldn't have been done on that PR.

@SuperSandro2000 Since it wasn't mentioned in that PR, do you recall the reasoning behind disabling IPv6?

But do you support enabling nss modules by default as well? I'll update the PR accordingly.

I would prefer another PR since IPv6 support itself should be less controversial.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 2, 2024
@github-actions github-actions bot added 8.has: documentation This PR adds or changes documentation 8.has: changelog labels Dec 2, 2024
@frederictobiasc
Copy link
Contributor Author

We also have to add release notes if we change the defaults.

Done!

I would prefer another PR since IPv6 support itself should be less controversial.

I opened #361191 for that.

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Dec 3, 2024

I would prefer another PR since IPv6 support itself should be less controversial.

It is super controversial and should stay turned off.

Since it wasn't mentioned in that PR, do you recall the reasoning behind disabling IPv6?

bba808d

#258424

https://github.com/avahi/nss-mdns#:~:text=in%20such%20a%20situation%20causes%20long%20timeouts%20when%20resolving%20hosts

I don't think anything has changed about this, or has it? If not this should absolutely stay turned off otherwise most people will wait long timeouts for no reason.

@fpletz
Copy link
Member

fpletz commented Dec 3, 2024

@SuperSandro2000 But as far as I understand the problem is NSS and not IPv6 support per se in Avahi, right? That's why I was advocating to split NSS into a different PR. IPv6 support shouldn't trigger the problem you encountered when only nssmdns4 is enabled as far as I understand it - please correct me if I'm wrong here. Note that we are not enabling NSS here.

I would personally love to have IPv6 support enabled for all applications we ship by default.

@SuperSandro2000
Copy link
Member

oh, true, not sure how this could be related or not to avahi itself 🤔

@frederictobiasc
Copy link
Contributor Author

Ok so it's fine to merge this now and continue the discussion regarding nss in #361191, correct?

@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 3, 2024
Avahi's default for `use-ipv6` is yes as well. I see no reason why we
should do this differently.
@fpletz fpletz merged commit be4a655 into NixOS:master Dec 7, 2024
31 checks passed
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: changelog 8.has: documentation This PR adds or changes documentation 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants