udev: fibre channel: fix NPIV support #2500

maurizio-lombardi · 2016-02-01T13:38:26Z

When using NPIV, you can create multiple virtual HBAs on top of the
physical one, this means that the physical N_Port can have multiple
port IDs associated to it.
Suppose a LUN is assigned to the physical HBA and to a virtual HBA,
in both cases the original code uses the ID of the physical HBA
to build the by-path link and udev will end up trying to create two by-path
links with the same name.

This patch fixes the problem by using the port ID of the virtual HBA
whenever it detects that the device belongs to a virtual HBA,
otherwise it uses the port ID of the physical HBA.

Signed-off-by: Maurizio Lombardi mlombard@redhat.com

When using NPIV, you can create multiple virtual HBAs on top of the physical one, this means that the physical N_Port can have multiple port IDs associated to it. Suppose a LUN is assigned to the physical HBA and to a virtual HBA, in both cases the original code uses the ID of the physical HBA to build the by-path link and udev will end up trying to create two by-path links with the same name. This patch fixes the problem by using the port ID of the virtual HBA whenever it detects that the device belongs to a virtual HBA, otherwise it uses the port ID of the physical HBA.

poettering · 2016-02-01T17:07:31Z

Quite frankly, this stuff really shouldn't be maintained within udev in the first place. We have no understanding about the peculiarities of this more exotic kind of hardware or even the details about the storage subsystems involved, we are not storage people.

We cannot make sense of patches like this pretty much, we hence cannot reasonably review them or test them. Given that's how it is, I'd really like to stop supporting this as part of udev. Instead, please move handling of this specific hardware into a package maintained by the storage people (my suggestion: sg3_utils), together with ata_id, scsi_id and cdrom_id, which are of the same kind. There's no reason to have these inside of udev, after all udev rules files and the tools they invoke can just be maintained and packaged outside of it just fine, they are drop-in extensions of udev.

Similar bugs like this one, where understanding of storage drivers and hw is required, which we don't have, and even if we had we couldn't reasonably test are #2362, #1779, #2485.

Hence, please understand that I am unwilling to merge this patch now. Sorry.

poettering · 2016-02-01T17:39:24Z

Also see Kay's comments on the situation: #2363 (comment)

lnykryn · 2016-02-03T10:39:09Z

This patch was verified by our users and it will be used downstream by rhel/centos6 and rhel/centos7, so I would say that at least the "test part" was done. Could you reconsider its inclusion in upstream?

johannbg · 2016-02-03T11:32:40Z

@lnykryn whether some downstream distribution tested this does not change the fact that none of the systemd upstreams neither possesses the skill, understanding and hardware to maintain this reliably and effectively upstream so accepting new patches against the scsi_id and ata_id tools makes no sense.

If Red Hat desperately needs to fix this for it's client base I suggest you pull some chains internally, contact David Sommerseth and see if he and or any other sg3_utils maintainer either within Red Hat or upstream can put some powder in integrating what remains of the udev tools in sg3_utils, the udev tools deprecated and sg3_utils used instead and that change thoroughly tested and included in RHEL 7.3 or 7.4 milestone if it cant make the 7.3 deadline since it's not like RHEL is or will ever be shipping the latest systemd release that includes this until 8.x ( unless of course it's going to break contracts and stability promises to it's customer base )

poettering · 2016-02-03T12:47:33Z

@lnykryn testing only at merge-time is not sufficient. This stuff needs to be maintained. Specifically, there needs to be somebody who understands this and has access to this hardware and makes sure this not only works when the patch is applied, but also in the future.

Please find some place for these things in some package maintained by storage people!

lnykryn · 2016-02-03T13:32:21Z

@poettering I get that. The current situation is unsustainable. And we will talk to our storage guys to find some solution. But meantime, we need some synchronization between downstreams. Otherwise everyone will implement a different naming scheme with downstream patches.

