vyos-1x icon indicating copy to clipboard operation
vyos-1x copied to clipboard

T5032: Add VRRP aware dhcp relay

Open devon-mar opened this issue 2 years ago • 7 comments

Change Summary

Add the ability to disable DHCP relay on interfaces not in the VRRP MASTER state.

This feature can be enabled with set service dhcp-relay vrrp-disable.

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [X] New feature (non-breaking change which adds functionality)
  • [ ] Code style update (formatting, renaming)
  • [ ] Refactoring (no functional changes)
  • [ ] Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • [ ] Other (please describe):

Related Task(s)

  • https://vyos.dev/T5032

Component(s) name

  • dhcp-relay
  • vrrp

Proposed changes

Add the ability to disable DHCP relay on interfaces not in the VRRP MASTER state.

This feature can be enabled with set service dhcp-relay vrrp-disable.

How to test

  1. Configure VRRP between two routers
  2. Configure DHCP relay on one of the VRRP interfaces
  3. The router in the BACKUP state should have the isc-dhcp-relay service stopped.
  4. The router in the MASTER state should have the isc-dhcp-relay service running.
  5. Test fail over between the routers and check the isc-dhcp-relay service state.

Checklist:

  • [X] I have read the CONTRIBUTING document
  • [X] I have linked this PR to one or more Phabricator Task(s)
  • [X] I have run the components SMOKETESTS if applicable
  • [X] My commit headlines contain a valid Task id
  • [ ] My change requires a change to the documentation
  • [ ] I have updated the documentation accordingly

devon-mar avatar Feb 24 '23 19:02 devon-mar

This feels very "hacky", is there a reason to do so?

Does it not work/is there an issue? Does the DHCP server see the request two times? From MASTER and BACKUP router?

c-po avatar Feb 26 '23 20:02 c-po

This feels very "hacky", is there a reason to do so?

I've implemented it using the same method as #887. Currently there's no way to disable a particular DHCP relay listen interface so this can't be accomplished with transition scripts.

Does it not work/is there an issue? Does the DHCP server see the request two times? From MASTER and BACKUP router?

Yes, the DHCP server ends up receiving packets from both routers.

devon-mar avatar Mar 01 '23 06:03 devon-mar

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Jul 19 '23 07:07 github-actions[bot]

This feels very "hacky", is there a reason to do so?

I've implemented it using the same method as #887. Currently there's no way to disable a particular DHCP relay listen interface so this can't be accomplished with transition scripts.

Does it not work/is there an issue? Does the DHCP server see the request two times? From MASTER and BACKUP router?

Yes, the DHCP server ends up receiving packets from both routers.

With dhcp-relay disable option, is this PR still valid? Now you may disable relays using transition scripts as requested

nicolas-fort avatar Aug 07 '23 10:08 nicolas-fort

@devon-mar update please PR as it has conflicts

sever-sever avatar Jan 05 '24 09:01 sever-sever

Conflicts have been resolved. A maintainer will review the pull request shortly.

github-actions[bot] avatar Jan 11 '24 04:01 github-actions[bot]

With dhcp-relay disable option, is this PR still valid? Now you may disable relays using transition scripts as requested

For my use case, it does. I can use the disable option as all dhcp relay interfaces are part of a vrrp group, but this wouldn't work if only some dhcp-relay interfaces were part of a vrrp group.

The transition script method also has the disadvantage of wiping out config history if many transitions occur.

update please PR as it has conflicts

I've resolved conflicts and tested it.

devon-mar avatar Jan 11 '24 20:01 devon-mar

@devon-mar Does the option set service dhcp-relay disable work for this case? It will be better than using "hacking" solutions.

sever-sever avatar Mar 31 '24 12:03 sever-sever

The implementation looks hacky I'd recommend using set service dhcp-relay disable Reopen this PR if required, and disable does not work in your case. I close it.

sever-sever avatar Apr 09 '24 17:04 sever-sever

@sever-sever

The implementation looks hacky

I do agree that it is "hacky." I implemented it based on how mdns works with VRRP, so that should probably be refactored at some point to use a less hacky solution.

It's also surprising that mdns has VRRP integration, but DHCP doesn't, given that DHCP is more commonly used, especially in enterprise environments.

I'd recommend using set service dhcp-relay disable

set service dhcp-relay disable works for me since all DHCP relay interfaces are also using VRRP. This wouldn't work though if only certain interfaces used VRRP.

This also has the downside of wiping out config history since each VRRP transition will create a new config.

devon-mar avatar Apr 09 '24 18:04 devon-mar