linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC - PATCH 0/3] SCSI: Initiator based LUN Masking - SCSI mid-layer
@ 2012-08-11  2:35 kgudipat
  2012-08-13 16:38 ` Steffen Maier
  0 siblings, 1 reply; 4+ messages in thread
From: kgudipat @ 2012-08-11  2:35 UTC (permalink / raw)
  To: JBottomley, linux-scsi, revers, michaelc, hch
  Cc: adapter_linux_open_src_team, amathur, Krishna Gudipati

From: Krishna Gudipati <kgudipat@brocade.com>

Hi James, All,

	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)
==============================================================================
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.

Changed Required:
=================
New Attributes:
===============
	- Add a new list "masked_lun_list", which holds 'scsi_lun' structures
	  to the SCSI target structure "struct scsi_target".

	  struct scsi_target {
		. . .
		struct list_head	masked_lun_list;
		. . .
	  };

	- Add a new attribute "is_masking_enabled" in struct scsi_target that
	  indicates if the LUN Masking for this target is enabled or not.

	  struct scsi_target {
		. . .
		unsigned is_masking_enabled;
		. . .
	  };

	- Add a new attribute "is_masked" to the "struct scsi_device" to indicate
	  if this particular scsi_device is masked (to be made visible).

	  struct scsi_device {
		. . .
		unsigned is_masked:1;
		. . .
	  };

	- Add a new structure struct scsi_mask_lun which contains the list of
	  masked luns

	  struct scsi_mask_lun {
		struct list_head list_entry;
		struct scsi_lun mask_lun;
	  };

	- Introduce a status flag SCSI_SCAN_MASK_LUN, which is used by the SCSI scan
	  routines scsi_report_lun_scan() and scsi_sequential_lun_scan() to skip
	  adding this LUN and continue the SCSI target scan.

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

	  /*
	   * The function scsi_target_mask_lun() takes both the scsi_target and
	   * the LUN to be added to the masked_lun_list as the input and adds
	   * the LUN to the masked_lun_list if it has already been not added.
	   * The masked_lun_list is sorted by the LUN number.
	   */
	   extern int scsi_target_mask_lun(struct Scsi_Host *shost, int channel,
					   int target_id, unsigned int lun);

	  /*
	   * The function scsi_target_unmask_lun() takes both the scsi_target
	   * and the LUN to be removed from the masked_lun_list as the input and
	   * removes the LUN from the masked_lun_list if it exists.
	   * The masked_lun_list is sorted by the LUN number.
	   */
	   extern int scsi_target_unmask_lun(struct Scsi_Host *shost, int channel,
					     int target_id, unsigned int lun);

	- Introduce an API to set/unset LUN Masking configuration

	  /*
	   * The function scsi_target_config_lunmask() takes both the scsi_target
	   * and the masking_config as input and configures the LUN Masking per
	   * SCSI target; masking_config can be 1 or 0 where 1 - is to set and
	   * 0 - unset LUN Masking.
	   */
	   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":
=================================
  - During initial SCSI scan for each newly discovered target SCSI mid-layer will
    invoke LLD target_alloc() entry point for LLD to configure target specific data.

  - 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.

Initial SCSI scan Flow:
=======================

__scsi_scan_target() {

    scsi_alloc_target() {

	xxx_target_alloc(starget) {	<-- LLD target_alloc() entry point

	   - Set scsi_target "is_masking_enabled" flag to enable LUN Masking
	     using the API scsi_target_config_lunmask();

	   - Add the LUNs to be masked to the masked_lun_list

	   list_add_tail(&slun->list_entry, &starget->masked_lun_list);

	   . . .
	}

   }

   scsi_probe_and_add_lun() {

	sdev = scsi_alloc_sdev();

	scsi_probe_lun();

	if (starget->is_masking_enabled) {

	    /*
	     * Check if the LUN is part of the scsi_target "masked_lun_list"
	     * If YES - set the sdev->is_masked flag for this particular LUN.
	     * It NOT - don't see the sdev->is_masked flag.
	     * Only the scsi_devices that have the is_masked flag set will be
	     * made visible in the sysfs.
	     */
	}

	. . .

	if (starget->is_masking_enabled) {
		if (sdev->is_masked)
			scsi_add_lun();
		else
			goto out_free_result;
	}

	. . .

    out_free_result:
	/* Remove scsi_device that is not part of LUN Masking list */
	/* Return status as SCSI_SCAN_MASK_LUN */
    . . .

    }
}

Note: The functions scsi_report_lun_scan() and scsi_sequential_lun_scan() when
      they receive the return status as SCSI_SCAN_MASK_LUN will skip adding this
      LUN and continue with the SCSI scan for rest of the LUNs.

SCSI Target Destroy:
===================
	- During the scsi_target_destroy() the masked_lun_list needs to be freed.

LUN Masking configuration change:
=================================
To enable LUN Masking from disabled state:
==========================================
    - Set LUN Masking in scsi_target using scsi_target_config_lunmask() API.
    - Add the list of visible luns to the "masked_lun_list".
    - 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) {

		    /* 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().
		     * 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 */
		    . . .
		}
		. . .
	}
    }

    Handling currently not-visible SCSI Devices:
    --------------------------------------------
    Flow:
    -----
    __scsi_scan_target() {
	scsi_probe_and_add_lun() {
		. . .
		- Add the discovered LUN to the sysfs.
		. . .
	}
    }

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.

Thanks,
Krishna C Gudipati.

Krishna Gudipati (3):
  SCSI: Implement LUN Masking in SCSI mid-layer.
  bfa: Revert current LUN Masking implementation using slave callouts
  bfa: Implement LUN Masking using SCSI mid-layer LUN Masking support.

 drivers/scsi/bfa/bfad_bsg.c |   56 ++++++++--
 drivers/scsi/bfa/bfad_im.c  |   96 +++++++++--------
 drivers/scsi/bfa/bfad_im.h  |   27 ++----
 drivers/scsi/scsi_scan.c    |  251 +++++++++++++++++++++++++++++++++++++++++--
 include/scsi/scsi.h         |    8 ++
 include/scsi/scsi_device.h  |    9 ++
 6 files changed, 361 insertions(+), 86 deletions(-)

--
1.7.3.rc1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC - PATCH 0/3] SCSI: Initiator based LUN Masking - SCSI mid-layer
  2012-08-11  2:35 [RFC - PATCH 0/3] SCSI: Initiator based LUN Masking - SCSI mid-layer kgudipat
@ 2012-08-13 16:38 ` Steffen Maier
  2012-08-13 22:48   ` Krishna Gudipati
  0 siblings, 1 reply; 4+ messages in thread
From: Steffen Maier @ 2012-08-13 16:38 UTC (permalink / raw)
  To: kgudipat
  Cc: JBottomley, linux-scsi, revers, michaelc, hch,
	adapter_linux_open_src_team, amathur, Martin Peschke,
	Daniel Hansel

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [RFC - PATCH 0/3] SCSI: Initiator based LUN Masking - SCSI mid-layer
  2012-08-13 16:38 ` Steffen Maier