And we already have such problem, recently this patch was merged to upstream:
ba86822
but rhel and sles already have a different implementation:
lnykryn/systemd-rhel@8572638a

poettering · 2016-02-19T12:41:16Z

I guess it makes no sense to leave this open, see the discussion in #2665. Closing.

mfoliveira · 2016-02-24T12:57:04Z

Hi.

It seems this patch replaces the port_name of the target (fc_rport) with that of the vport (the change to the fcdev pointer), assuming it is from the fc_host ("otherwise it uses the port ID of the physical HBA.").

Is that correct? In that case, it wouldn't create unique symlinks when 2 or more fc_rports / targets are used on the same vport.

#2731 merges this commit and #2665 in order to address both patches' problems

Thanks

With the Fibre Channel NPIV (N_Port ID Virtualization) feature, a single physical N_Port (e.g., PCI address) can have multiple N_Port IDs (e.g., different fc_host nodes) - which can connect to the same target LUN (e.g., fc_remote_port's port_name and LUN number). Thus, in order to be unique, the device persistent path should include the fc_vport's port_name (only if the fc_vport is used), in addition to the already in use PCI address, fc_remote_port's port_name and LUN number. The patch merges the 2 proposals submitted upstream, addressing some problems with both: - #2500 (don't replace the fc_rport's port_name with fc_vport's) - #2665 (don't add a fc_host/fc_vport's port_name if not needed) Links - systemd/systemd#2500 - systemd/systemd#2665 Built, checked, tested on RHEL Server 7.1 with no regressions. With the patch, /dev/disk/by-path symlinks are created correctly, and are unique, backward-compatible with the physical port case: - physical port (no vport field) - virtual ports (w/ vport field) # ls -l /dev/disk/by-path | grep 0001:09:00.0 <...> pci-0001:09:00.0-fc-0x500507680b2255fe-lun-0 -> ../../sdh <...> pci-0001:09:00.0-fc-0x500507680b2255ff-lun-0 -> ../../sdi <...> pci-0001:09:00.0-vport-0x5001a4aaf00a6785-fc-0x500507680b2255fe-lun-0 -> ../../sde <...> pci-0001:09:00.0-vport-0x5001a4aaf00a6785-fc-0x500507680b2255ff-lun-0 -> ../../sdd <...> pci-0001:09:00.0-vport-0x5001a4ad99d8c2de-fc-0x500507680b2255fe-lun-0 -> ../../sdc <...> pci-0001:09:00.0-vport-0x5001a4ad99d8c2de-fc-0x500507680b2255ff-lun-0 -> ../../sdb Accordingly w/ sysfs: # ls -ld /sys/block/sd* | grep host1 <...> /sys/block/sdb -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/vport-1:0-0/host3/rport-3:0-1/target3:0:0/3:0:0:0/block/sdb <...> /sys/block/sdc -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/vport-1:0-0/host3/rport-3:0-2/target3:0:1/3:0:1:0/block/sdc <...> /sys/block/sdd -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/vport-1:0-2/host5/rport-5:0-1/target5:0:0/5:0:0:0/block/sdd <...> /sys/block/sde -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/vport-1:0-2/host5/rport-5:0-2/target5:0:1/5:0:1:0/block/sde <...> /sys/block/sdh -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/rport-1:0-3/target1:0:0/1:0:0:0/block/sdh <...> /sys/block/sdi -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/rport-1:0-4/target1:0:1/1:0:1:0/block/sdi The symlinks still include the fc_remote_port's (target) port_name: # grep . /sys/class/fc_remote_ports/rport-{3:0-{1,2},5:0-{1,2},1:0-{3,4}}/port_name /sys/class/fc_remote_ports/rport-3:0-1/port_name:0x500507680b2255ff /sys/class/fc_remote_ports/rport-3:0-2/port_name:0x500507680b2255fe /sys/class/fc_remote_ports/rport-5:0-1/port_name:0x500507680b2255ff /sys/class/fc_remote_ports/rport-5:0-2/port_name:0x500507680b2255fe /sys/class/fc_remote_ports/rport-1:0-3/port_name:0x500507680b2255fe /sys/class/fc_remote_ports/rport-1:0-4/port_name:0x500507680b2255ff And now include the fc_vport's (virtual host) port_name *if* it's from a fc_vport: # grep . /sys/class/fc_host/host1/port_name /sys/class/fc_vports/vport-1:0-{0,2}/port_name /sys/class/fc_host/host1/port_name:0x10000090fa8f0ebc /sys/class/fc_vports/vport-1:0-0/port_name:0x5001a4ad99d8c2de /sys/class/fc_vports/vport-1:0-2/port_name:0x5001a4aaf00a6785 Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> Reported-by: Srikanth B. Aithal <bssrikanth@in.ibm.com> Cherry-picked from: 6a3d3f9e5970cf982ac37c65d0b856146b675a12 Resolves: #1266934

