Re: [RFC - PATCH 0/3] SCSI: Initiator based LUN Masking - SCSI mid-layer

[Steffen Maier <maier@linux.vnet.ibm.com>]

Hi Krishna,

On 08/11/2012 04:35 AM, kgudipat@brocade.com wrote:
> 	The following RFC patch-set adds Initiator based LUN Masking support to SCSI mid-layer
> 	and tries to address the feedback we have received to have LUN Masking feature in the
> 	SCSI mid-layer and made available to everyone.
>
> 	We came up with a design proposal to implement LUN Masking in SCSI mid-layer.
>
> 	I have listed the proposal details below and also attached the RFC patch-set
> 	that enables LUN Masking in the SCSI mid-layer and the implementation of the
> 	LUN Masking feature by the Brocade BFA driver using the APIs provided by the
> 	SCSI mid-layer to support LUN Masking feature.
>
> 	Can you please review the design proposal, patches and please let us know your opinion?
>
> Thanks in advance,
> Krishna C Gudipati.
>
> (LUN masked = LUN to be made visible)

This terminology definition strikes me a bit odd, though I have to admit 
that I'm not a native speaker. One of the definitions I found says "to 
conceal from view" which is also what I would feel naturally and seems 
the opposite of your proposed usage. How is "masking" defined for target 
lun masking on the storage side?

> ==============================================================================
> Proposal - LUN Masking implementation in the SCSI mid-layer using SCSI target
> ==============================================================================
>
> Design Proposal:
> ================
> Maintain a masked LUN list (the luns to be made visible) per target in the
> SCSI target structure and use this list to determine whether to add the LUN
> to the system config or to skip adding this LUN during the SCSI scan.
>
> The masked LUN list can be populated during the LLD target_alloc() entry point
> and can also be modified using the APIs that re-config this masked LUN list.
> The re-config is followed by a SCSI scan to honor the modified masked LUN list.

> API changes:
> ============
> 	- Introduce APIs to add or remove LUNs from SCSI target masked_lun_list.

> 	   extern int scsi_target_mask_lun(struct Scsi_Host *shost, int channel,
> 					   int target_id, unsigned int lun);

> 	   extern int scsi_target_unmask_lun(struct Scsi_Host *shost, int channel,
> 					     int target_id, unsigned int lun);

> 	   extern int scsi_target_config_lunmask(struct Scsi_Host *shost, int channel,
> 						 int target_id, int masking_config);

> Pseudocode/Logic:
> =================
> Populating the "masked_lun_list":
> =================================

>    - For implementing LUN Masking, LLD can add the list of LUNs that are to be
>      masked to the scsi_target "masked_lun_list" in the target_alloc() entry point.
>
>    - Further add/remove modifications to "masked_lun_list" can be done using APIs
>      scsi_target_mask_lun() or scsi_target_unmask_lun() respectively.

Please correct me if my understanding is wrong. It seems to me as if the 
only interface to add/removing luns from the masking list in the scsi 
midlayer are three in-kernel functions that are supposed to be called by 
an LLD. I understand this makes sense in your case with bfa, where you 
seem to have a vendor-specific interface via bsg to user space to 
add/remove luns from masking (among other things), and persistent 
storage for the configured stuff in flash on the HBA.

Zfcp has been having initiator based lun masking since a long time. 
Originally, we disabled all midlayer lun scanning on registering the 
scsi_host. Only on explicit user actions through our own driver-specific 
sysfs interface, we would ask the midlayer to probe and add one 
particular lun by calling scsi_scan_target() for this one specific lun only.

When we introduced automatic lun scanning and probing through the 
midlayer for NPIV setups, we also had to introduce -ENXIO in 
slave_alloc() to suppress luns we do not want as an LLD when running in 
non-NPIV setups, where we still support hardware virtualized HBAs and 
must not touch each reported lun in each virtual Linux image sharing the 
same HBA and thus the same physical WWPN making the Linux images 
indistinguishable in the fabric and on the target.

However, in both cases we only have the lun masking knowledge in the LLD 
because we did not know of any other existing mechanism we could reuse 
but to implement it on our own. The user space interface is by means of 
unit_add and unit_remove sysfs attributes in the LLD specific sysfs tree 
reflecting HBAs, remote ports, and fcp luns. Basically, it is a shadow 
of common code objects of type scsi/fc_host, fc_rport/target, 
scsi_device making up the hierarchical whitelist of resources the user 
would like to use. The user interface is described in the scsi/zfcp 
chapter of our device drivers book on 
http://www.ibm.com/developerworks/linux/linux390/documentation_dev.html.

In other words, we don't have our own persistence layer and would be 
happy to rely on a common user space interface possibly owned by the 
scsi midlayer. Other LLDs, that also don't have their own persistence 
layer, could then also build on this generic lun masking and some user 
space tool could do persistence if needed (such as udev rules).

Without such common user space interface, I don't yet see why the scsi 
stack should maintain a (copy of an LLD) list if this list can only come 
from an LLD which could already do the lun masking in slave_alloc()?
I admit that it would be a first step, though.

> LUN Masking configuration change:
> =================================
> To enable LUN Masking from disabled state:
> ==========================================

>      Handling already attached SCSI Devices:
>      ---------------------------------------
>      Flow:
>      -----
>      __scsi_scan_target() {
> 	scsi_probe_and_add_lun() {
> 		. . .
> 		sdev = scsi_device_lookup_by_target(starget, lun);
> 		if (sdev) {
>
> 		    /* Check if this LUN is part of the "masked_lun_list"
> 		     * If YES - set the sdev->is_masked to 1 and return
> 		     * SCSI_SCAN_LUN_PRESENT as done currently.
> 		     * If the LUN is NOT part of the "masked_lun_list" then
> 		     * remove this device using the __scsi_remove_device().

Is __scsi_remove_device() safe to call? The device could be in use. Of 
course it would still have a refcount > 1 but we've had quite a number 
of scsi device removal races lately so I might be a bit overcautious.

> 		     * The return status in this case will be SCSI_SCAN_MASK_LUN
> 		     */

> To disable LUN Masking from enabled state:
> ==========================================
>      - Unset LUN Masking in scsi_target using scsi_target_config_lunmask() API.
>      - Trigger a new LUN scan to reflect the new config.
>
>      Handling already attached SCSI Devices:
>      ---------------------------------------
>      Flow:
>      -----
>      __scsi_scan_target() {
> 	scsi_probe_and_add_lun() {
> 		. . .
> 		sdev = scsi_device_lookup_by_target(starget, lun);
> 		if (sdev) {
> 		    . . .
> 		    /* Unset the sdev->is_masked bit */
> 		    . . .

Dito, since this also calls __scsi_remove_device() eventually.

> 		}
> 		. . .
> 	}
>      }

> Pros:
> =====
> 	- Implementation is not that complex
> 	- LLD can choose at run time / during SCSI scan LUNs to be masked.
> 	- SCSI Stack controls the device addition/removal during the scan
> 	  based on the LLD LUN masking config.
> 	- SCSI Stack maintains the list of masked luns.


Steffen Maier

Linux on System z Development

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html