fix: Remove MAC address matching from SysUtil.link_info_find()

The link_info_find() function previously allowed searching for links by
MAC address, but this introduced ambiguity and could cause false alarms
in certain cases (e.g. retrieving the link info by MAC might return the
link info that only matches the current MAC instead of the permanent
MAC). To ensure reliable behavior, this function should accept and match
the link info only by interface name.

To address the issues, the following changes were made:
- Removed MAC address matching logic to eliminate ambiguity.
- Simplified the function to only check ifname, making it more
  predictable.
- Updated all callers to adapt to this change, ensuring correctness.
- When a profile is tied to an interface via mac only, the validation of
  the existence of interface will now be delegated to NetworkManager
instead.

Resolves: https://issues.redhat.com/browse/RHEL-84197

Signed-off-by: Wen Liang <liangwen12year@gmail.com>
This commit is contained in:
Wen Liang 2025-03-13 18:10:47 -04:00 committed by liangwen12year
parent 6a5ca9309e
commit cac2bbb43e
8 changed files with 207 additions and 34 deletions

View file

@ -222,31 +222,15 @@ class SysUtil:
return linkinfos
@classmethod
def link_info_find(cls, refresh=False, mac=None, ifname=None):
if mac is not None:
mac = Util.mac_norm(mac)
for linkinfo in cls.link_infos(refresh).values():
perm_address = linkinfo.get("perm-address", None)
current_address = linkinfo.get("address", None)
def link_info_find(cls, ifname):
result = None
# Match by perm-address (prioritized)
if mac is not None and perm_address not in [None, "00:00:00:00:00:00"]:
if mac == perm_address:
return linkinfo
for linkinfo in cls.link_infos().values():
if ifname == linkinfo["ifname"]:
result = linkinfo
break
# Fallback to match by address
if mac is not None and (perm_address in [None, "00:00:00:00:00:00"]):
if mac == current_address:
matched_by_address = linkinfo # Save for potential fallback
if ifname is not None and ifname == linkinfo.get("ifname", None):
return linkinfo
# Return fallback match by address if no perm-address match found
if "matched_by_address" in locals():
return matched_by_address
return None
return result
###############################################################################
@ -2155,18 +2139,8 @@ class Cmd(object):
# permanent MAC address.
li_mac = None
li_ifname = None
if connection["mac"]:
li_mac = SysUtil.link_info_find(mac=connection["mac"])
if not li_mac:
self.log_fatal(
idx,
"profile specifies mac '%s' but no such interface exists"
% (connection["mac"]),
)
if connection["interface_name"]:
li_ifname = SysUtil.link_info_find(
ifname=connection["interface_name"]
)
li_ifname = SysUtil.link_info_find(connection["interface_name"])
if not li_ifname:
if connection["type"] == "ethernet":
self.log_fatal(

View file

@ -79,6 +79,7 @@ ibution_major_version | int < 9",
"playbooks/tests_infiniband.yml": {},
"playbooks/tests_ipv6_disabled.yml": {},
"playbooks/tests_ipv6_dns_search.yml": {},
"playbooks/tests_mac_address_match.yml": {},
"playbooks/tests_provider.yml": {
MINIMUM_VERSION: "'1.20.0'",
"comment": "# NetworKmanager 1.20.0 added support for forgetting profiles",

View file

@ -0,0 +1,75 @@
# SPDX-License-Identifier: BSD-3-Clause
---
- name: Play for testing MAC address match on device
hosts: all
vars:
# This test requires an Ethernet interface whose permanent
# MAC address matches its current MAC address. Ensure that the
# specified interface meets this condition.
#
# Two VLAN profiles are defined to test deterministic behavior when fetching
# link information from sysfs. The issue being tested arises when `os.listdir(path)`
# returns interfaces in an arbitrary order, potentially listing a VLAN device
# before its parent interface. This can cause intermittent "no such interface
# exists" errors when applying configurations repeatedly.
#
# - `vlan_profile1` (e.g., `eth1.3732`) is named with the VLAN ID appended
# after the parent interface, following the standard `<parent>.<vlan_id>` format.
# - `vlan_profile2` (e.g., `120-vlan`) has a fixed name, designed to test a scenario
# where lexicographic sorting causes the VLAN to appear before its parent interface.
interface: "{{ lookup('env', 'MAC_ADDR_MATCH_INTERFACE') | default('eth1', true) }}"
profile: "{{ interface }}"
vlan_profile1: "{{ interface }}.3732"
vlan_profile2: "120-vlan"
lsr_fail_debug:
- __network_connections_result
tags:
- "tests::match"
tasks:
- name: Show playbook name
debug:
msg: "this is: playbooks/tests_mac_address_match.yml"
tags:
- always
- name: Install ethtool (test dependency)
package:
name: ethtool
state: present
use: "{{ (__network_is_ostree | d(false)) |
ternary('ansible.posix.rhel_rpm_ostree', omit) }}"
- name: Retrieve MAC address using ethtool
command: ethtool -P {{ interface }}
register: mac_address_result
changed_when: false
failed_when: mac_address_result.rc != 0
- name: Set the MAC address variable
set_fact:
mac: "{{ mac_address_result.stdout_lines[-1].split(' ')[-1] }}"
- name: Display the retrieved MAC address
debug:
msg: "Retrieved MAC address for {{ interface }}: {{ mac }}"
- name: Test the MAC address match
tags:
- tests::match:match
block:
- name: Include the task 'run_test.yml'
include_tasks: tasks/run_test.yml
vars:
lsr_description: Test MAC address match on device
lsr_setup:
- tasks/find+remove_profile.yml
- tasks/assert_profile_absent.yml
lsr_test:
- tasks/create_mac_address_match.yml
- tasks/create_mac_address_match.yml
lsr_assert:
- tasks/assert_profile_present.yml
- tasks/assert_network_connections_succeeded.yml
lsr_cleanup:
- tasks/cleanup_vlan_and_parent_profile+device.yml
- tasks/check_network_dns.yml