With the Fibre Channel NPIV (N_Port ID Virtualization) feature, a single physical N_Port (e.g., PCI address) can have multiple N_Port IDs (e.g., different fc_host nodes) - which can connect to the same target LUN (e.g., fc_remote_port's port_name and LUN number). Thus, in order to be unique, the device persistent path should include the fc_vport's port_name (only if the fc_vport is used), in addition to the already in use PCI address, fc_remote_port's port_name and LUN number. The patch merges the 2 proposals submitted upstream, addressing some problems with both: - #2500 (don't replace the fc_rport's port_name with fc_vport's) - #2665 (don't add a fc_host/fc_vport's port_name if not needed) Links - systemd/systemd#2500 - systemd/systemd#2665 Built, checked, tested on RHEL Server 7.1 with no regressions. With the patch, /dev/disk/by-path symlinks are created correctly, and are unique, backward-compatible with the physical port case: - physical port (no vport field) - virtual ports (w/ vport field) # ls -l /dev/disk/by-path | grep 0001:09:00.0 <...> pci-0001:09:00.0-fc-0x500507680b2255fe-lun-0 -> ../../sdh <...> pci-0001:09:00.0-fc-0x500507680b2255ff-lun-0 -> ../../sdi <...> pci-0001:09:00.0-vport-0x5001a4aaf00a6785-fc-0x500507680b2255fe-lun-0 -> ../../sde <...> pci-0001:09:00.0-vport-0x5001a4aaf00a6785-fc-0x500507680b2255ff-lun-0 -> ../../sdd <...> pci-0001:09:00.0-vport-0x5001a4ad99d8c2de-fc-0x500507680b2255fe-lun-0 -> ../../sdc <...> pci-0001:09:00.0-vport-0x5001a4ad99d8c2de-fc-0x500507680b2255ff-lun-0 -> ../../sdb Accordingly w/ sysfs: # ls -ld /sys/block/sd* | grep host1 <...> /sys/block/sdb -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/vport-1:0-0/host3/rport-3:0-1/target3:0:0/3:0:0:0/block/sdb <...> /sys/block/sdc -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/vport-1:0-0/host3/rport-3:0-2/target3:0:1/3:0:1:0/block/sdc <...> /sys/block/sdd -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/vport-1:0-2/host5/rport-5:0-1/target5:0:0/5:0:0:0/block/sdd <...> /sys/block/sde -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/vport-1:0-2/host5/rport-5:0-2/target5:0:1/5:0:1:0/block/sde <...> /sys/block/sdh -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/rport-1:0-3/target1:0:0/1:0:0:0/block/sdh <...> /sys/block/sdi -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/rport-1:0-4/target1:0:1/1:0:1:0/block/sdi The symlinks still include the fc_remote_port's (target) port_name: # grep . /sys/class/fc_remote_ports/rport-{3:0-{1,2},5:0-{1,2},1:0-{3,4}}/port_name /sys/class/fc_remote_ports/rport-3:0-1/port_name:0x500507680b2255ff /sys/class/fc_remote_ports/rport-3:0-2/port_name:0x500507680b2255fe /sys/class/fc_remote_ports/rport-5:0-1/port_name:0x500507680b2255ff /sys/class/fc_remote_ports/rport-5:0-2/port_name:0x500507680b2255fe /sys/class/fc_remote_ports/rport-1:0-3/port_name:0x500507680b2255fe /sys/class/fc_remote_ports/rport-1:0-4/port_name:0x500507680b2255ff And now include the fc_vport's (virtual host) port_name *if* it's from a fc_vport: # grep . /sys/class/fc_host/host1/port_name /sys/class/fc_vports/vport-1:0-{0,2}/port_name /sys/class/fc_host/host1/port_name:0x10000090fa8f0ebc /sys/class/fc_vports/vport-1:0-0/port_name:0x5001a4ad99d8c2de /sys/class/fc_vports/vport-1:0-2/port_name:0x5001a4aaf00a6785 Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> Reported-by: Srikanth B. Aithal <bssrikanth@in.ibm.com> (cherry-picked from commit 6a3d3f9e5970cf982ac37c65d0b856146b675a12) Resolves: #1032218

With the Fibre Channel NPIV (N_Port ID Virtualization) feature, a single physical N_Port (e.g., PCI address) can have multiple N_Port IDs (e.g., different fc_host nodes) - which can connect to the same target LUN (e.g., fc_remote_port's port_name and LUN number). Thus, in order to be unique, the device persistent path should include the fc_vport's port_name (only if the fc_vport is used), in addition to the already in use PCI address, fc_remote_port's port_name and LUN number. The patch merges the 2 proposals submitted upstream, addressing some problems with both: - #2500 (don't replace the fc_rport's port_name with fc_vport's) - #2665 (don't add a fc_host/fc_vport's port_name if not needed) Links - systemd/systemd#2500 - systemd/systemd#2665 Built, checked, tested on RHEL Server 7.1 with no regressions. With the patch, /dev/disk/by-path symlinks are created correctly, and are unique, backward-compatible with the physical port case: - physical port (no vport field) - virtual ports (w/ vport field) # ls -l /dev/disk/by-path | grep 0001:09:00.0 <...> pci-0001:09:00.0-fc-0x500507680b2255fe-lun-0 -> ../../sdh <...> pci-0001:09:00.0-fc-0x500507680b2255ff-lun-0 -> ../../sdi <...> pci-0001:09:00.0-vport-0x5001a4aaf00a6785-fc-0x500507680b2255fe-lun-0 -> ../../sde <...> pci-0001:09:00.0-vport-0x5001a4aaf00a6785-fc-0x500507680b2255ff-lun-0 -> ../../sdd <...> pci-0001:09:00.0-vport-0x5001a4ad99d8c2de-fc-0x500507680b2255fe-lun-0 -> ../../sdc <...> pci-0001:09:00.0-vport-0x5001a4ad99d8c2de-fc-0x500507680b2255ff-lun-0 -> ../../sdb Accordingly w/ sysfs: # ls -ld /sys/block/sd* | grep host1 <...> /sys/block/sdb -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/vport-1:0-0/host3/rport-3:0-1/target3:0:0/3:0:0:0/block/sdb <...> /sys/block/sdc -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/vport-1:0-0/host3/rport-3:0-2/target3:0:1/3:0:1:0/block/sdc <...> /sys/block/sdd -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/vport-1:0-2/host5/rport-5:0-1/target5:0:0/5:0:0:0/block/sdd <...> /sys/block/sde -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/vport-1:0-2/host5/rport-5:0-2/target5:0:1/5:0:1:0/block/sde <...> /sys/block/sdh -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/rport-1:0-3/target1:0:0/1:0:0:0/block/sdh <...> /sys/block/sdi -> ../devices/pci0001:00/<...>/0001:09:00.0/host1/rport-1:0-4/target1:0:1/1:0:1:0/block/sdi The symlinks still include the fc_remote_port's (target) port_name: # grep . /sys/class/fc_remote_ports/rport-{3:0-{1,2},5:0-{1,2},1:0-{3,4}}/port_name /sys/class/fc_remote_ports/rport-3:0-1/port_name:0x500507680b2255ff /sys/class/fc_remote_ports/rport-3:0-2/port_name:0x500507680b2255fe /sys/class/fc_remote_ports/rport-5:0-1/port_name:0x500507680b2255ff /sys/class/fc_remote_ports/rport-5:0-2/port_name:0x500507680b2255fe /sys/class/fc_remote_ports/rport-1:0-3/port_name:0x500507680b2255fe /sys/class/fc_remote_ports/rport-1:0-4/port_name:0x500507680b2255ff And now include the fc_vport's (virtual host) port_name *if* it's from a fc_vport: # grep . /sys/class/fc_host/host1/port_name /sys/class/fc_vports/vport-1:0-{0,2}/port_name /sys/class/fc_host/host1/port_name:0x10000090fa8f0ebc /sys/class/fc_vports/vport-1:0-0/port_name:0x5001a4ad99d8c2de /sys/class/fc_vports/vport-1:0-2/port_name:0x5001a4aaf00a6785 Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> Reported-by: Srikanth B. Aithal <bssrikanth@in.ibm.com> Cherry-picked from: 6a3d3f9e5970cf982ac37c65d0b856146b675a12 Resolves: #1266934