@ 2012-08-13 22:48   ` Krishna Gudipati
  2012-08-14 11:16     ` Steffen Maier
  0 siblings, 1 reply; 4+ messages in thread
From: Krishna Gudipati @ 2012-08-13 22:48 UTC (permalink / raw)
  To: Steffen Maier
  Cc: JBottomley@Parallels.com, linux-scsi@vger.kernel.org, Rob Evers,
	michaelc@cs.wisc.edu, hch@infradead.org,
	Adapter Linux Open SRC Team, Akshay Mathur, Martin Peschke,
	Daniel Hansel

Hi Steffen,

Thanks for reviewing the design, please find my responses in line with tag [KRISHNA].

Thanks,
Krishna.

> From: Steffen Maier [mailto:maier@linux.vnet.ibm.com]
> On 08/11/2012 04:35 AM, kgudipat@brocade.com 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC - PATCH 0/3] SCSI: Initiator based LUN Masking - SCSI mid-layer
  2012-08-13 22:48   ` Krishna Gudipati
@ 2012-08-14 11:16     ` Steffen Maier
  0 siblings, 0 replies; 4+ messages in thread
From: Steffen Maier @ 2012-08-14 11:16 UTC (permalink / raw)
  To: Krishna Gudipati
  Cc: JBottomley@Parallels.com, linux-scsi@vger.kernel.org, Rob Evers,
	michaelc@cs.wisc.edu, hch@infradead.org,
	Adapter Linux Open SRC Team, Akshay Mathur, Martin Peschke,
	Daniel Hansel

Hi Krishna,

On 08/14/2012 12:48 AM, Krishna Gudipati wrote:
>> From: Steffen Maier [mailto:maier@linux.vnet.ibm.com]
>> On 08/11/2012 04:35 AM, kgudipat@brocade.com wrote:

> [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.

Right, but this would mean unnecessary code duplication because each LLD 
would have to implement its own user space interface, whereas common 
code in the midlayer could handle all this transparently for any 
(unmodified) LLD.

A common user space interface would also ease establishing a common 
persistency mechanism in user space that can be used transparently by 
any LLD.

> 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.

I understand this makes sense for an LLD that already has a persistency 
layer. However, in general, this is probably not the case. Therefore, it 
seems to me as if your approach focuses on the specific case rather than 
the generic one.

> [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.

Good point, thanks for the explanation. This is definitely a big pro for 
midlayer lun masking.

In zfcp, we haven't had this issue so far, because it was either no lun 
scanning at all or allow all luns scanned by the midlayer. With midlayer 
lun masking, we could even allow the user to filter/select luns for the 
latter case which is very useful for big storage sites with relaxed 
target lun masking.

Speaking of big SANs, this could also extend to initiator based "zoning" 
one day, i.e. having a common mechanism in scsi_transport_fc to let the 
user specify which remote target port WWPNs she would like to use and 
only login to those. But I guess, this is just wishful thinking right 
now. We have users with >30 Linux images sharing each (of multiple for 
multipathing) HBA and thus making up a lot of initiators often 
configured into the same FC zone and this can cause all kinds of trouble 
especially if rezoning the fabric (preferably into single initiator 
zones which can get many) is not an option.

> 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.

Have a look at how SuSE configures s390 devices by means of udev rules 
/etc/udev/rules.d/51-....rules written by /sbin/zfcp_disk_configure 
being part of the s390-tools rpm package. Instead of writing to zfcp 
specific sysfs attributes, it could write to midlayer sysfs attributes 
provided for lun masking, as an example:
https://build.opensuse.org/package/view_file?file=zfcp_disk_configure&package=s390-tools&project=Base%3ASystem

Regards,
Steffen

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-08-14 11:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-11  2:35 [RFC - PATCH 0/3] SCSI: Initiator based LUN Masking - SCSI mid-layer kgudipat
2012-08-13 16:38 ` Steffen Maier
2012-08-13 22:48   ` Krishna Gudipati
2012-08-14 11:16     ` Steffen Maier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).