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

interfaces: T6592: moving an interface between VRF instances failed #3834

Merged
merged 2 commits into from
Jul 20, 2024

Conversation

c-po
Copy link
Member

@c-po c-po commented Jul 20, 2024

Change Summary

To reproduce:

set vrf name mgmt table '150'
set vrf name no-mgmt table '151'
set interfaces ethernet eth2 vrf 'mgmt'
commit

set interfaces ethernet eth2 vrf no-mgmt
commit

This resulted in an error while interacting with nftables:
[Errno 1] failed to run command: nft add element inet vrf_zones ct_iface_map { "eth2" : 151 }

The reason is that the old mapping entry still exists and was not removed.

This commit adds a new utility function get_vrf_tableid() and compares the current and new VRF table IDs assigned to an interface. If the IDs do not match, the nftables ct_iface_map entry is removed before the new entry is added.

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)

Component(s) name

Proposed changes

How to test

Smoketest result

[email protected]:~$ /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_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_support (__main__.EthernetInterfaceTest.test_eapol_support) ... ok
test_ethtool_evpn_uplink_tarcking (__main__.EthernetInterfaceTest.test_ethtool_evpn_uplink_tarcking) ... 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 <--- NEW TEST FOR THIS BUG
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_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 32 tests in 132.495s

OK
[email protected]:~$ /usr/libexec/vyos/tests/smoke/cli/test_protocols_static.py
test_01_static (__main__.TestProtocolsStatic.test_01_static) ... ok
test_02_static_table (__main__.TestProtocolsStatic.test_02_static_table) ... ok
test_03_static_vrf (__main__.TestProtocolsStatic.test_03_static_vrf) ... ok

----------------------------------------------------------------------
Ran 3 tests in 31.304s

OK
[email protected]:~$ /usr/libexec/vyos/tests/smoke/cli/test_vrf.py
test_vrf_assign_interface (__main__.VRFTest.test_vrf_assign_interface) ... ok
test_vrf_bind_all (__main__.VRFTest.test_vrf_bind_all) ... ok
test_vrf_conntrack (__main__.VRFTest.test_vrf_conntrack) ... ok
test_vrf_disable_forwarding (__main__.VRFTest.test_vrf_disable_forwarding) ... ok
test_vrf_ip_ipv6_nht (__main__.VRFTest.test_vrf_ip_ipv6_nht) ... ok
test_vrf_ip_ipv6_protocol_non_existing_route_map (__main__.VRFTest.test_vrf_ip_ipv6_protocol_non_existing_route_map) ... ok
test_vrf_ip_protocol_route_map (__main__.VRFTest.test_vrf_ip_protocol_route_map) ... ok
test_vrf_ipv6_protocol_route_map (__main__.VRFTest.test_vrf_ipv6_protocol_route_map) ... ok
test_vrf_link_local_ip_addresses (__main__.VRFTest.test_vrf_link_local_ip_addresses) ... ok
test_vrf_loopbacks_ips (__main__.VRFTest.test_vrf_loopbacks_ips) ... ok
test_vrf_static_route (__main__.VRFTest.test_vrf_static_route) ... ok
test_vrf_table_id_is_unalterable (__main__.VRFTest.test_vrf_table_id_is_unalterable) ... ok
test_vrf_vni_add_change_remove (__main__.VRFTest.test_vrf_vni_add_change_remove) ... ok
test_vrf_vni_and_table_id (__main__.VRFTest.test_vrf_vni_and_table_id) ... ok
test_vrf_vni_duplicates (__main__.VRFTest.test_vrf_vni_duplicates) ... ok

----------------------------------------------------------------------
Ran 15 tests in 134.162s

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

Copy link

github-actions bot commented Jul 20, 2024


Commit title 'utils: migrate to new get_vrf_tableid() helper. . Commit 452068c ("interfaces: T6592: moving an interface between VRF instances. failed") introduced a new helper to retrieve the VRF table ID from the Kernel.. This commit migrates the old code path where the individual fields got queried. to the new helper vyos.utils.network.get_vrf_tableid().' does not match the required format!. Valid title example: T99999: make IPsec secure

Copy link

github-actions bot commented Jul 20, 2024

👍
No issues in unused-imports

@c-po c-po marked this pull request as draft July 20, 2024 08:56
@c-po c-po marked this pull request as ready for review July 20, 2024 08:58
@c-po c-po marked this pull request as draft July 20, 2024 09:33
c-po added 2 commits July 20, 2024 11:46
To reproduce:

    set vrf name mgmt table '150'
    set vrf name no-mgmt table '151'
    set interfaces ethernet eth2 vrf 'mgmt'
    commit

    set interfaces ethernet eth2 vrf no-mgmt
    commit

This resulted in an error while interacting with nftables:
[Errno 1] failed to run command: nft add element inet vrf_zones ct_iface_map { "eth2" : 151 }

The reason is that the old mapping entry still exists and was not removed.

This commit adds a new utility function get_vrf_tableid() and compares the
current and new VRF table IDs assigned to an interface. If the IDs do not
match, the nftables ct_iface_map entry is removed before the new entry is added.
Commit 452068c ("interfaces: T6592: moving an interface between VRF instances
failed") introduced a new helper to retrieve the VRF table ID from the Kernel.
This commit migrates the old code path where the individual fields got queried
to the new helper vyos.utils.network.get_vrf_tableid().
@c-po c-po marked this pull request as ready for review July 20, 2024 09:48
@c-po
Copy link
Member Author

c-po commented Jul 20, 2024

@Mergifyio backport circinus sagitta

@c-po c-po enabled auto-merge July 20, 2024 09:59
Copy link
Contributor

mergify bot commented Jul 20, 2024

backport circinus sagitta

✅ Backports have been created

Copy link

CI integration ❌ failed!

Details

CI logs

  • CLI Smoketests ❌ failed
  • Config tests 👍 passed
  • RAID1 tests 👍 passed

@c-po c-po deleted the interface-vrf-move branch July 20, 2024 18:02
c-po added a commit that referenced this pull request Jul 22, 2024
interfaces: T6592: moving an interface between VRF instances failed (backport #3834)
c-po added a commit that referenced this pull request Jul 22, 2024
interfaces: T6592: moving an interface between VRF instances failed (backport #3834)
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.

3 participants