Skip to content

nixos/network-interfaces: Implement the ability to add arbitrary routes#3929

Closed
wkennington wants to merge 5 commits intoNixOS:masterfrom
wkennington:master.routes
Closed

nixos/network-interfaces: Implement the ability to add arbitrary routes#3929
wkennington wants to merge 5 commits intoNixOS:masterfrom
wkennington:master.routes

Conversation

@wkennington
Copy link
Contributor

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.

@wkennington wkennington force-pushed the master.routes branch 6 times, most recently from 71b0308 to e71558f Compare September 2, 2014 05:16
@wkennington
Copy link
Contributor Author

This should be in perfectly working order.

@robberer
Copy link
Contributor

robberer commented Sep 2, 2014

Is this, yet another great improvement of network-interfaces, ready for testing ?

@robberer
Copy link
Contributor

robberer commented Sep 2, 2014

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really needed? Nix is not a general-purpose language, so doing stuff like parsing IP addresses is likely to be quite slow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could add ip related types and functions to nix? It would be nice if I could sanitize things at evaluation time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to know the interface? With ip route add default you're not required to give an interface, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like
PartOf = mapAttrsToList (i: _: "${i}-cfg.service") cfg.network.interfaces
on the network-setup.service

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@robberer
Copy link
Contributor

robberer commented Sep 2, 2014

@edolstra how much slower is it compared to manually fixing lost routes ?

@wkennington
Copy link
Contributor Author

@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.

@wkennington wkennington force-pushed the master.routes branch 8 times, most recently from 9cdd018 to 6c35098 Compare September 2, 2014 16:16
@wkennington
Copy link
Contributor Author

@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.

@7c6f434c
Copy link
Member

What is the state of networkd conversion?

@wkennington
Copy link
Contributor Author

This is the next thing I'm working on starting tomorrow. I'm not far yet
but it's a fairly straightforward mapping from the current network
definitions. I'll keep this updated with more info.
On Nov 12, 2014 1:03 PM, "Michael Raskin" [email protected] wrote:

What is the state of networkd conversion?


Reply to this email directly or view it on GitHub
#3929 (comment).

@7c6f434c
Copy link
Member

@wkennington is this PR now obsolete? I think that using a fresh PR for a completely fresh implementation would benefit discoverability…

@wkennington
Copy link
Contributor Author

Definitely, I'll go ahead and close this one when I make the new one so I
can reference it.
On Nov 15, 2014 3:34 AM, "Michael Raskin" [email protected] wrote:

@wkennington https://github.com/wkennington is this PR now obsolete? I
think that using a fresh PR for a completely fresh implementation would
benefit discoverability…


Reply to this email directly or view it on GitHub
#3929 (comment).

@thoughtpolice thoughtpolice added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Mar 11, 2015
@jagajaga
Copy link
Member

So what about this PR?

@wkennington
Copy link
Contributor Author

It's completely out of date with the current networks interfaces but it
could be done.
On Mar 26, 2015 2:04 AM, "Arseniy Seroka" [email protected] wrote:

So what about this PR?


Reply to this email directly or view it on GitHub
#3929 (comment).

@rasendubi
Copy link
Member

(triage) @wkennington do you expect to work on this further?

@wkennington
Copy link
Contributor Author

Nope

On Mon, Jul 25, 2016, 4:07 PM Alexey Shmalko [email protected]
wrote:

(triage) @wkennington https://github.com/wkennington do you expect to
work on this further?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3929 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABsO_eA6o-L8JACuZeMdIdtSZ5X96BVjks5qZUHAgaJpZM4CdYH_
.

@rasendubi
Copy link
Member

I think this can be closed then.

@joachifm
Copy link
Contributor

Closing as abandoned.

@joachifm joachifm closed this Aug 20, 2016
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants