Skip to content

Conversation

@hedrok
Copy link
Contributor

@hedrok hedrok commented Aug 5, 2025

Change summary

Fixes various problems with routes via dhcp gateway by moving responsibility to FRR.

First PR to vyos-build must be merged: vyos/vyos-build#1004

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • 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)

Related PR(s)

vyos/vyos-build#1004

How to test / Smoketest result

Create topology of VyOS DHCP client and VyOS DHCP server.
Add static route for client:

set protocols static route 10.1.2.0/24 dhcp-interface 'eth0'

Stop link between client-server, reboot client. Wait a little. Start link between client-server.
Before fix no route is added. After fix route is added.

Checklist:

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

@github-actions
Copy link

github-actions bot commented Aug 5, 2025

👍
No issues in PR Title / Commit Title

First
https://github.com/hedrok/vyos-build/pull/new/T5811-add-frr-dhcp-gateway
must be merged, as this change requires new FRR route
type:

ip route 10.1.2.0/24 dhcp-gateway eth0

That is added in the PR to vyos-build.

This commit mitigates various problems that arise from changing of DHCP
gateway for an interface and no updates of routes accordingly.
@hedrok hedrok force-pushed the T5811-static-dhcp-routes-reboot branch from b8dce51 to 0cf21cf Compare August 5, 2025 09:56
@hedrok hedrok marked this pull request as ready for review August 5, 2025 11:25
@github-actions
Copy link

CI integration ❌ failed!

Details

CI logs

  • CLI Smoketests (no interfaces) ❌ failed
  • CLI Smoketests (interfaces only) ❌ failed
  • Config tests ❌ failed
  • RAID1 tests ❌ failed
  • TPM tests ❌ failed

@sever-sever sever-sever requested a review from Copilot October 1, 2025 21:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves DHCP gateway address lookup responsibility from the VyOS configuration layer to FRR (Free Range Routing), fixing issues with routes via DHCP gateway not being properly maintained when DHCP state changes.

  • Replaces explicit DHCP router IP lookup with FRR's dhcp-gateway keyword
  • Adds a DHCP client exit hook to notify FRR when DHCP events occur
  • Removes template logic that resolved DHCP gateway IPs at configuration time

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/etc/dhcp/dhclient-exit-hooks.d/04-frr-dhcp-gateway-hook Adds DHCP client hook to notify FRR of DHCP state changes
data/templates/frr/staticd.frr.j2 Replaces hardcoded DHCP gateway IPs with FRR's dhcp-gateway keyword

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

# Copyright (C) 2025 VyOS Inc.
# Kyrylo Yatsenko

# Try to inform FRR of any events that can change ip or DHCP state
Copy link

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

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

The comment contains a grammatical error. 'ip' should be capitalized as 'IP' when referring to Internet Protocol.

Suggested change
# Try to inform FRR of any events that can change ip or DHCP state
# Try to inform FRR of any events that can change IP or DHCP state

Copilot uses AI. Check for mistakes.
@hedrok
Copy link
Contributor Author

hedrok commented Oct 16, 2025

I see merging into FRR upstream any dhcpgw feature quite improbable, starting developing VyOS-side solution, so close this PR.

@hedrok hedrok closed this Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

1 participant