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

nixos/networkd: Add routes from interfaces to [Route] section of .network file #93635

Closed
wants to merge 1 commit into from

Conversation

duairc
Copy link
Contributor

@duairc duairc commented Jul 22, 2020

Motivation for this change

Similarly to my motivation for #93598, I want to improve consistency between when networking.useNetworkd is true and when it's false. In particular I want to be able to write a NixOS module that can abstract out a non-trivial networking setup but still be agnostic to whether the system uses networkd or not.

Currently any routes that are set in networking.interfaces.<interface>.routes are completely ignored when using networkd. This commit instead adds them to the systemd.network.networks.<network>.routes option of the corresponding networkd network.

One thing that's tricky is that there isn't a trivial mapping between the options in networking.interfaces.<interface>.routes, which allows you to specify interface, via and options (the latter is passed more or less directly to the ip route command) and systemd.network's [Route] options. I've not attempted to do this in a complete way, but I do look at some of the options attributes and try to do the most sensible thing with them. I chose the particular options I did simply because they were useful to me. The "proper" way to do this would probably be to move them to be top level options of networking.interfaces.<interface>.routes rather than untyped strings in the options attrset..

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.

@duairc
Copy link
Contributor Author

duairc commented Dec 28, 2020

I didn't mean to close this. I messed up a force-push and GitHub closed it on my behalf. I've fixed that but I can't seem to reopen it.

@erictapen erictapen reopened this Dec 28, 2020
@stale
Copy link

stale bot commented Jun 28, 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 28, 2021
@erictapen
Copy link
Member

I'm using this for over a year now on my systems and would like to have it merged.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 28, 2021
@andir
Copy link
Member

andir commented Jun 28, 2021

Do we have VM tests that exercise this code? If so I am in favor of merging.

@Luflosi
Copy link
Contributor

Luflosi commented Nov 4, 2021

I improved upon this PR in #144590.

@rnhmjoj rnhmjoj closed this in ca58bd0 Jan 20, 2022
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