nixos/network-interfaces: Implement the ability to add arbitrary routes#3929
nixos/network-interfaces: Implement the ability to add arbitrary routes#3929wkennington wants to merge 5 commits intoNixOS:masterfrom
Conversation
71b0308 to
e71558f
Compare
|
This should be in perfectly working order. |
e71558f to
ccb91ec
Compare
|
Is this, yet another great improvement of network-interfaces, ready for testing ? |
|
BTW: IMO "probably not quite working" is the wrong sentence. Should be "definitely not quite working". I wonder that nobody else is stumbled on missing default gateway and nixos-rebuild errors regarding static routes in networking.localCommands. |
There was a problem hiding this comment.
Is this really needed? Nix is not a general-purpose language, so doing stuff like parsing IP addresses is likely to be quite slow.
There was a problem hiding this comment.
Ideally I want to make the old defaultGateway option defunct as the routes interface is more general. If we don't support the defaultGateway option, we can remove all of this code. However, in order to support the old option we need to know which interface the default route belongs. The speed of this implementation will not affect users who do not use the defaultGateway option, so it would probably be wise to steer them to writing more general routes using a warning.
There was a problem hiding this comment.
Maybe we could add ip related types and functions to nix? It would be nice if I could sanitize things at evaluation time.
There was a problem hiding this comment.
Why do we need to know the interface? With ip route add default you're not required to give an interface, right?
There was a problem hiding this comment.
Correct, however it seems as though there were issues in the past with the default gateway updates through networking-setup.service not always being triggered. Is there a better system for triggering these updates as opposed to the scripted system we have now. Is there some way we could use bindTo or some similar systemd unit descriptor to trigger the reloads?
There was a problem hiding this comment.
Maybe something like
PartOf = mapAttrsToList (i: _: "${i}-cfg.service") cfg.network.interfaces
on the network-setup.service
There was a problem hiding this comment.
Maybe we can have a udev action triggered whenever an interface is added, which would call "ip route add default ...".
Or maybe we should look into systemd-network, which might take care of this for us in a presumably udev-friendly way.
There was a problem hiding this comment.
I would definitely be in favor of converting all of this to networkd. It seems more robust than any convoluted shell scripting we can put together. With the added bonus that it is easier to maintain.
There was a problem hiding this comment.
Were you planning on pushing the systemd-216 changes in the near future or should I attempt enabling networkd for 212? I suppose I could just base my work on the 216 branch.
There was a problem hiding this comment.
Yes, I'd like to merge it into staging after we've merged the current staging (today or so). 216 seems to break surprisingly few things :-)
But yeah, you can just use the 216 branch already.
|
@edolstra how much slower is it compared to manually fixing lost routes ? |
|
@robberer You won't encounter any of the slowdown related to that code if you don't use the defaultGateway option and instead use the new route options. |
9cdd018 to
6c35098
Compare
|
@robberer It should be fine to use this patch for the time being. However I'll be doing a complete conversion to networkd so this will likely never be mainlined. |
ef7c188 to
b2a40df
Compare
b2a40df to
ec939d8
Compare
ec939d8 to
05e1c29
Compare
|
What is the state of networkd conversion? |
|
This is the next thing I'm working on starting tomorrow. I'm not far yet
|
|
@wkennington is this PR now obsolete? I think that using a fresh PR for a completely fresh implementation would benefit discoverability… |
|
Definitely, I'll go ahead and close this one when I make the new one so I
|
|
So what about this PR? |
|
It's completely out of date with the current networks interfaces but it
|
|
(triage) @wkennington do you expect to work on this further? |
|
Nope On Mon, Jul 25, 2016, 4:07 PM Alexey Shmalko [email protected]
|
|
I think this can be closed then. |
|
Closing as abandoned. |
This is currently a work in progress as supporting default routes is probably not quite working. I'd appreciate testers when this code actually evaluates.