maurizio-lombardi force-pushed the master branch from c7ec8c9 to 155a760 Compare February 1, 2016 13:44

zonque added the udev label Feb 1, 2016

poettering added the needs-new-home label Feb 1, 2016

This was referenced Feb 1, 2016

ata_id failing on SATA drives behind JMicron USB bridge #2362

Open

cdrom_id does not handle certain USB sticks with fake CD rom devices on them #1779

Open

udev: some usb cd/dvd burners have (wrongly?) ID_TYPE=generic #2485

Open

poettering mentioned this pull request Feb 19, 2016

udev: path-id - include fc_host's port_name in device persistent path #2665

Closed

poettering closed this Feb 19, 2016

mfoliveira mentioned this pull request Feb 24, 2016

udev: path-id: fibre channel NPIV - use fc_vport's port_name #2731

Closed

poettering mentioned this pull request Apr 18, 2016

udev: can't get the filesystem in multisession CD-ROMs #2872

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

udev: fibre channel: fix NPIV support #2500

udev: fibre channel: fix NPIV support #2500

Uh oh!

maurizio-lombardi commented Feb 1, 2016

Uh oh!

poettering commented Feb 1, 2016

Uh oh!

poettering commented Feb 1, 2016

Uh oh!

lnykryn commented Feb 3, 2016

Uh oh!

johannbg commented Feb 3, 2016

Uh oh!

poettering commented Feb 3, 2016

Uh oh!

lnykryn commented Feb 3, 2016

Uh oh!

poettering commented Feb 19, 2016

Uh oh!

mfoliveira commented Feb 24, 2016

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

6 participants

Uh oh!

udev: fibre channel: fix NPIV support #2500

udev: fibre channel: fix NPIV support #2500

Uh oh!

Conversation

maurizio-lombardi commented Feb 1, 2016

Uh oh!

poettering commented Feb 1, 2016

Uh oh!

poettering commented Feb 1, 2016

Uh oh!

lnykryn commented Feb 3, 2016

Uh oh!

johannbg commented Feb 3, 2016

Uh oh!

poettering commented Feb 3, 2016

Uh oh!

lnykryn commented Feb 3, 2016

Uh oh!

poettering commented Feb 19, 2016

Uh oh!

mfoliveira commented Feb 24, 2016

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

6 participants