Hi Steffen, Thanks for reviewing the design, please find my responses in line with tag [KRISHNA]. Thanks, Krishna. > From: Steffen Maier [mailto:maier@xxxxxxxxxxxxxxxxxx] > On 08/11/2012 04:35 AM, kgudipat@xxxxxxxxxxx wrote: > > (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? [KRISHNA]: Here masking is used synonymously to zoning. So, only the LUNs that are part of the scsi_target masked lun list (i.e. LUNs of a specific target zoned to the initiator to be seen) - can be attached during the SCSI scan and rest are supposed to be skipped/not attached. > > > > ========================================================== > ==================== > > 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. [KRISHNA]: Steffen, yes you are right, currently in this proposal we only have 3 interfaces exported from SCSI mid-layer for LLD to configure LUN Masking. I believe with the interfaces provided currently we should be able to enable LUN masking even though we don't have a persistent storage or a vendor-specific BSG based interface. The interfaces scsi_target_mask_lun() to mask a LUN and scsi_target_unmask_lun() to unmask a LUN can be called dynamically even from your current sysfs implementation of LUN Masking. The only action required on a configuration change is to trigger a new SCSI scan and the state of the masked LUNs and LUN Masking configuration state is maintained by the SCSI mid-layer until the scsi target is destroyed. On a target discovery we need to re-configure the LUN Masking from sysfs again. The advantage of having persistent storage in the LLD is we can have LUN Masking enabled/configured during the driver load, as we can call the APIs to configure LUN Masking from target_alloc() entry point and can avoid LUN Masking configuration from user space during every module load and during the target offline/online events as above. > > 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()? [KRISHNA]: The disadvantage of the LUN Masking implementation using the SCSI slave callouts is we cannot do a REPORT_LUNS based SCSI scan and need to fall back to Sequential LUN scan. The reason being if we return -ENXIO from slave_alloc() for any LUN that is part of the REPORT_LUNS response payload this would result in the scan being aborted. So we need to do a sequential LUN scan which is not so good as we end up scanning for 16K LUNs, for SPARSE LUNs. So we came up with this proposal. > I admit that it would be a first step, though. [KRISHNA]: Steffen, thanks for the review. Please let me know if I have answered your questions. In addition the design can be enhanced to have something like udev rules to maintain persistency as you mentioned; I will think it through, please let me know if you have some ideas. > > > 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. [KRISHNA]: During my testing I haven't seen any issue - but I would double-check. > > > * 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
