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/networking: start dhcpcd even when defaultGateway6 is set #35064

Closed
wants to merge 1 commit into from

Conversation

rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Feb 17, 2018

Motivation for this change

Fix #34716

Things done

@@ -155,8 +155,7 @@ in

systemd.services.dhcpcd = let
cfgN = config.networking;
hasDefaultGatewaySet = (cfgN.defaultGateway != null && cfgN.defaultGateway.address != "")
|| (cfgN.defaultGateway6 != null && cfgN.defaultGateway6.address != "");
hasDefaultGatewaySet = (cfgN.defaultGateway != null && cfgN.defaultGateway.address != "");
Copy link
Member

Choose a reason for hiding this comment

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

That this is invalid too -- this would prevent starting dhcpcd for resolving IPv6 addresses before declaring a network online if IPv4 gateway is set. We probably want network-online.target to signify that both stacks are ready.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@abbradar you are perfectly right. I'm closing this in favour of #35141

Copy link
Member

@abbradar abbradar Feb 19, 2018

Choose a reason for hiding this comment

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

Nope, you actually have an advantage of yours: you also fix hasDefaultGatewaySet in network-interface-scripted.nix which I didn't know was there :3 I think it's better to unify those as a hidden NixOS option...

Copy link
Member

Choose a reason for hiding this comment

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

I'll do this in #35141.

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

3 participants