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.

Sign up for GitHub

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

Jump to bottom

nixos/tinc: remove unnecessary networking.interfaces #48682

Merged
merged 1 commit into from Oct 19, 2018
Merged

nixos/tinc: remove unnecessary networking.interfaces #48682

merged 1 commit into from Oct 19, 2018

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Oct 18, 2018 •
edited

This breaks with both networking backends and
also creates large delays on boot when some services depends
on the network target. It is also not required
because tinc does create those interfaces itself.

fixes #27070

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

@Mic92
tinc: remove unnecessary networking.interfaces
5a1f0f9 
This breaks with networking backends enabled and
also creates large delays on boot when some services depends
on the network target. It is also not really required
because tinc does create those interfaces itself.

fixes NixOS#27070
@Mic92
Copy link
Member Author

Mic92 commented Oct 18, 2018

cc @volth @nh2

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: tinc

Partial log (click to expand)

these paths will be fetched (0.17 MiB download, 0.67 MiB unpacked):
  /nix/store/afxxrzwxfw94rjwygb3w0k6yzxxnhvjg-tinc-1.0.35
  /nix/store/m7ad6j9wibf50nsxmydbxcjr0rxibh8m-lzo-2.10
copying path '/nix/store/m7ad6j9wibf50nsxmydbxcjr0rxibh8m-lzo-2.10' from 'https://cache.nixos.org'...
copying path '/nix/store/afxxrzwxfw94rjwygb3w0k6yzxxnhvjg-tinc-1.0.35' from 'https://cache.nixos.org'...
/nix/store/afxxrzwxfw94rjwygb3w0k6yzxxnhvjg-tinc-1.0.35

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: tinc

Partial log (click to expand)

these paths will be fetched (0.18 MiB download, 0.68 MiB unpacked):
  /nix/store/5cz8hgr53pvh0r6w70l81c5il2qz3217-lzo-2.10
  /nix/store/kbv6bqf3x10qikkjwiqwgpynxyw6zgcl-tinc-1.0.35
copying path '/nix/store/5cz8hgr53pvh0r6w70l81c5il2qz3217-lzo-2.10' from 'https://cache.nixos.org'...
copying path '/nix/store/kbv6bqf3x10qikkjwiqwgpynxyw6zgcl-tinc-1.0.35' from 'https://cache.nixos.org'...
/nix/store/kbv6bqf3x10qikkjwiqwgpynxyw6zgcl-tinc-1.0.35

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: tinc

Partial log (click to expand)

make[2]: Nothing to be done for 'install-exec-am'.
make[2]: Nothing to be done for 'install-data-am'.
make[2]: Leaving directory '/private/tmp/nix-build-tinc-1.0.35.drv-0/tinc-1.0.35'
make[1]: Leaving directory '/private/tmp/nix-build-tinc-1.0.35.drv-0/tinc-1.0.35'
post-installation fixup
gzipping man pages under /nix/store/7jax3qbiprpdwmkaiip25as7v6z6d1lz-tinc-1.0.35/share/man/
strip is /nix/store/g5r4apl0za012ffs6ladinwa5w0m1l3k-cctools-binutils-darwin/bin/strip
stripping (with command strip and flags -S) in /nix/store/7jax3qbiprpdwmkaiip25as7v6z6d1lz-tinc-1.0.35/sbin
patching script interpreter paths in /nix/store/7jax3qbiprpdwmkaiip25as7v6z6d1lz-tinc-1.0.35
moving /nix/store/7jax3qbiprpdwmkaiip25as7v6z6d1lz-tinc-1.0.35/sbin/* to /nix/store/7jax3qbiprpdwmkaiip25as7v6z6d1lz-tinc-1.0.35/bin

@Mic92 Mic92 changed the title tinc: remove unnecessary networking.interfaces nixos/tinc: remove unnecessary networking.interfaces Oct 18, 2018
@Mic92 Mic92 merged commit 74a1b92 into NixOS:master Oct 19, 2018
@Mic92 Mic92 deleted the tincd branch October 19, 2018 07:33
@Mic92
Copy link
Member Author

Mic92 commented Oct 19, 2018

backport: cde886e

@minijackson
Copy link
Member

minijackson commented Oct 31, 2018 •
edited

Unfortunately this PR has broken some things for me:

since I used a configuration very similar to the one on the wiki via networking.interfaces, I had to add the virtual = true and virtualType = "tun" to the interface, since they were removed in this PR.

I personally prefer the network.interfaces way, since it's clearer than coding and putting the scripts in tinc-up and tinc-down.

I was taken by surprise for the backport, and it could have gotten bad if my server were to be rebooted, since it auto-updates, and I wouldn't be able to access it via the Tinc VPN :-/

One way to make it backwards compatible would be to add an option createInterface that defaults to true and add the interface with the virtual and virtualType options in it.

@minijackson
Copy link
Member

Oops, I didn't see that the backport was reverted, carry on ^^

@Mic92
Copy link
Member Author

Mic92 commented Oct 31, 2018 •
edited

At least 18.09-small has now the revert. I hope it did not affect to many setups.
This is the revert for reference: ea30555

@Mic92
Copy link
Member Author

Mic92 commented Oct 31, 2018 •
edited

The way it works with network.interfaces is very broken at the moment. We definitely need a better solution for this. At the moment it adds a device dependencies that just times out. In some setups there are even race condition between our network management creating the interface which leads to tinc failing to start.

@Mic92
Copy link
Member Author

Mic92 commented Oct 31, 2018

This is the setup I use on 4 nodes, which works well: https://github.com/Mic92/dotfiles/blob/de5e97c52f4ce2acd387f5614342b567d9a26870/nixos/vms/modules/retiolum.nix#L66

@Mic92
Copy link
Member Author

Mic92 commented Oct 31, 2018

@volth I also had problems when networkd was not managing tun interfaces. Correct me if I am wrong here. The difference was that I did not notice the bug adding ip addresses with network.interfaces as in my case networkd would set the link in an up state. I hope we find a solution where network.interfaces would just do that without waiting forever for a tun interface to appear or racing with tinc to create a tun interface.

@Mic92
Copy link
Member Author

Mic92 commented Oct 31, 2018

The question is should I revert the change on master as well or should I go forward to find a better solution?

@Mic92
Copy link
Member Author

Mic92 commented Nov 1, 2018 •
edited

Interestingly it also caused long delays on machines with gigabit ethernet and no dependencies on dns.
I also studied the code a bit and as I understood it, it seems to be the very first thing after creating tun/tap to also run tinc-up no matter if any other peer is connected or not: https://github.com/gsliepen/tinc/blob/228a03aaa707a5fcced9dd5148a4bdb7e5ef025b/src/net_setup.c#L866 So from my point of view tinc-up could be used for device setup at least also some people would prefer their existing modules

@nh2
Copy link
Contributor

nh2 commented Mar 2, 2019

I'm confused by various things here. One is that in

The issue is in Tinc FAQ

the mentioned error is under Platform specific questions and specifically says "on MacOSX or *BSD".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tinc -netdev systemd service can fail with ioctl(TUNSETIFF): Device or resource busy
4 participants
@Mic92 @GrahamcOfBorg @minijackson @nh2