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

wireguard: allow routes to overlap with other routes #60818

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ryantrinkle
Copy link
Contributor

Motivation for this change

I like to use wireguard to maintain a constant connection to my office and my home network, but I'm frequently physically connected to those networks. Without this patch, starting wireguard would fail for whichever network I was physically connected to, because ip route replace didn't play nicely with the (correct) overlapping local routes.

This patch fixes this by explicitly deleting the routes for the wireguard interface, then re-adding the new ones (in case they have changed). I didn't make an attempt to avoid deleting and re-adding unchanged routes, because I figure people will expect restarting wireguard to entail a short downtime (and this is very short).

I also arbitrarily picked a metric of 10000 to ensure that traffic is directed to the local connection in preference to wireguard, when the local connection is available. I'm sure there's a better way of choosing a metric, but this works 100% reliably for me, and I suspect it will also for most others.

This is the kind of PR that often languishes, so if I don't get any pushback within the next week or so, I'll assume it's good and go ahead and merge.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • [N/A] macOS
    • [N/A] other Linux distributions
  • [N/A] Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • [N/A] Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • [N/A] Tested execution of all binary files (usually in ./result/bin/)
  • [N/A] Determined the impact on package closure size (by running nix path-info -S before and after)
  • [N/A] Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Previously, `ip route replace` was tripped-up by non-wireguard routes that overlap the wireguard routes.  This commit fixes that by using `ip route del` and `ip route add` separately, and also adds a metric of 10000 to avoid competing with local interfaces for traffic
@prusnak
Copy link
Member

prusnak commented Apr 29, 2020

This seems stalled.

@stale
Copy link

stale bot commented Jun 5, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 5, 2021
@otavio
Copy link
Contributor

otavio commented Feb 15, 2022

Is this still needed?

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Feb 15, 2022
@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 14, 2022
@wegank wegank marked this pull request as draft March 20, 2024 15:07
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants