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

dhcpd startup if all isolation/registration networks are remote #1565 #71

Merged

Conversation

candlerb
Copy link
Contributor

If dhcpd is answering remote requests only, i.e. there is no directly-
connected isolation/registration network, then add an empty subnet { }
declaration so that dhcpd will still listen on the interface.

…se-inc#1565

If dhcpd is answering remote requests only, i.e. there is no directly-
connected isolation/registration network, then add an empty subnet { }
declaration so that dhcpd will still listen on the interface.
@fgaudreault
Copy link

That patch appears to also listen on the management interface, which can lead to many problems on some deployment. Exclude the management interface from the listen.

Again, this patch appears to fix your case where you try to run PF on 1 interface. That will cause many regressions to people running PF on multiple interfaces. This is a NO GO for me.

@candlerb
Copy link
Contributor Author

candlerb commented Oct 1, 2012

The patch makes no change to what interfaces dhcpd listens on, nor does it do anything with management interfaces.

  • In lib/pf/services.pm, $service_launchers{'dhcpd'} appends @listen_ints as the list of interfaces for dhcpd to listen on.
  • In lib/pf/config.pm, @listen_ints is set to every "internal" interface (except those which match /:\d+$/, i.e. IP aliases). Management interfaces are included in @dhcplistener_ints but not in @listen_ints.
  • The patch just ensures there is at least one subnet {} declaration for every interface in @listen_ints

Hence I can see no possibility for regression. Management interfaces remain excluded from dhcpd.conf, and they remain excluded from the dhcpd command line.

You are correct that I am suggesting that with this patch, you can configure a single interface to be of type "management" and "internal". The GUI already allows this - it has a multi-selection box.

If you object to this, can you explain what the problem is?

@fgaudreault
Copy link

Fair enough. Does someone have time to test this in lab? If it works, we will merge.

@fdurand
Copy link
Member

fdurand commented Oct 5, 2012

I have tested and seems to be ok

fgaudreault pushed a commit that referenced this pull request Oct 11, 2012
dhcpd startup if all isolation/registration networks are remote #1565
@fgaudreault fgaudreault merged commit 0bd32b2 into inverse-inc:stable Oct 11, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants