Skip to content

Commit

Permalink
Revert "udev-builtin-net_id: use firmware_node/sun for ID_NET_NAME_SLOT"
Browse files Browse the repository at this point in the history
This reverts commit 7af151c.

It seems that virtio devices always have "0" in
the firmware_node/sun. And because of that, udev will
always name the device ens0, which leads to collisions.
So let's disable it for now.

rhel-only: policy

Reverts: RHEL-50103
  • Loading branch information
lnykryn authored and github-actions[bot] committed Aug 22, 2024
1 parent 11992ca commit ae92c09
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 64 deletions.
5 changes: 2 additions & 3 deletions man/systemd.net-naming-scheme.xml
Original file line number Diff line number Diff line change
Expand Up @@ -510,10 +510,9 @@
to distinguish between devices. However, name conflict can occur if these devices are not
children of the same PCI bridge, e.g. there are multiple PCI bridges in the same slot.
</para>

<para>PCI slot number is now read from <constant>firmware_node/sun</constant> sysfs file.</para>
</listitem>
</varlistentry>

</variablelist>

<para>By default <constant>rhel-9.0</constant> is used.</para>
Expand Down Expand Up @@ -667,7 +666,7 @@ ID_NET_NAME_ONBOARD_LABEL=Ethernet Port 1
</example>

<example>
<title>PCI Ethernet card in slot with firmware index number</title>
<title>PCI Ethernet card in hotplug slot with firmware index number</title>

<programlisting># /sys/devices/pci0000:00/0000:00:1c.3/0000:05:00.0/net/ens1
ID_NET_NAME_MAC=enx000000000466
Expand Down
3 changes: 1 addition & 2 deletions src/shared/netif-naming-scheme.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ typedef enum NamingSchemeFlags {
* This is disabled since rhel-9.5, as it seems not to work at least for some setups. See upstream issue #28929. */
NAMING_DEVICETREE_ALIASES = 1 << 15, /* Generate names from devicetree aliases */
NAMING_SR_IOV_R = 1 << 17, /* Use "r" suffix for SR-IOV VF representors */
NAMING_FIRMWARE_NODE_SUN = 1 << 18, /* Use firmware_node/sun to get PCI slot number */

/* And now the masks that combine the features above */
NAMING_V238 = 0,
Expand Down Expand Up @@ -74,7 +73,7 @@ typedef enum NamingSchemeFlags {
NAMING_RHEL_9_2 = NAMING_RHEL_9_0,
NAMING_RHEL_9_3 = NAMING_RHEL_9_0 | NAMING_SR_IOV_R,
NAMING_RHEL_9_4 = NAMING_RHEL_9_3,
NAMING_RHEL_9_5 = (NAMING_RHEL_9_4 & ~NAMING_BRIDGE_MULTIFUNCTION_SLOT) | NAMING_FIRMWARE_NODE_SUN,
NAMING_RHEL_9_5 = NAMING_RHEL_9_4 & ~NAMING_BRIDGE_MULTIFUNCTION_SLOT,

EXTRA_NET_NAMING_SCHEMES

Expand Down
66 changes: 7 additions & 59 deletions src/udev/udev-builtin-net_id.c
Original file line number Diff line number Diff line change
Expand Up @@ -442,51 +442,6 @@ static int pci_get_hotplug_slot(sd_device *dev, uint32_t *ret) {
return -ENOENT;
}

static int get_device_firmware_node_sun(sd_device *dev, uint32_t *ret) {
const char *attr;
int r;

assert(dev);
assert(ret);

r = device_get_sysattr_value_filtered(dev, "firmware_node/sun", &attr);
if (r < 0)
return log_device_debug_errno(dev, r, "Failed to read firmware_node/sun, ignoring: %m");

r = safe_atou32(attr, ret);
if (r < 0)
return log_device_warning_errno(dev, r, "Failed to parse firmware_node/sun '%s', ignoring: %m", attr);

return 0;
}

static int pci_get_slot_from_firmware_node_sun(sd_device *dev, uint32_t *ret) {
int r;
sd_device *slot_dev;

assert(dev);
assert(ret);

/* Try getting the ACPI _SUN for the device */
if (get_device_firmware_node_sun(dev, ret) >= 0)
return 0;

r = sd_device_get_parent_with_subsystem_devtype(dev, "pci", NULL, &slot_dev);
if (r < 0)
return log_device_debug_errno(dev, r, "Failed to find pci parent, ignoring: %m");

if (is_pci_bridge(slot_dev) && is_pci_multifunction(dev) <= 0)
return log_device_debug_errno(dev, SYNTHETIC_ERRNO(ESTALE),
"Not using slot information because the parent pcieport "
"is a bridge and the PCI device is not multifunction.");

/* Try getting the ACPI _SUN from the parent pcieport */
if (get_device_firmware_node_sun(slot_dev, ret) >= 0)
return 0;

return -ENOENT;
}

static int dev_pci_slot(sd_device *dev, const LinkInfo *info, NetNames *names) {
const char *sysname, *attr;
unsigned domain, bus, slot, func;
Expand Down Expand Up @@ -562,20 +517,13 @@ static int dev_pci_slot(sd_device *dev, const LinkInfo *info, NetNames *names) {
domain, bus, slot, func, strempty(info->phys_port_name), dev_port,
special_glyph(SPECIAL_GLYPH_ARROW_RIGHT), empty_to_na(names->pci_path));

if (naming_scheme_has(NAMING_FIRMWARE_NODE_SUN))
r = pci_get_slot_from_firmware_node_sun(names->pcidev, &hotplug_slot);
else
r = -1;
/* If we don't find a slot using firmware_node/sun, fallback to hotplug_slot */
if (r < 0) {
r = pci_get_hotplug_slot(names->pcidev, &hotplug_slot);
if (r < 0)
return r;
if (r > 0)
/* If the hotplug slot is found through the function ID, then drop the domain from the name.
* See comments in parse_hotplug_slot_from_function_id(). */
domain = 0;
}
r = pci_get_hotplug_slot(names->pcidev, &hotplug_slot);
if (r < 0)
return r;
if (r > 0)
/* If the hotplug slot is found through the function ID, then drop the domain from the name.
* See comments in parse_hotplug_slot_from_function_id(). */
domain = 0;

s = names->pci_slot;
l = sizeof(names->pci_slot);
Expand Down

0 comments on commit ae92c09

Please sign in to comment.