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.

Sign up for GitHub

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

Jump to bottom

vyos.ifconfig: T5103: force dhclient restart on VRF change #4335

Merged
merged 3 commits into from
Feb 6, 2025
Merged

vyos.ifconfig: T5103: force dhclient restart on VRF change #4335

merged 3 commits into from
Feb 6, 2025

Conversation

c-po
Copy link
Member

@c-po c-po commented Feb 5, 2025 •
edited
Loading

Change summary

Moving an interface in, out or between VRFs will not re-install the received default route. This is because the dhclient binary is not restarted in the new VRF. Dhclient itself will report an error like: receive_packet failed on eth0.10: Network is down.

Take the return value of vyos.ifconfig.Interface().set_vrf() into account to forcefully restart the DHCP client process and optain a proper lease.

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)

How to test / Smoketest result

image

[email protected]:~$ TEST_ETH="eth1 eth2" /usr/libexec/vyos/tests/smoke/cli/test_interfaces_ethernet.py
test_add_multiple_ip_addresses (__main__.EthernetInterfaceTest.test_add_multiple_ip_addresses) ... ok
test_add_single_ip_address (__main__.EthernetInterfaceTest.test_add_single_ip_address) ... ok
test_add_to_invalid_vrf (__main__.EthernetInterfaceTest.test_add_to_invalid_vrf) ... ok
test_dhcp_client_options (__main__.EthernetInterfaceTest.test_dhcp_client_options) ... ok
test_dhcp_disable_interface (__main__.EthernetInterfaceTest.test_dhcp_disable_interface) ... ok
test_dhcp_vrf (__main__.EthernetInterfaceTest.test_dhcp_vrf) ... ok
test_dhcpv6_client_options (__main__.EthernetInterfaceTest.test_dhcpv6_client_options) ... ok
test_dhcpv6_vrf (__main__.EthernetInterfaceTest.test_dhcpv6_vrf) ... ok
test_dhcpv6pd_auto_sla_id (__main__.EthernetInterfaceTest.test_dhcpv6pd_auto_sla_id) ... ok
test_dhcpv6pd_manual_sla_id (__main__.EthernetInterfaceTest.test_dhcpv6pd_manual_sla_id) ... ok
test_eapol (__main__.EthernetInterfaceTest.test_eapol) ... ok
test_ethtool_evpn_uplink_tracking (__main__.EthernetInterfaceTest.test_ethtool_evpn_uplink_tracking) ... ok
test_ethtool_flow_control (__main__.EthernetInterfaceTest.test_ethtool_flow_control) ... ok
test_ethtool_ring_buffer (__main__.EthernetInterfaceTest.test_ethtool_ring_buffer) ... ok
test_interface_description (__main__.EthernetInterfaceTest.test_interface_description) ... ok
test_interface_disable (__main__.EthernetInterfaceTest.test_interface_disable) ... ok
test_interface_ip_options (__main__.EthernetInterfaceTest.test_interface_ip_options) ... ok
test_interface_ipv6_options (__main__.EthernetInterfaceTest.test_interface_ipv6_options) ... ok
test_interface_mtu (__main__.EthernetInterfaceTest.test_interface_mtu) ... ok
test_ipv6_link_local_address (__main__.EthernetInterfaceTest.test_ipv6_link_local_address) ... ok
test_move_interface_between_vrf_instances (__main__.EthernetInterfaceTest.test_move_interface_between_vrf_instances) ... ok
test_mtu_1200_no_ipv6_interface (__main__.EthernetInterfaceTest.test_mtu_1200_no_ipv6_interface) ... ok
test_non_existing_interface (__main__.EthernetInterfaceTest.test_non_existing_interface) ... ok
test_offloading_rfs (__main__.EthernetInterfaceTest.test_offloading_rfs) ... ok
test_offloading_rps (__main__.EthernetInterfaceTest.test_offloading_rps) ... ok
test_span_mirror (__main__.EthernetInterfaceTest.test_span_mirror) ... ok
test_speed_duplex_verify (__main__.EthernetInterfaceTest.test_speed_duplex_verify) ... ok
test_switchdev (__main__.EthernetInterfaceTest.test_switchdev) ... ok
test_vif_8021q_interfaces (__main__.EthernetInterfaceTest.test_vif_8021q_interfaces) ... ok
test_vif_8021q_lower_up_down (__main__.EthernetInterfaceTest.test_vif_8021q_lower_up_down) ... ok
test_vif_8021q_mtu_limits (__main__.EthernetInterfaceTest.test_vif_8021q_mtu_limits) ... ok
test_vif_8021q_qos_change (__main__.EthernetInterfaceTest.test_vif_8021q_qos_change) ... ok
test_vif_s_8021ad_vlan_interfaces (__main__.EthernetInterfaceTest.test_vif_s_8021ad_vlan_interfaces) ... ok
test_vif_s_protocol_change (__main__.EthernetInterfaceTest.test_vif_s_protocol_change) ... ok

----------------------------------------------------------------------
Ran 34 tests in 434.781s

OK

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

@c-po
vyos.ifconfig: T5103: force dhclient restart on VRF change
68002a3 
Moving an interface in, out or between VRFs will not re-install the received
default route. This is because the dhclient binary is not restarted in the new
VRF. Dhclient itself will report an error like: "receive_packet failed on
eth0.10: Network is down".

Take the return value of vyos.ifconfig.Interface().set_vrf() into account to
forcefully restart the DHCP client process and optain a proper lease.
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 5, 2025 •
edited
Loading

👍
No issues in PR Title / Commit Title

c-po added 2 commits February 5, 2025 23:12
@c-po
vyos.ifconfig: T7135: only restart DHCPv6 client if needed
bc4adcf 
Previously the DHCPv6 client was restarted on any change to the interface,
including changes only to the interface description. Re-use pattern from IPv4
DHCP to only restart the DHCP client if necessary.
@c-po
vyos.ifconfig: T5103: revert change to del_addr() signature
a04bd49 
An optional argument vrf_changed was added to the function signature but it
was not put to use. We only need to restart DHCP client on add_addr().
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 6, 2025

CI integration ❌ failed!

Details

CI logs

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

@c-po c-po merged commit edda97d into vyos:current Feb 6, 2025
14 of 16 checks passed
@c-po c-po deleted the T5103 branch February 8, 2025 10:32
@c-po c-po mentioned this pull request Feb 8, 2025
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jestabro jestabro jestabro approved these changes

@sever-sever sever-sever sever-sever approved these changes

@dmbaturin dmbaturin Awaiting requested review from dmbaturin

@sarthurdev sarthurdev Awaiting requested review from sarthurdev

@fett0 fett0 Awaiting requested review from fett0

@nicolas-fort nicolas-fort Awaiting requested review from nicolas-fort

@zdc zdc Awaiting requested review from zdc

Assignees

@c-po c-po

Labels
current
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@c-po @jestabro @sever-sever