View file

@ -0,0 +1,8 @@
# SPDX-License-Identifier: BSD-3-Clause
---
- name: Assert that configuring network connections is succeeded
assert:
that:
- __network_connections_result.failed == false
msg: Configuring network connections is failed with the error
"{{ __network_connections_result.stderr }}"

View file

@ -0,0 +1,26 @@
# SPDX-License-Identifier: BSD-3-Clause
---
- name: Clean up the test devices and the connection profiles
tags:
- "tests::cleanup"
block:
- name: Import network role
import_role:
name: linux-system-roles.network
vars:
network_connections:
- name: "{{ profile }}"
persistent_state: absent
state: down
- name: "{{ vlan_profile1 }}"
persistent_state: absent
state: down
- name: "{{ vlan_profile2 }}"
persistent_state: absent
state: down
failed_when: false
- name: Delete the device '{{ interface }}'
command: ip link del {{ interface }}
failed_when: false
changed_when: false
...

View file

@ -0,0 +1,51 @@
# SPDX-License-Identifier: BSD-3-Clause
---
- name: Include network role
include_role:
name: linux-system-roles.network
vars:
network_connections:
- name: "{{ interface }}"
state: up
persistent_state: present
autoconnect: true
type: ethernet
interface_name: "{{ interface }}"
mac: "{{ mac }}"
ip:
dhcp4: false
auto6: false
- name: "{{ vlan_profile1 }}"
state: up
persistent_state: present
type: vlan
parent: "{{ interface }}"
vlan:
id: 3732
autoconnect: true
ip:
auto_gateway: false
gateway4: 10.10.0.1
address: 10.10.0.6/24
dhcp4: false
auto6: false
- name: "{{ vlan_profile2 }}"
state: up
persistent_state: present
type: vlan
parent: "{{ interface }}"
vlan:
id: 120
autoconnect: true
ip:
auto_gateway: false
gateway4: 10.10.120.1
address: 10.10.120.120/24
dhcp4: false
auto6: false
- name: Show result
debug:
var: __network_connections_result
...

View file

@ -0,0 +1,15 @@
# SPDX-License-Identifier: BSD-3-Clause
---
- name: Get connection profile for '{{ interface }}'
command: "nmcli -g GENERAL.CONNECTION device show {{ interface }}"
register: connection_name
changed_when: false
- name: Bring down and delete the connection profile for '{{ interface }}'
include_role:
name: linux-system-roles.network
vars:
network_connections:
- name: "{{ connection_name.stdout }}"
persistent_state: absent
state: down

View file

@ -0,0 +1,23 @@
# SPDX-License-Identifier: BSD-3-Clause
# This file was generated by ensure_provider_tests.py
---
# set network provider and gather facts
# yamllint disable rule:line-length
- name: Run playbook 'playbooks/tests_mac_address_match.yml' with nm as provider
hosts: all
tasks:
- name: Include the task 'el_repo_setup.yml'
include_tasks: tasks/el_repo_setup.yml
- name: Set network provider to 'nm'
set_fact:
network_provider: nm
tags:
- always
# The test requires or should run with NetworkManager, therefore it cannot run
# on RHEL/CentOS 6
- name: Import the playbook 'playbooks/tests_mac_address_match.yml'
import_playbook: playbooks/tests_mac_address_match.yml
when:
- ansible_distribution_major_version != '6'