[PATCH] zfcp: Report FCP LUN to SCSI midlayer

[PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Swen Schillig]

From: Christof Schmitt <christof.schmitt@de.ibm.com>

When reporting SCSI devices to the SCSI midlayer, use the FCP LUN as
LUN reported to the SCSI layer. With this approach, zfcp does not have
to create unique LUNS, and this code can be removed.

Signed-off-by: Christof Schmitt <christof.schmitt@de.ibm.com>
Signed-off-by: Swen Schillig <swen@vnet.ibm.com>

---

 drivers/s390/scsi/zfcp_aux.c |   22 ++++------------------
 drivers/scsi/scsi_scan.c     |    3 ++-
 include/scsi/scsi_device.h   |    1 +
 3 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_aux.c b/drivers/s390/scsi/zfcp_aux.c
index 821cde6..c631cf7 100644
--- a/drivers/s390/scsi/zfcp_aux.c
+++ b/drivers/s390/scsi/zfcp_aux.c
@@ -815,9 +815,7 @@ zfcp_get_adapter_by_busid(char *bus_id)
 struct zfcp_unit *
 zfcp_unit_enqueue(struct zfcp_port *port, fcp_lun_t fcp_lun)
 {
-	struct zfcp_unit *unit, *tmp_unit;
-	unsigned int scsi_lun;
-	int found;
+	struct zfcp_unit *unit;

 	/*
 	 * check that there is no unit with this FCP_LUN already in list
@@ -863,22 +861,10 @@ zfcp_unit_enqueue(struct zfcp_port *port, fcp_lun_t fcp_lun)
 	}

 	zfcp_unit_get(unit);
-
-	scsi_lun = 0;
-	found = 0;
+	unit->scsi_lun = scsilun_to_int((struct scsi_lun *)&unit->fcp_lun);
+	
 	write_lock_irq(&zfcp_data.config_lock);
-	list_for_each_entry(tmp_unit, &port->unit_list_head, list) {
-		if (tmp_unit->scsi_lun != scsi_lun) {
-			found = 1;
-			break;
-		}
-		scsi_lun++;
-	}
-	unit->scsi_lun = scsi_lun;
-	if (found)
-		list_add_tail(&unit->list, &tmp_unit->list);
-	else
-		list_add_tail(&unit->list, &port->unit_list_head);
+	list_add_tail(&unit->list, &port->unit_list_head);
 	atomic_clear_mask(ZFCP_STATUS_COMMON_REMOVE, &unit->status);
 	atomic_set_mask(ZFCP_STATUS_COMMON_RUNNING, &unit->status);
 	write_unlock_irq(&zfcp_data.config_lock);
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 662577f..cf95ad9 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1213,7 +1213,7 @@ static void scsi_sequential_lun_scan(struct scsi_target *starget,
  *     Given a struct scsi_lun of: 0a 04 0b 03 00 00 00 00, this function returns
  *     the integer: 0x0b030a04
  **/
-static int scsilun_to_int(struct scsi_lun *scsilun)
+int scsilun_to_int(struct scsi_lun *scsilun)
 {
 	int i;
 	unsigned int lun;
@@ -1224,6 +1224,7 @@ static int scsilun_to_int(struct scsi_lun *scsilun)
 			      scsilun->scsi_lun[i + 1]) << (i * 8));
 	return lun;
 }
+EXPORT_SYMBOL(scsilun_to_int);

 /**
  * int_to_scsilun: reverts an int into a scsi_lun
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 2f3c5b8..6fe1cf6 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -287,6 +287,7 @@ extern void scsi_target_block(struct device *);
 extern void scsi_target_unblock(struct device *);
 extern void scsi_remove_target(struct device *);
 extern void int_to_scsilun(unsigned int, struct scsi_lun *);
+extern int scsilun_to_int(struct scsi_lun *);
 extern const char *scsi_device_state_name(enum scsi_device_state);
 extern int scsi_is_sdev_device(const struct device *);
 extern int scsi_is_target_device(const struct device *);

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Hannes Reinecke]

Swen Schillig wrote:
> From: Christof Schmitt <christof.schmitt@de.ibm.com>
> 
> When reporting SCSI devices to the SCSI midlayer, use the FCP LUN as
> LUN reported to the SCSI layer. With this approach, zfcp does not have
> to create unique LUNS, and this code can be removed.
> 
> Signed-off-by: Christof Schmitt <christof.schmitt@de.ibm.com>
> Signed-off-by: Swen Schillig <swen@vnet.ibm.com>
> 
Grand. Another zfcp-ism removed.

FWIW:

Signed-off-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
-
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

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Stefan Richter]

Swen Schillig wrote:
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -1213,7 +1213,7 @@ static void scsi_sequential_lun_scan(struct scsi_target *starget,
>   *     Given a struct scsi_lun of: 0a 04 0b 03 00 00 00 00, this function returns
>   *     the integer: 0x0b030a04
>   **/
> -static int scsilun_to_int(struct scsi_lun *scsilun)
> +int scsilun_to_int(struct scsi_lun *scsilun)
>  {
>  	int i;
>  	unsigned int lun;
> @@ -1224,6 +1224,7 @@ static int scsilun_to_int(struct scsi_lun *scsilun)
>  			      scsilun->scsi_lun[i + 1]) << (i * 8));
>  	return lun;
>  }
> +EXPORT_SYMBOL(scsilun_to_int);

This export will be useful for fw-sbp2 too.
-- 
Stefan Richter
-=====-=-=== -==- =--==
http://arcgraph.de/sr/

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Christoph Hellwig]

On Tue, Jun 19, 2007 at 10:25:30AM +0200, Swen Schillig wrote:
> +	unit->scsi_lun = scsilun_to_int((struct scsi_lun *)&unit->fcp_lun);
> +	

fcp_lun is an unsigned long long (and should be a __be64), so casting
this to a struct type is not very nice.  Care to add a version that takes
a __be64 intead?  In fact using that variant in scsi_scan.c might be
benefical aswell, so you could aswell just convert over the existing
scsilun_to_int.

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[James Bottomley]

On Tue, 2007-06-19 at 18:12 +0100, Christoph Hellwig wrote:
> fcp_lun is an unsigned long long (and should be a __be64), so casting
> this to a struct type is not very nice.  Care to add a version that
> takes
> a __be64 intead?  In fact using that variant in scsi_scan.c might be
> benefical aswell, so you could aswell just convert over the existing
> scsilun_to_int. 

Actually, I don't think this is necessary for zfcp: all of these lun
values are input by a user and translated using a strtoull() so it's
going to get highly confusing trying to keep the be64 label [whether
it's desirable for s390 people to be entering BE LUN values is another
matter].  I'd really rather not encourage the use of __be64 u64 for SCSI
luns because it's asking for alignment issues ... instead, the internal
struct scsi_lun contains all the necessary information, and is a stream
of u8 in bus order.

James

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Christof Schmitt]

Now that everybody is happy with using the FCP LUN in the SCSI
midlayer, this raises a question from the user's perspective:

sysfs and lsscsi display the LUN as decimal number. For the FCP LUN
0x401040c300000000, sysfs and lsscsi display this:

$ ls -l /sys/bus/scsi/devices/
total 0
lrwxrwxrwx 1 root root 0 Jun 21 16:49 0:0:0:1086537744 ->
../../../devices/css0/0.0.0010/0.0.181d/host0/rport-0:0-0/target0:0:0/0:0:0:1086537744

$ lsscsi
[0:0:0:1086537744]disk    IBM      2107900          .270  /dev/sda

Asking a system admin to translate the number to hex (0x40C34010) and
then swap the pairs to get the real FCP LUN (0x401040c3) is too much.

Are there any plans to improve this? Showing the LUN in sysfs as hex
number would be the first step. Or does it make more sense to keep the
decimal number in sysfs and let tools like lsscsi do the conversion?

--
Christof Schmitt

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Stefan Richter]

Christof Schmitt wrote:
> sysfs and lsscsi display the LUN as decimal number. For the FCP LUN
> 0x401040c300000000, sysfs and lsscsi display this:
> 
> $ ls -l /sys/bus/scsi/devices/
> total 0
> lrwxrwxrwx 1 root root 0 Jun 21 16:49 0:0:0:1086537744 ->
> ../../../devices/css0/0.0.0010/0.0.181d/host0/rport-0:0-0/target0:0:0/0:0:0:1086537744
> 
> $ lsscsi
> [0:0:0:1086537744]disk    IBM      2107900          .270  /dev/sda

The error here is that these are *not* LUNs.  They are some mangled
derivatives of LUNs.

You probably want that Linux' SCSI mid layer passes the LUNs through and
shows them for example in a representation format as per SAM-4 4.6.2.

Or maybe you want that all the different transport layer implementations
expose their target port identifiers + logical unit identifiers (and
perhaps also initiator port identifiers) in their transport-dependent
formats but in a unified sysfs attribute.

For example, the SCSI midlayer H:C:T:L tuple is useless for
SBP-2-attached devices.  What is useful is the ieee1394_id sysfs
attribute which we expose as a scsi_device's sysfs attribute.  This
attribute was something implementation-defined until now, but I changed
it to the concatenation of target port identifier and logical unit
identifier for Linux 2.6.22, as per SAM(-4) annex A.  (That change is
merely an alternative format for the old sbp2 driver and the only format
for the new fw-sbp2 driver.)

Or did I miss something and there is already a mechanism for transports
to expose target/LU identifiers?  So that userland doesn't have to care
what transport it is, except as far as the details of the identifiers'
formats are concerned?
-- 
Stefan Richter
-=====-=-=== -==- =-=-=
http://arcgraph.de/sr/

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Stefan Richter]

> the SCSI midlayer H:C:T:L

NB, the H, C, T, and L therein are implementation-defined things and not
to be mistaken for actual SCSI identifiers or names.
-- 
Stefan Richter
-=====-=-=== -==- =-=-=
http://arcgraph.de/sr/

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[James Bottomley]

On Thu, 2007-06-21 at 18:46 +0200, Stefan Richter wrote:
> For example, the SCSI midlayer H:C:T:L tuple is useless for
> SBP-2-attached devices.  What is useful is the ieee1394_id sysfs
> attribute which we expose as a scsi_device's sysfs attribute.  This
> attribute was something implementation-defined until now, but I changed
> it to the concatenation of target port identifier and logical unit
> identifier for Linux 2.6.22, as per SAM(-4) annex A.  (That change is
> merely an alternative format for the old sbp2 driver and the only format
> for the new fw-sbp2 driver.)
> 
> Or did I miss something and there is already a mechanism for transports
> to expose target/LU identifiers?  So that userland doesn't have to care
> what transport it is, except as far as the details of the identifiers'
> formats are concerned? 

H:C is really mid-layer defined (although I'd like to get rid of C
eventually).  They really correspond to physical enumeration of the HBA
devices.  T (or I) is the one we think could be abstracted and placed
within the gift of the transport, but so far there's been a lot of
debate with few actual concrete proposals.  L is basically defined by
SAM for every transport, but I'm really unsure how it should be
represented in all its SAM specified glory.

James

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Stefan Richter]

James Bottomley wrote:
> H:C is really mid-layer defined (although I'd like to get rid of C
> eventually).  They really correspond to physical enumeration of the HBA
> devices.  T (or I) is the one we think could be abstracted and placed
> within the gift of the transport, but so far there's been a lot of
> debate with few actual concrete proposals.  L is basically defined by
> SAM for every transport, but I'm really unsure how it should be
> represented in all its SAM specified glory.

If we reduce this discussion to the sysfs representation, then there is
one important point to understand:  The sysfs device path is
implementation-defined.  It always has been, always will be.  That's
inherent to sysfs.

So, having the current implementation-defined H:C:T:L in the sysfs
device path is alright.

Trying to squeeze the SAM LU identifiers and target port identifiers
into the sysfs path might not be such a good idea, exactly because there
is a number of different formats of these identifiers.  We certainly
want to find devices in sysfs in a transport-independent way.

So these identifiers should go somewhere else.  How about exporting
sysfs attributes for each sdev with common names like
	logical_unit_identifier
	target_port_identifier
	initiator_port_identifier
but with transport-dependent contents?  The various transport-layer
implementations can do this without hand-holding of the SCSI core; the
only thing that's important is that all transports use the same names
for these attributes.  The attributes shall be human-readable and can be
used by system management software like udev, hal, lsscsi, whatever.

If SCSI mid-layer wants to control what attribute names are used but
wants to leave the formatting of the contents to the transports, add for
example something like:

struct scsi_transport_template {
	...
	int (* print_logical_unit_identifier)(char *buf,
					struct scsi_device *sdev);
	int (* print_target_port_identifier)(char *buf,
					struct scsi_device *sdev);
	int (* print_initiator_port_identifier)(char *buf,
					struct scsi_device *sdev);
	...
};

Then SCSI mid-layer creates and destroys the attributes and
transport-layer implementations deliver contents for them.  The print_
hooks can also be used for debug printk's or the like if need be.

What's then left of the issues with H:C:T:L are the varying difficulties
that some transports have to fill in some more or less sensible values
in the mid-layer's variables corresponding to H:C:T:L.  Well, actually
the values don't have to make a lot of sense because they are
implementation defined and not for users to draw conclusions from them.

Or is there more to it?
-- 
Stefan Richter
-=====-=-=== -==- =-==-
http://arcgraph.de/sr/

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[James Bottomley]

On Thu, 2007-06-21 at 17:03 +0200, Christof Schmitt wrote:
> Now that everybody is happy with using the FCP LUN in the SCSI
> midlayer, this raises a question from the user's perspective:
> 
> sysfs and lsscsi display the LUN as decimal number. For the FCP LUN
> 0x401040c300000000, sysfs and lsscsi display this:
> 
> $ ls -l /sys/bus/scsi/devices/
> total 0
> lrwxrwxrwx 1 root root 0 Jun 21 16:49 0:0:0:1086537744 ->
> ../../../devices/css0/0.0.0010/0.0.181d/host0/rport-0:0-0/target0:0:0/0:0:0:1086537744
> 
> $ lsscsi
> [0:0:0:1086537744]disk    IBM      2107900          .270  /dev/sda
> 
> Asking a system admin to translate the number to hex (0x40C34010) and
> then swap the pairs to get the real FCP LUN (0x401040c3) is too much.
> 
> Are there any plans to improve this? Showing the LUN in sysfs as hex
> number would be the first step. Or does it make more sense to keep the
> decimal number in sysfs and let tools like lsscsi do the conversion?

A proposal to display the correct form of the LUN would be useful if you
wish to make it? ...  The problem is really that SAM specifies a
possible 4 level structure with 4 possible address methods per level.

The well known LUNs should be simple; there are only three: Report Lun,
Access Controls and Target Log Pages.  The rest we really need input on.
For instance, I could see the vendors wishing us to combine a
multi-level flat addressing space into a single logical unit number,
whereas I could see them wanting us to supply some sort of hierarchy for
the peripheral and logical unit methods of addressing.

Since you're already using 2 level flat addressing, how do you want to
see that represented?

James

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Mike Anderson]

James Bottomley <James.Bottomley@SteelEye.com> wrote:
> A proposal to display the correct form of the LUN would be useful if you
> wish to make it? ...  The problem is really that SAM specifies a
> possible 4 level structure with 4 possible address methods per level.
> 
> The well known LUNs should be simple; there are only three: Report Lun,
> Access Controls and Target Log Pages.  The rest we really need input on.
> For instance, I could see the vendors wishing us to combine a
> multi-level flat addressing space into a single logical unit number,
> whereas I could see them wanting us to supply some sort of hierarchy for
> the peripheral and logical unit methods of addressing.
> 
> Since you're already using 2 level flat addressing, how do you want to
> see that represented?

James, why would we would not want to display the lun per SAM4 4.6.2 as
suggested by Stefan in a previous comment.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Christof Schmitt]

On Thu, Jun 21, 2007 at 09:40:35PM -0700, Mike Anderson wrote:
> James Bottomley <James.Bottomley@SteelEye.com> wrote:
> > A proposal to display the correct form of the LUN would be useful if you
> > wish to make it? ...  The problem is really that SAM specifies a
> > possible 4 level structure with 4 possible address methods per level.
> > 
> > The well known LUNs should be simple; there are only three: Report Lun,
> > Access Controls and Target Log Pages.  The rest we really need input on.
> > For instance, I could see the vendors wishing us to combine a
> > multi-level flat addressing space into a single logical unit number,
> > whereas I could see them wanting us to supply some sort of hierarchy for
> > the peripheral and logical unit methods of addressing.
> > 
> > Since you're already using 2 level flat addressing, how do you want to
> > see that represented?
> 
> James, why would we would not want to display the lun per SAM4 4.6.2 as
> suggested by Stefan in a previous comment.

I would also agree to display the LUN in sysfs as 64 bit hex number as
advised in SAM4.

While the SAM describes different formats and addressing schemes, i
don't see how this affects the SCSI layer in the kernel. The kernel
gets the 64 bit LUNs and uses them as unique ids. 4.6.1 in SAM4 says
in the last sentence, that different values for the 64 bits represent
different LUNs, so the assumption of the LUNs being unique is
guaranteed.

My point of view is mainly from the device driver's (zfcp) and user's
point of view. It is only that the disk systems i deal with use the 2
level addressing in the LUNs. The user interface of the disk system
displays something like: Volume F000 is exported as FCP LUN 40F04000.
When the SCSI midlayer uses and exports this value in sysfs, it should
be displayed in the same format (hex).

I don't know if the multi level scheme is actually used as hierarchy
somewhere. If somebody would be interested in using this information,
i think a userspace tool could grab the value from sysfs and display
the detailed information according to SAM.

--
Christof Schmitt

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[James Bottomley]

On Thu, 2007-06-21 at 21:40 -0700, Mike Anderson wrote:
> James Bottomley <James.Bottomley@SteelEye.com> wrote:
> > A proposal to display the correct form of the LUN would be useful if you
> > wish to make it? ...  The problem is really that SAM specifies a
> > possible 4 level structure with 4 possible address methods per level.
> > 
> > The well known LUNs should be simple; there are only three: Report Lun,
> > Access Controls and Target Log Pages.  The rest we really need input on.
> > For instance, I could see the vendors wishing us to combine a
> > multi-level flat addressing space into a single logical unit number,
> > whereas I could see them wanting us to supply some sort of hierarchy for
> > the peripheral and logical unit methods of addressing.
> > 
> > Since you're already using 2 level flat addressing, how do you want to
> > see that represented?
> 
> James, why would we would not want to display the lun per SAM4 4.6.2 as
> suggested by Stefan in a previous comment.

Because in a two LUN system, what was LUN 1 would then become LUN
0x1000000000000 which looks a bit unpalatable.

James

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Doug Maxey]

On Fri, 22 Jun 2007 09:11:39 CDT, James Bottomley wrote:
> On Thu, 2007-06-21 at 21:40 -0700, Mike Anderson wrote:
> > James Bottomley <James.Bottomley@SteelEye.com> wrote:
> > > A proposal to display the correct form of the LUN would be useful if you
> > > wish to make it? ...  The problem is really that SAM specifies a
> > > possible 4 level structure with 4 possible address methods per level.
> > > 
> > > The well known LUNs should be simple; there are only three: Report Lun,
> > > Access Controls and Target Log Pages.  The rest we really need input on.
> > > For instance, I could see the vendors wishing us to combine a
> > > multi-level flat addressing space into a single logical unit number,
> > > whereas I could see them wanting us to supply some sort of hierarchy for
> > > the peripheral and logical unit methods of addressing.
> > > 
> > > Since you're already using 2 level flat addressing, how do you want to
> > > see that represented?
> > 
> > James, why would we would not want to display the lun per SAM4 4.6.2 as
> > suggested by Stefan in a previous comment.
> 
> Because in a two LUN system, what was LUN 1 would then become LUN
> 0x1000000000000 which looks a bit unpalatable.
> 

Just my $.02.

If you use a pseries, and have to specifiy the LUN to the OFW on the later 
releases, you must train the fingers to type the extra 12 chars.  

Iy you are watching an iSCSI target with tcpdump, you can observe the 
PDU fields with the lun encoded thusly.   It may not be historical, but 
is certainly more accurate to use the 0x1000000000000 as 64 bit (or 
more correctly 8 bytes) encoded as hex.

++doug

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Luben Tuikov]

--- Doug Maxey <dwm@enoyolf.org> wrote:
> On Fri, 22 Jun 2007 09:11:39 CDT, James Bottomley wrote:
> > On Thu, 2007-06-21 at 21:40 -0700, Mike Anderson wrote:
> > > James Bottomley <James.Bottomley@SteelEye.com> wrote:
> > > > A proposal to display the correct form of the LUN would be useful if you
> > > > wish to make it? ...  The problem is really that SAM specifies a
> > > > possible 4 level structure with 4 possible address methods per level.
> > > > 
> > > > The well known LUNs should be simple; there are only three: Report Lun,
> > > > Access Controls and Target Log Pages.  The rest we really need input on.
> > > > For instance, I could see the vendors wishing us to combine a
> > > > multi-level flat addressing space into a single logical unit number,
> > > > whereas I could see them wanting us to supply some sort of hierarchy for
> > > > the peripheral and logical unit methods of addressing.
> > > > 
> > > > Since you're already using 2 level flat addressing, how do you want to
> > > > see that represented?
> > > 
> > > James, why would we would not want to display the lun per SAM4 4.6.2 as
> > > suggested by Stefan in a previous comment.
> > 
> > Because in a two LUN system, what was LUN 1 would then become LUN
> > 0x1000000000000 which looks a bit unpalatable.
> > 
> 
> Just my $.02.
> 
> If you use a pseries, and have to specifiy the LUN to the OFW on the later 
> releases, you must train the fingers to type the extra 12 chars.  
> 
> Iy you are watching an iSCSI target with tcpdump, you can observe the 
> PDU fields with the lun encoded thusly.   It may not be historical, but 
> is certainly more accurate to use the 0x1000000000000 as 64 bit (or 
> more correctly 8 bytes) encoded as hex.

Indeed it is more accurate to represent LUNs in 64 bit format.

It is even more accurate to represent them as u8 LUN[8], and possibly
print them as
  "0x%016llx"   ((unsigned long long) be64_to_cpu(*(__be64 *)(LUN))).

What I haven't been able to figure out is the source of the insistence
of bottomley et al., that SCSI Core should interpret the LUN structure
and that it should transform it into an integer entity, such that the
LUN space is enumerable.

But this discussion isn't new.  This is the 6th year that someone
brings the SCSU Core's LUN subject up.

I think the professional part of this audience would recognize the
significance of the intended entity which has interest in the
LUN structure when designing their storage (protocol) stacks.

   Luben

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Stefan Richter]

Luben Tuikov wrote:
> Indeed it is more accurate to represent LUNs in 64 bit format.
> 
> It is even more accurate to represent them as u8 LUN[8], and possibly
> print them as
>   "0x%016llx"   ((unsigned long long) be64_to_cpu(*(__be64 *)(LUN))).

Might need a little precaution on some architectures.

--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -249,7 +249,7 @@ struct ccs_modesel_head {
  */
 struct scsi_lun {
        __u8 scsi_lun[8];
-};
+} __attribute__((aligned(8)));

 /*
  *  MESSAGE CODES
-- 
Stefan Richter
-=====-=-=== -==- ==-=-
http://arcgraph.de/sr/

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Swen Schillig]

On Friday 22 June 2007 16:11, James Bottomley wrote:
> On Thu, 2007-06-21 at 21:40 -0700, Mike Anderson wrote:
> > James Bottomley <James.Bottomley@SteelEye.com> wrote:
> > > A proposal to display the correct form of the LUN would be useful if you
> > > wish to make it? ...  The problem is really that SAM specifies a
> > > possible 4 level structure with 4 possible address methods per level.
> > > 
> > > The well known LUNs should be simple; there are only three: Report Lun,
> > > Access Controls and Target Log Pages.  The rest we really need input on.
> > > For instance, I could see the vendors wishing us to combine a
> > > multi-level flat addressing space into a single logical unit number,
> > > whereas I could see them wanting us to supply some sort of hierarchy for
> > > the peripheral and logical unit methods of addressing.
> > > 
> > > Since you're already using 2 level flat addressing, how do you want to
> > > see that represented?
> > 
> > James, why would we would not want to display the lun per SAM4 4.6.2 as
> > suggested by Stefan in a previous comment.
> 
> Because in a two LUN system, what was LUN 1 would then become LUN
> 0x1000000000000 which looks a bit unpalatable.
> 
> James
> 
> 
> -

Despite the issue whether we should display and/or use (sysfs !?)
full blown 64bit values, so with leading zeros, you still have the option to display, 
for the single level addressing, the LUN as a 2 byte value as described in  SAM4 4.6.2. 
So for your example this would be 0x0001 or 0x1, which is exactly what you'd like to see, right ?
Only for 2+ level environment the 64bit representation comes into the play.

And, because you were asking how we'd like that to be represented, I personally prefer
a full blown 64bit hex value with leading zeros.
It makes the comparison to internal structures/models easier and gets us a bit closer
to the documented (SAM4) standard.
...and I think we all can deal with a few extra digits being displayed.

Cheers Swen

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Hannes Reinecke]

Swen Schillig wrote:
> On Friday 22 June 2007 16:11, James Bottomley wrote:
>> On Thu, 2007-06-21 at 21:40 -0700, Mike Anderson wrote:
>>> James Bottomley <James.Bottomley@SteelEye.com> wrote:
>>>> A proposal to display the correct form of the LUN would be useful if you
>>>> wish to make it? ...  The problem is really that SAM specifies a
>>>> possible 4 level structure with 4 possible address methods per level.
>>>>
>>>> The well known LUNs should be simple; there are only three: Report Lun,
>>>> Access Controls and Target Log Pages.  The rest we really need input on.
>>>> For instance, I could see the vendors wishing us to combine a
>>>> multi-level flat addressing space into a single logical unit number,
>>>> whereas I could see them wanting us to supply some sort of hierarchy for
>>>> the peripheral and logical unit methods of addressing.
>>>>
>>>> Since you're already using 2 level flat addressing, how do you want to
>>>> see that represented?
>>> James, why would we would not want to display the lun per SAM4 4.6.2 as
>>> suggested by Stefan in a previous comment.
>> Because in a two LUN system, what was LUN 1 would then become LUN
>> 0x1000000000000 which looks a bit unpalatable.
>>
>> James
>>
>>
>> -
> 
> Despite the issue whether we should display and/or use (sysfs !?)
> full blown 64bit values, so with leading zeros, you still have the option to display, 
> for the single level addressing, the LUN as a 2 byte value as described in  SAM4 4.6.2. 
> So for your example this would be 0x0001 or 0x1, which is exactly what you'd like to see, right ?
> Only for 2+ level environment the 64bit representation comes into the play.
> 
> And, because you were asking how we'd like that to be represented, I personally prefer
> a full blown 64bit hex value with leading zeros.
> It makes the comparison to internal structures/models easier and gets us a bit closer
> to the documented (SAM4) standard.
> ...and I think we all can deal with a few extra digits being displayed.
> 
Well ...
why don't we stick with the original implementation like zfcp had it?
We can simpley expand the midlayer to add an attribute 'lun'
to each scsi_device. This would be the LUN as returned by eg
REPORT LUNS.
No translation, nothing. Would be easy to implement and would allow
any administrator to map the h:c:i:l value to the 'real' lun.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
-
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

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Stefan Richter]

Hannes Reinecke wrote:
> why don't we stick with the original implementation like zfcp had it?
> We can simpley expand the midlayer to add an attribute 'lun'
> to each scsi_device. This would be the LUN as returned by eg
> REPORT LUNS.
> No translation, nothing. Would be easy to implement and would allow
> any administrator to map the h:c:i:l value to the 'real' lun.

I would also like to have the Target Port identifier in there.  This, in
combination with the LUN alias Logical Unit identifier is useful for all
kinds of persistent device mapping.

The LUN could be the same data format (and display format) for all
transports == 8 bytes.  SBP-3's native format is 2 bytes but we can
transform and print it in the 8 bytes format; shouldn't hurt.

Actually, the SCSI midlayer integer 'l' can be used as internal
intermediary data format as long as only those LUN variants are in
real-world use which can be losslessly transformed to and from SCSI
midlayer's integer 'l'.

The transport identifiers have transport-dependent data sizes, and the
display formats are not standardized.  So that's requiring a little bit
more care than the LUNs but it isn't overly complicated either.

So,
  - SCSI mid layer should maintain sysfs attributes for each sdev which
    show Logical Unit identifier and Target Port identifier.

  - Do we pass the LUN through to SCSI midlayer's sysfs code via the
    theoretically lossy scsilun_to_int/ int_to_scsilun transformation?

  - Do we pass the target port identifier through as a data member in
    struct scsi_device, or as a hook "int (* print_target_port_id)(..);"
    in struct scsi_transport_template?

  - Do we also want initiator port identifier, initiator port name,
    target port name, LU name?  If so, create a subdirectory for that
    whole bunch of additional attributes?  (Most of these are just
    optional.  In case of SBP-2/3, the initiator port identifier is
    totally uninteresting to userspace; the initiator port name is of
    mild interest but can be retrieved elsewhere from sysfs by some
    black magic.)

I am slowly working on loosely related stuff in the fw-sbp2
transport-layer driver and could produce a respective patch for the
midlayer infrastructure along the way, if nobody else does before me.
(Next weekend at the earliest.)
-- 
Stefan Richter
-=====-=-=== -==- ==--=
http://arcgraph.de/sr/

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Luben Tuikov]

--- Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> Hannes Reinecke wrote:
> > why don't we stick with the original implementation like zfcp had it?
> > We can simpley expand the midlayer to add an attribute 'lun'
> > to each scsi_device. This would be the LUN as returned by eg
> > REPORT LUNS.
> > No translation, nothing. Would be easy to implement and would allow
> > any administrator to map the h:c:i:l value to the 'real' lun.

This would be the best way to go (what Hannes said).
 
> I would also like to have the Target Port identifier in there.  This, in

Well this is not correct because the TPI's format and representation
is transport dependent and because it is a property of the SCSI Target,
not the SCSI device (struct scsi device).  And currently SCSI Core
has no representation of SCSI Targets.

SCSI Core can have a burden if it needs to know about TPIs.

Ideally SCSI Core is presented with an opaque token (void *, unsigned long (long),
etc) which represents a target port, which it uses to send SPC commands
to the target to find out how many if any LUs the target has, INQUIRY them,
and then create struct scsi_dev for each LU.

When presented with this opaque token*, SCSI Core allocates and
initializes a struct scsi_target, whose children are LUs, later
to be discovered by SCSI Core.  SCSI Core then uses that opaque token
to ask the transport protocol layer to send commands to the I_T_L
nexus (WLUN or LUN if it knows it from previous discovery attempt).

* Note this means that there is no "active" scanning on part
of the SCSI Core.  This belongs to the transport layer.
(And thus the "concept" of "async-scanning" recently introduced
to SCSI Core, becomes moot.)

> combination with the LUN alias Logical Unit identifier is useful for all
> kinds of persistent device mapping.

The OS distros have wanted this for a long time, but they've
correctly wanted properties/attribues of the _device_, which can
be found by the various INQUIRY facilities.

That is, how you get to the device isn't important, what is
important is the device itself.

Ideally that would be the SCSI Device Name, which is also transport
dependent, but using memcmp() or strcmp() shouldn't be that bad.

> The LUN could be the same data format (and display format) for all
> transports == 8 bytes.  SBP-3's native format is 2 bytes but we can
> transform and print it in the 8 bytes format; shouldn't hurt.

Yes, a LUN is u8[8] and SCSI Core should absolutely never care
about what it actually is.  It should just pass it around
unaltered.

> Actually, the SCSI midlayer integer 'l' can be used as internal
> intermediary data format as long as only those LUN variants are in
> real-world use which can be losslessly transformed to and from SCSI
> midlayer's integer 'l'.
> 
> The transport identifiers have transport-dependent data sizes, and the
> display formats are not standardized.  So that's requiring a little bit
> more care than the LUNs but it isn't overly complicated either.
> 
> So,
>   - SCSI mid layer should maintain sysfs attributes for each sdev which
>     show Logical Unit identifier and Target Port identifier.
> 
>   - Do we pass the LUN through to SCSI midlayer's sysfs code via the
>     theoretically lossy scsilun_to_int/ int_to_scsilun transformation?
> 
>   - Do we pass the target port identifier through as a data member in
>     struct scsi_device, or as a hook "int (* print_target_port_id)(..);"
>     in struct scsi_transport_template?
> 
>   - Do we also want initiator port identifier, initiator port name,
>     target port name, LU name?  If so, create a subdirectory for that
>     whole bunch of additional attributes?  (Most of these are just
>     optional.  In case of SBP-2/3, the initiator port identifier is
>     totally uninteresting to userspace; the initiator port name is of
>     mild interest but can be retrieved elsewhere from sysfs by some
>     black magic.)

I'd try to _shrink_ SCSI Core and get it out of the game of knowing
anything about formats/representation/print methods/protocol dependent
stuff/etc.  The smaller you get SCSI Core, the better off it is.

   Luben

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Stefan Richter]

Luben Tuikov wrote:
> --- Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>> I would also like to have the Target Port identifier in there.

("in there" = in sysfs)

> Well this is not correct because the TPI's format and representation
> is transport dependent

My thought was that SCSI core's role would only be to create the
respective sysfs attributes (thus enforcing uniform naming of the
attributes), but that the transport layer implementations are
responsible to feed string representations there; in one way or another.

> and because it is a property of the SCSI Target,
> not the SCSI device (struct scsi device).  And currently SCSI Core
> has no representation of SCSI Targets.

And because SCSI targets are not represented in sysfs, I'd lazily add
target-related information to the LU's sysfs representation.

Of course, if a representation of the target in sysfs is preferred, we
should add this before tainting the LU's sysfs representation with
attributes that belong to the target.

So, would people like targets reflected as sysfs devices ( = parent of
any LU devices)?  We should be able to add or remove parent devices into
sysfs device paths without breaking userspace, says Kay Sievers at the
end of http://lkml.org/lkml/2007/6/8/491 .

[...]
> Ideally SCSI Core is presented with an opaque token (void *, unsigned long (long),
> etc) which represents a target port, which it uses to send SPC commands
> to the target to find out how many if any LUs the target has, INQUIRY them,
> and then create struct scsi_dev for each LU.
> 
> When presented with this opaque token*, SCSI Core allocates and
> initializes a struct scsi_target, whose children are LUs, later
> to be discovered by SCSI Core.  SCSI Core then uses that opaque token
> to ask the transport protocol layer to send commands to the I_T_L
> nexus (WLUN or LUN if it knows it from previous discovery attempt).

Won't work with SBP-2/3.  There the LUs are discovered via IEEE 1212
mechanisms (configuration ROM) rather than SPC.  Plus there is SBP-2/3
login protocol.
-- 
Stefan Richter
-=====-=-=== -==- ==--=
http://arcgraph.de/sr/

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Luben Tuikov]

--- Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> My thought was that SCSI core's role would only be to create the
> respective sysfs attributes (thus enforcing uniform naming of the
> attributes), but that the transport layer implementations are
> responsible to feed string representations there; in one way or another.

This way one gets cross-contamination of object properties.
But I guess SCSI Core doesn't have to be perfect, consistent
or complete.  Business-wise it's much better if it weren't.

> > and because it is a property of the SCSI Target,
> > not the SCSI device (struct scsi device).  And currently SCSI Core
> > has no representation of SCSI Targets.
> 
> And because SCSI targets are not represented in sysfs, I'd lazily add
> target-related information to the LU's sysfs representation.

LOL, okay. :-)

> Of course, if a representation of the target in sysfs is preferred, we
> should add this before tainting the LU's sysfs representation with
> attributes that belong to the target.

I didn't have sysfs/ in mind.  I had internal representation of
the external world (storage domain) in mind.  Then sysfs/ would
follow suit and represent the internal picture, thus representing
the physical world of the entities and their relationship in the
storage domain.  (Just like it is done in my SAS/SATL stack.)

I guess either way this is done sysfs->SCSI Core, or SCSI Core->sysfs,
it's all good for business.

> So, would people like targets reflected as sysfs devices ( = parent of
> any LU devices)?  We should be able to add or remove parent devices into

I had actually always wanted full scsi target support in SCSI Core first,
and then SCSI Core to discover the LUs (i.e. struct scsi_dev) and register
them with the block layer.

This greatly simplifies low(er) level transport storage stacks design
whereby the only entity "registered" with SCSI Core is the scsi target.

It also simplifies the SAM event model which currently needs to be
hackishly emulated by the transport stacks, as opposed to just
passing the event to SCSI Core where it belongs and let it handle it.

> sysfs device paths without breaking userspace, says Kay Sievers at the
> end of http://lkml.org/lkml/2007/6/8/491 .

That is an excellent write-up!

> > Ideally SCSI Core is presented with an opaque token (void *, unsigned long (long),
> > etc) which represents a target port, which it uses to send SPC commands
> > to the target to find out how many if any LUs the target has, INQUIRY them,
> > and then create struct scsi_dev for each LU.
> > 
> > When presented with this opaque token*, SCSI Core allocates and
> > initializes a struct scsi_target, whose children are LUs, later
> > to be discovered by SCSI Core.  SCSI Core then uses that opaque token
> > to ask the transport protocol layer to send commands to the I_T_L
> > nexus (WLUN or LUN if it knows it from previous discovery attempt).
> 
> Won't work with SBP-2/3.  There the LUs are discovered via IEEE 1212
> mechanisms (configuration ROM) rather than SPC.  Plus there is SBP-2/3
> login protocol.

And there are others too.  The model mentioned above isn't absolute.
It makes sense to leave a "backdoor" where transport stacks can register
an LU themselves, possibly pointing to an already registered scsi target
(opaque token).

   Luben

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Matthew Wilcox]

On Mon, Jun 25, 2007 at 02:27:01PM -0700, Luben Tuikov wrote:
> Ideally SCSI Core is presented with an opaque token (void *, unsigned long (long),
> etc) which represents a target port, which it uses to send SPC commands
> to the target to find out how many if any LUs the target has, INQUIRY them,
> and then create struct scsi_dev for each LU.
> 
> When presented with this opaque token*, SCSI Core allocates and
> initializes a struct scsi_target, whose children are LUs, later
> to be discovered by SCSI Core.  SCSI Core then uses that opaque token
> to ask the transport protocol layer to send commands to the I_T_L
> nexus (WLUN or LUN if it knows it from previous discovery attempt).
> 
> * Note this means that there is no "active" scanning on part
> of the SCSI Core.  This belongs to the transport layer.
> (And thus the "concept" of "async-scanning" recently introduced
> to SCSI Core, becomes moot.)

Not really.  The async scanning code is there to ensure that scsi
devices get the same name as they would if the devices were scanned
one after another.  We can make all this code moot if we determine that
we're willing to accept a world where 'sda' may refer to any random scsi
disc.

I'd be quite happy to move the code that does the "active scanning" to
scsi_transport_spi.c, but right now that would be quite a bit of extra
code for drivers which haven't been converted to use scsi_transport_spi
yet.  My nefarious plan is to convert drivers to use it, then move that
code there.  I do agreee with you that SPI-free systems shouldn't have
to carry that legacy code around (but actually, it's very little code,
so this is more of a purity thing than a real concern).

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[James Bottomley]

On Mon, 2007-06-25 at 20:57 +0200, Stefan Richter wrote:
> Hannes Reinecke wrote:
> > why don't we stick with the original implementation like zfcp had it?
> > We can simpley expand the midlayer to add an attribute 'lun'
> > to each scsi_device. This would be the LUN as returned by eg
> > REPORT LUNS.
> > No translation, nothing. Would be easy to implement and would allow
> > any administrator to map the h:c:i:l value to the 'real' lun.
> 
> I would also like to have the Target Port identifier in there.  This, in
> combination with the LUN alias Logical Unit identifier is useful for all
> kinds of persistent device mapping.

Really, no.  Target port is a target, not a LUN property; worse, it's
potentially transport defined.  The way target ports are currently shown
is by the relevant transport class as per target things (see the FC
transport for an example).  There's no real reason the mid-layer has to
know about these.

> The LUN could be the same data format (and display format) for all
> transports == 8 bytes.  SBP-3's native format is 2 bytes but we can
> transform and print it in the 8 bytes format; shouldn't hurt.
> 
> Actually, the SCSI midlayer integer 'l' can be used as internal
> intermediary data format as long as only those LUN variants are in
> real-world use which can be losslessly transformed to and from SCSI
> midlayer's integer 'l'.

You're sort of confusing what we display as the LUN and what we
represent it as internally (admittedly they're the same at the moment).
The object would be to separate these and the debate is really about
what to display.  I think the number for single level simple peripheral
addressing bus==0 or flat addressing and the full 64 bit entity
otherwise seems to be what people are converging on.

> The transport identifiers have transport-dependent data sizes, and the
> display formats are not standardized.  So that's requiring a little bit
> more care than the LUNs but it isn't overly complicated either.
> 
> So,
>   - SCSI mid layer should maintain sysfs attributes for each sdev which
>     show Logical Unit identifier and Target Port identifier.

LUN possibly, TPID no; the transport class does that one.

>   - Do we pass the LUN through to SCSI midlayer's sysfs code via the
>     theoretically lossy scsilun_to_int/ int_to_scsilun transformation?

Absolutely ... otherwise you won't get the correct representation when
that gets decided.

>   - Do we pass the target port identifier through as a data member in
>     struct scsi_device, or as a hook "int (* print_target_port_id)(..);"
>     in struct scsi_transport_template?
> 
>   - Do we also want initiator port identifier, initiator port name,
>     target port name, LU name?  If so, create a subdirectory for that
>     whole bunch of additional attributes?  (Most of these are just
>     optional.  In case of SBP-2/3, the initiator port identifier is
>     totally uninteresting to userspace; the initiator port name is of
>     mild interest but can be retrieved elsewhere from sysfs by some
>     black magic.)

Again, no ... these are transport dependent quantities and belong in the
transport class (if they're relevant to the transport).

> I am slowly working on loosely related stuff in the fw-sbp2
> transport-layer driver and could produce a respective patch for the
> midlayer infrastructure along the way, if nobody else does before me.
> (Next weekend at the earliest.)

James

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Stefan Richter]

James Bottomley wrote:
> On Mon, 2007-06-25 at 20:57 +0200, Stefan Richter wrote:
>> I would also like to have the Target Port identifier in there.  This, in
>> combination with the LUN alias Logical Unit identifier is useful for all
>> kinds of persistent device mapping.
> 
> Really, no.  Target port is a target, not a LUN property; worse, it's
> potentially transport defined.  The way target ports are currently shown
> is by the relevant transport class as per target things (see the FC
> transport for an example).  There's no real reason the mid-layer has to
> know about these.

Do all these transports use the same *name* for the attribute holding
the target port identifier's?  In other words, is userspace able to find
the target port identifier without knowing which transport is at work?
(It's a bit difficult for me to check for myself, not having any other
SCSI hardware than SBP-2 at hand.)

>> The LUN could be the same data format (and display format) for all
>> transports == 8 bytes.  SBP-3's native format is 2 bytes but we can
>> transform and print it in the 8 bytes format; shouldn't hurt.
>>
>> Actually, the SCSI midlayer integer 'l' can be used as internal
>> intermediary data format as long as only those LUN variants are in
>> real-world use which can be losslessly transformed to and from SCSI
>> midlayer's integer 'l'.
> 
> You're sort of confusing what we display as the LUN and what we
> represent it as internally (admittedly they're the same at the moment).

No, I'm not. :-)

(Actually we currently don't display the LUN at all.  There is something
in the device path name which more or rather less resembles the LUN
after some rough treatment.  But it's not a LUN; and as I said in
another post:  We need the LUN in sysfs, but we don't need it as
component of the device path.)

> The object would be to separate these and the debate is really about
> what to display.

Yes. :-)

[...]
>> The transport identifiers have transport-dependent data sizes, and the
>> display formats are not standardized.  So that's requiring a little bit
>> more care than the LUNs but it isn't overly complicated either.
>>
>> So,
>>   - SCSI mid layer should maintain sysfs attributes for each sdev which
>>     show Logical Unit identifier and Target Port identifier.
> 
> LUN possibly, TPID no; the transport class does that one.

Regarding the TPID:  The /how/ should be left to the transport layer
implementation; but the /where/ should be uniform in all transports.

Besides, the transports don't manage the lifetime of the sysfs device
which represent Logical Units.  Why not let the SCSI core also manage
the lifetime of sysfs attributes (or to-be-implemented sysfs devices)
which represent targets?  The concept of target is
transport-independent; merely some of their properties look different
from transport to transport.

>>   - Do we pass the LUN through to SCSI midlayer's sysfs code via the
>>     theoretically lossy scsilun_to_int/ int_to_scsilun transformation?
> 
> Absolutely ... otherwise you won't get the correct representation when
> that gets decided.
> 
>>   - Do we pass the target port identifier through as a data member in
>>     struct scsi_device, or as a hook "int (* print_target_port_id)(..);"
>>     in struct scsi_transport_template?
>>
>>   - Do we also want initiator port identifier, initiator port name,
>>     target port name, LU name?  If so, create a subdirectory for that
>>     whole bunch of additional attributes?  (Most of these are just
>>     optional.  In case of SBP-2/3, the initiator port identifier is
>>     totally uninteresting to userspace; the initiator port name is of
>>     mild interest but can be retrieved elsewhere from sysfs by some
>>     black magic.)
> 
> Again, no ... these are transport dependent quantities and belong in the
> transport class (if they're relevant to the transport).

"Yes, but" on the how and the where as above.
-- 
Stefan Richter
-=====-=-=== -==- ==-=-
http://arcgraph.de/sr/

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Luben Tuikov]

--- Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> Do all these transports use the same *name* for the attribute holding
> the target port identifier's?

Sure, it is "tpid".

> In other words, is userspace able to find
> the target port identifier without knowing which transport is at work?

How can that be?  The target port identifier is by definition
a transport property?

"Userspace" which you have in mind will be more interested in the
device name and other ID properties as returned by the INQUIRY
facilities.  Some are transport specific some are not.

Either way userspace can follow a pointer from the sysfs device
entry to the transport, just as sg-utils and lsscsi does now for
the SAS Stack (my version of it at least).

> Regarding the TPID:  The /how/ should be left to the transport layer
> implementation; but the /where/ should be uniform in all transports.

This maybe hard to do since the structure of the domain is different
for different protocols.

Of course this can be hacked by using symlinks...

   Luben

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Stefan Richter]

Luben Tuikov wrote:
> --- Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>> Do all these transports use the same *name* for the attribute holding
>> the target port identifier's?
> 
> Sure, it is "tpid".

In mainline's transport classes?  Or/and in your stack?  OK, I'm lazy, I
should look into the source.

>> In other words, is userspace able to find
>> the target port identifier without knowing which transport is at work?
> 
> How can that be?  The target port identifier is by definition
> a transport property?

Target Port Identifier is a property of targets.

Only /how its value looks like/ depends on transport protocols.  Doesn't it?

So let's put the can always in the same shelf, regardless of the flavor
of the soup in the can.

(That's really why I joined the discussion.  We already have all
userspace requirements covered in sbp2, regarding which properties to
expose how.  Except that we do it in sbp2's own place rather than a
place common to all transport layer implementations.)

> "Userspace" which you have in mind will be more interested in the
> device name and other ID properties as returned by the INQUIRY
> facilities.  Some are transport specific some are not.
> 
> Either way userspace can follow a pointer from the sysfs device
> entry to the transport, just as sg-utils and lsscsi does now for
> the SAS Stack (my version of it at least).

As a side note:  What I said was because I'm a lot SBP-2/3 biased and
didn't deal with other transports myself yet.  In SBP-2/3 we are only
interested in LUs, we are interested in the concatenation of TPI and LUN
for worldwide unique persistent identification of LUs, we get both from
IEEE 1212 facilities, and SPC support is not so extensive.

>> Regarding the TPID:  The /how/ should be left to the transport layer
>> implementation; but the /where/ should be uniform in all transports.
> 
> This maybe hard to do since the structure of the domain is different
> for different protocols.
> 
> Of course this can be hacked by using symlinks...

If there are going to be sysfs representations of targets, i.e. target
devices, can't we express the target--LU relationships as parent device
relationships?  Could also be grand-(grand-)parent device relationships.

If neither is possible with some transports, then we indeed have to
resort to explicit attributes, e.g. symlinks.
-- 
Stefan Richter
-=====-=-=== -==- ==-=-
http://arcgraph.de/sr/

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[James Bottomley]

On Tue, 2007-06-26 at 12:44 +0200, Stefan Richter wrote:
> Luben Tuikov wrote:
> > --- Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> >> Do all these transports use the same *name* for the attribute holding
> >> the target port identifier's?
> > 
> > Sure, it is "tpid".
> 
> In mainline's transport classes?  Or/and in your stack?  OK, I'm lazy, I
> should look into the source.

It's currently port_name and port_id in the FC transport.

> >> In other words, is userspace able to find
> >> the target port identifier without knowing which transport is at work?
> > 
> > How can that be?  The target port identifier is by definition
> > a transport property?
> 
> Target Port Identifier is a property of targets.
> 
> Only /how its value looks like/ depends on transport protocols.  Doesn't it?

Which is why it's obtained by the transport class

> So let's put the can always in the same shelf, regardless of the flavor
> of the soup in the can.

And why it's placed in the target directory.

> (That's really why I joined the discussion.  We already have all
> userspace requirements covered in sbp2, regarding which properties to
> expose how.  Except that we do it in sbp2's own place rather than a
> place common to all transport layer implementations.)
> 
> > "Userspace" which you have in mind will be more interested in the
> > device name and other ID properties as returned by the INQUIRY
> > facilities.  Some are transport specific some are not.
> > 
> > Either way userspace can follow a pointer from the sysfs device
> > entry to the transport, just as sg-utils and lsscsi does now for
> > the SAS Stack (my version of it at least).
> 
> As a side note:  What I said was because I'm a lot SBP-2/3 biased and
> didn't deal with other transports myself yet.  In SBP-2/3 we are only
> interested in LUs, we are interested in the concatenation of TPI and LUN
> for worldwide unique persistent identification of LUs, we get both from
> IEEE 1212 facilities, and SPC support is not so extensive.
> 
> >> Regarding the TPID:  The /how/ should be left to the transport layer
> >> implementation; but the /where/ should be uniform in all transports.
> > 
> > This maybe hard to do since the structure of the domain is different
> > for different protocols.
> > 
> > Of course this can be hacked by using symlinks...
> 
> If there are going to be sysfs representations of targets, i.e. target
> devices, can't we express the target--LU relationships as parent device
> relationships?  Could also be grand-(grand-)parent device relationships.
> 
> If neither is possible with some transports, then we indeed have to
> resort to explicit attributes, e.g. symlinks.

OK, you've lost me ... this is our current sysfs representation of a
disk:

/sys/devices/pci0000:00/0000:00:1f.1/host0/target0:0:0/0:0:0:0

target0:0:0 is what I think of as the target ... is that different from
what you're asking for?

James

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Stefan Richter]

James Bottomley wrote:
> On Tue, 2007-06-26 at 12:44 +0200, Stefan Richter wrote:
>> If there are going to be sysfs representations of targets,
[...]
> OK, you've lost me ... this is our current sysfs representation of a
> disk:
> 
> /sys/devices/pci0000:00/0000:00:1f.1/host0/target0:0:0/0:0:0:0
> 
> target0:0:0 is what I think of as the target ...

Strange.  How did I miss this?  #-|

Actually I missed this because I only looked into linux/include/scsi/*.h
and found

struct scsi_device {
	...
	struct scsi_target *sdev_target; /* used only for single_lun */
	...
}

Is the comment wrong or is sdev_target something different?

> is that different from what you're asking for?

That's part of what I asked for.  I also thought of SCSI core populating
it with certain attributes; let's call them port_name and port_id.  To
fill values into these attributes, transport layer
drivers/classes/whatever supply to SCSI core either strings or functions
which print strings into buffers.  ->  SCSI core controls that these
properties go into host*/target*:*:*/port_{name,id}.  Transports control
how the values of these attributes look.

If that's not wanted, then what is the way?  Simply the following code
in a transport driver/class/whatever?

	target_gendev = get_device(sdev->sdev_gendev.parent);
	/* error check omitted */
	err = device_create_file(target_gendev, &my_port_name_attr);
	/* error check omitted */
	err = device_create_file(target_gendev, &my_port_id_attr);
	/* error check omitted */
	put_device(target_gendev);

And I do this once, typically for the first sdev of each target?
(Repeating it won't hurt, just gives -EEXIST.)
-- 
Stefan Richter
-=====-=-=== -==- ==-=-
http://arcgraph.de/sr/

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[James Bottomley]

On Tue, 2007-06-26 at 17:47 +0200, Stefan Richter wrote:
> James Bottomley wrote:
> > On Tue, 2007-06-26 at 12:44 +0200, Stefan Richter wrote:
> >> If there are going to be sysfs representations of targets,
> [...]
> > OK, you've lost me ... this is our current sysfs representation of a
> > disk:
> > 
> > /sys/devices/pci0000:00/0000:00:1f.1/host0/target0:0:0/0:0:0:0
> > 
> > target0:0:0 is what I think of as the target ...
> 
> Strange.  How did I miss this?  #-|
> 
> Actually I missed this because I only looked into linux/include/scsi/*.h
> and found
> 
> struct scsi_device {
> 	...
> 	struct scsi_target *sdev_target; /* used only for single_lun */
> 	...
> }

It was procedural ... the only use the mid layer made of this field was
for the single lun processing ... although the code could be rewritten
not actually to do this (we could just use the dev->parent to get the
target).  I suspect a lot of the transport classes now hook into this as
well.

> Is the comment wrong or is sdev_target something different?
> 
> > is that different from what you're asking for?
> 
> That's part of what I asked for.  I also thought of SCSI core populating
> it with certain attributes; let's call them port_name and port_id.  To
> fill values into these attributes, transport layer
> drivers/classes/whatever supply to SCSI core either strings or functions
> which print strings into buffers.  ->  SCSI core controls that these
> properties go into host*/target*:*:*/port_{name,id}.  Transports control
> how the values of these attributes look.

Really, look at the fc code which already does it ... to try to provide
methods from the mid-layer would make this quite complex.

> If that's not wanted, then what is the way?  Simply the following code
> in a transport driver/class/whatever?
> 
> 	target_gendev = get_device(sdev->sdev_gendev.parent);
> 	/* error check omitted */
> 	err = device_create_file(target_gendev, &my_port_name_attr);
> 	/* error check omitted */
> 	err = device_create_file(target_gendev, &my_port_id_attr);
> 	/* error check omitted */
> 	put_device(target_gendev);
> 
> And I do this once, typically for the first sdev of each target?
> (Repeating it won't hurt, just gives -EEXIST

Not really ... look at scsi_transport_fc.c line 911 ... the entire
transport class is designed to set attributes like this and manage the
lifetimes of the underlying objects.

James

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Stefan Richter]

James Bottomley wrote:
> On Tue, 2007-06-26 at 17:47 +0200, Stefan Richter wrote:
>> To fill values into these attributes, transport layer
>> drivers/classes/whatever supply to SCSI core either strings or functions
>> which print strings into buffers.  ->  SCSI core controls that these
>> properties go into host*/target*:*:*/port_{name,id}.  Transports control
>> how the values of these attributes look.
> 
> Really, look at the fc code which already does it ...

Good, I did a kscope session now to get an idea of the "transport
classes" API and how the "XYZ Transport Attributes" drivers use it.

> to try to provide methods from the mid-layer would make this quite complex.

No, it's mostly trivial.  Mid-layer can add its own attributes to
starget->dev as well as to sdev->sdev_gendev.  As far as attribute
values are transport protocol dependent, the show functions of these
attributes can make use of sprintf-like functions to be supplied for
example by scsi_transport_template, as I mentioned earlier in the thread
in http://marc.info/?l=linux-scsi&m=118253907125635 .  I can explain
this by a demo patch, if I find time at the weekend.

[...]
>> 	err = device_create_file(target_gendev, &my_port_id_attr);
[...]
> Not really ... look at scsi_transport_fc.c line 911 ... the entire
> transport class is designed to set attributes like this and manage the
> lifetimes of the underlying objects.

OK, thanks for the pointer into the source.

Of course the mid-layer doesn't have to provide attributes like I
suggested them --- because the alternative is that all or at least the
majority of transport drivers provide them on their own but use
*uniform* attribute names for them.

This is what we have at the moment:

   - The scsi_tranport_fc.c transport driver supplies the Target Port
     Identifier and Name in attributes of the starget->dev.  The
     attributes are called "port_id" and "port_name".

   - The scsi_transport_sas.c transport driver supplies an
     "rphy_sas_address" of which the Target Port Identifier could
     presumably be obtained.  I am not sure in which sysfs devices this
     attribute is located and how these devices relate to starget->dev.

   - The scsi_transport_iscsi.c transport driver supplies the
     attributes "targetname" and "tpgt" of which the Target Port
     Identifier could be obtained.  I am not sure in which sysfs devices
     these attributes are located and how these devices relate to
     starget->dev.

   - I don't know if scsi_transport_spi.c's attributes are suitable to
     obtain object identifiers or object names.

   - Luben's SAS stack obviously supplies the Target Port Identifier in
     the grandparent of LUNS devices.  The attribute is called sas_addr.
     The Logical Unit Identifier is used as the sysfs name of LUNS
     devices, i.e. as sysfs directory names.

   - The sbp2.c transport driver supplies the concatenation of Target
     Port Identifier and Logical Unit Identifier in an attribute of the
     sdev->sdev_gendev.  The attribute is called "ieee1394_id".  sbp2.c
     needs to be told though by a recently added module parameter to
     use SAM-4 conforming formats for the identifiers.  It's partly
     SAM's fault because there is an editorial mistake in SAM regarding
     what an SBP-3 target port identifier is.

   - The fw-sbp2.c transport driver (candidate to eventually replace
     sbp2.c) supplies the concatenation of Target Port Identifier and
     Logical Unit Identifier in an attribute of the sdev->sdev_gendev.
     The attribute is called "ieee1394_id" and conforms to SAM-4 formats
     out of the box.

So, only the SBP-2 transport and the out-of-tree SAS transport expose
Logical Unit Identifiers and Target Port Identifiers.  (sbp2.c has been
doing so for years.)  Some of the other transports expose only Target
Port Identifiers.  Object identifiers are  exposed in non-uniform places
defined by the respective transport implementation.
-- 
Stefan Richter
-=====-=-=== -==- ==-==
http://arcgraph.de/sr/

[PATCH] SCSI: delete outdated comment in API reference

[Stefan Richter]

and move a comment on scsi_target<->scsi_device relationship to a more
appropriate place.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 include/scsi/scsi_device.h |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Index: linux/include/scsi/scsi_device.h
===================================================================
--- linux.orig/include/scsi/scsi_device.h
+++ linux/include/scsi/scsi_device.h
@@ -84,7 +84,7 @@ struct scsi_device {
 	const char * model;		/* ... after scan; point to static string */
 	const char * rev;		/* ... "nullnullnullnull" before scan */
 	unsigned char current_tag;	/* current tag */
-	struct scsi_target      *sdev_target;   /* used only for single_lun */
+	struct scsi_target      *sdev_target;
 
 	unsigned int	sdev_bflags; /* black/white flags as also found in
 				 * scsi_devinfo.[hc]. For now used only to
@@ -168,12 +168,12 @@ enum scsi_target_state {
 };
 
 /*
- * scsi_target: representation of a scsi target, for now, this is only
- * used for single_lun devices. If no one has active IO to the target,
- * starget_sdev_user is NULL, else it points to the active sdev.
+ * scsi_target: representation of a scsi target
  */
 struct scsi_target {
-	struct scsi_device	*starget_sdev_user;
+	struct scsi_device	*starget_sdev_user;  /* For now, this is only
+	 * used for single_lun devices. If no one has active IO to the target,
+	 * starget_sdev_user is NULL, else it points to the active sdev. */
 	struct list_head	siblings;
 	struct list_head	devices;
 	struct device		dev;

-- 
Stefan Richter
-=====-=-=== -==- ==-==
http://arcgraph.de/sr/

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Luben Tuikov]

--- James Bottomley <James.Bottomley@SteelEye.com> wrote:
> You're sort of confusing what we display as the LUN and what we
> represent it as internally (admittedly they're the same at the moment).
> The object would be to separate these and the debate is really about
> what to display.

Display this:
 "%016llx", ((unsigned long long) be64_to_cpu(*(__be64 *)(LUN))),
where LUN is u8[8].  Notice the ugly, C-centric "0x" prefix
is missing.  Not sure who would confuse this number for decimal
or octal base representation of a LUN...

This also makes it great to be a directory name, since it is the
same width and format, and it looks great in "tree" listing format,
as shown in here:
http://marc.info/?l=linux-scsi&m=112629509826900&w=2

    Luben

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Stefan Richter]

Luben Tuikov wrote:
> Display this:
>  "%016llx", ((unsigned long long) be64_to_cpu(*(__be64 *)(LUN))),
> where LUN is u8[8].  Notice the ugly, C-centric "0x" prefix
> is missing.

I agree.  We don't need 0x prefix nor h suffix nor delimiters like
spaces, dashes, colons... in the kernel's printouts of LUNs (i.e. in
sysfs attribute values or sysfs path components or dmesg debug printouts
or wherever the kernel might hand out LUN represemtations to userspace).

...
> This also makes it great to be a directory name, since it is the
> same width and format,
...

Yes, that's quite OK for syfs path components.  But remember that syfs
path components are considered unstable sysfs ABI elements.
-- 
Stefan Richter
-=====-=-=== -==- ==-=-
http://arcgraph.de/sr/

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Swen Schillig]

On Monday 25 June 2007 16:03, Hannes Reinecke wrote:
> Swen Schillig wrote:
> > On Friday 22 June 2007 16:11, James Bottomley wrote:
> >> On Thu, 2007-06-21 at 21:40 -0700, Mike Anderson wrote:
> >>> James Bottomley <James.Bottomley@SteelEye.com> wrote:
> >>>> A proposal to display the correct form of the LUN would be useful if you
> >>>> wish to make it? ...  The problem is really that SAM specifies a
> >>>> possible 4 level structure with 4 possible address methods per level.
> >>>>
> >>>> The well known LUNs should be simple; there are only three: Report Lun,
> >>>> Access Controls and Target Log Pages.  The rest we really need input on.
> >>>> For instance, I could see the vendors wishing us to combine a
> >>>> multi-level flat addressing space into a single logical unit number,
> >>>> whereas I could see them wanting us to supply some sort of hierarchy for
> >>>> the peripheral and logical unit methods of addressing.
> >>>>
> >>>> Since you're already using 2 level flat addressing, how do you want to
> >>>> see that represented?
> >>> James, why would we would not want to display the lun per SAM4 4.6.2 as
> >>> suggested by Stefan in a previous comment.
> >> Because in a two LUN system, what was LUN 1 would then become LUN
> >> 0x1000000000000 which looks a bit unpalatable.
> >>
> >> James
> >>
> >>
> >> -
> > 
> > Despite the issue whether we should display and/or use (sysfs !?)
> > full blown 64bit values, so with leading zeros, you still have the option to display, 
> > for the single level addressing, the LUN as a 2 byte value as described in  SAM4 4.6.2. 
> > So for your example this would be 0x0001 or 0x1, which is exactly what you'd like to see, right ?
> > Only for 2+ level environment the 64bit representation comes into the play.
> > 
> > And, because you were asking how we'd like that to be represented, I personally prefer
> > a full blown 64bit hex value with leading zeros.
> > It makes the comparison to internal structures/models easier and gets us a bit closer
> > to the documented (SAM4) standard.
> > ...and I think we all can deal with a few extra digits being displayed.
> > 
> Well ...
> why don't we stick with the original implementation like zfcp had it?
> We can simpley expand the midlayer to add an attribute 'lun'
> to each scsi_device. This would be the LUN as returned by eg
> REPORT LUNS.
> No translation, nothing. Would be easy to implement and would allow
> any administrator to map the h:c:i:l value to the 'real' lun.
> 
> Cheers,
> 
> Hannes

What do you mean with "like we had it" ?
I thought you were quite happy with the new version making
the old manual counting thing ("zfcp-ism") obsolete.

What's the benefit of adding another (LUN) attribute to the midlayer, I don't get
what this is supposed to solve.
The initial question was, should we not display the long-ish LUN value in 
hex format making it easier for admins (and us) to actually use it
as it is.
I think Luben and Stefan suggested a good way to do that, ok apart from the be64 bits :-)
So, just use the struct scsi_lun in its full extend, for the sysfs without a leading "0x",
and forgetting about the scsilun_to_int conversion which isn't good for anything.

This way we would be conform to SAM4 as well even though I'm getting the impression
that this isn't the major goal of this discussion.

Anyway, before anyone else is suggesting some other magic attribute extension to the LUN value, 
why not leave it as it is and just have it serving this one purpose of being a unique number ?
(yeah, yeah I know there's already some meaning behind the 4 levels)

Just my 2 cents !

Cheers Swen

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Luben Tuikov]

--- Swen Schillig <swen@vnet.ibm.com> wrote:
> I think Luben and Stefan suggested a good way to do that, ok apart from the be64 bits :-)

What is the objection to the use of be64_to_cpu()?

> So, just use the struct scsi_lun in its full extend, for the sysfs without a leading "0x",
> and forgetting about the scsilun_to_int conversion which isn't good for anything.

Yes, I agree with you.  This is simple, straightforward, intuitive and reasonable.

In fact, I'd do
  typedef __u8 lun_t[8];
and then define the macro
  #define LUN_TO_U64(_lun) ((unsigned long long) be64_to_cpu(*(__be64 *)(_lun)))
This way one could do stuff like:*
  printk(KERN_NOTICE "problem xyz with LUN:%016llx\n", LUN_TO_U64(sdev->LUN));
where
  struct scsi_dev {
      ...
      lun_t LUN;  /* Or if you prefer all lowercase. */
      ...
  };
And as can be seen in my SAS stack code:
  kobject_set_name(&lu->lu_obj, "%016llx", LUN_TO_U64(lu->LUN));

(I actually don't have LUN_TO_U64() but reuse the SAS_ADDR() macro
 which is identical.)

* It is in fact more correct to do, at the _transport_ layer:

  printk(... "problem xyz with %016llx:%016llx\n",
         sdev->target->tpid, sdev->LUN);

Here, it is explicitly shown that sdev (/dev/sdXYZ) is a child of
a target, having a tpid attribute, and the sdev has a property
of lun_t called LUN.

So, for example, for SAS, the message would print like this:

problem xyz with 5000a50000f13427:0000000000000000

which uniquely identifies the device and then the sysadmin
can go to the storage farm, find it and replace it if need be.

> Anyway, before anyone else is suggesting some other magic attribute extension to the LUN value, 
> why not leave it as it is and just have it serving this one purpose of being a unique number ?
> (yeah, yeah I know there's already some meaning behind the 4 levels)

Again, you're exactly right.  SCSI Core should NOT be interpreting the LUN.
It should just pass it around as an opaque token, as accomplished by the
lun_t type definition as shown above.

    Luben

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Stefan Richter]

Luben Tuikov wrote:
> In fact, I'd do
>   typedef __u8 lun_t[8];
> and then define the macro
>   #define LUN_TO_U64(_lun) ((unsigned long long) be64_to_cpu(*(__be64 *)(_lun)))

Don't forget __attribute__((aligned(8))) on lun_t then.  Some
architectures need it.

The macro should perhaps be called LUN_TO_ULL, as there is also an u64
type in the kernel.

> This way one could do stuff like:*
>   printk(KERN_NOTICE "problem xyz with LUN:%016llx\n", LUN_TO_U64(sdev->LUN));

How about:

union scsi_lun {
	__u8	lun[8];
	__be64	lun64;
	__be16	lun16;
};

This will also be sufficiently aligned.

[...]
> * It is in fact more correct to do, at the _transport_ layer:
> 
>   printk(... "problem xyz with %016llx:%016llx\n",
>          sdev->target->tpid, sdev->LUN);
> 
> Here, it is explicitly shown that sdev (/dev/sdXYZ) is a child of
> a target, having a tpid attribute, and the sdev has a property
> of lun_t called LUN.
> 
> So, for example, for SAS, the message would print like this:
> 
> problem xyz with 5000a50000f13427:0000000000000000
> 
> which uniquely identifies the device and then the sysadmin
> can go to the storage farm, find it and replace it if need be.

There are two things to consider:
  - Is ->target representing a Target Device?  Then it could have more
    than one port, each one with a different identifier.  Or is it
    representing a Target Port?  Or is it representing a Target Device
    with the twist that we instantiate as many ->target objects as the
    device is showing ports to us?
  - If the identifier is stored in ->target, and is an object known to
    mid-layer, then we need a datatype for ->target->tpid which is
    flexible enough for all flavors of TPID.

So, if tpid ends up in objects seen by mid-layer, the datatype of ->tpid
could be either

    __u8 target_port_identifier[233];  /* enough for all */

or
    __u8 target_port_identifier[0];  /* variable length */

or
    struct scsi_target_port_identifier {
	enum transport_protocol {
		SCSI_TRANSPORT_UNKNOWN = 0,
		SCSI_TRANSPORT_SPI,
		SCSI_TRANSPORT_FCP,
		SCSI_TRANSPORT_SRP,
		SCSI_TRANSPORT_ISCSI,
		SCSI_TRANSPORT_SBP,
		SCSI_TRANSPORT_SAS,
	} format;
	union {
		unsigned spi_tpid:4;

		__u8 fcp_tpid[3];  /* or __be32 fcp_tpid:24; ? */

		struct {
			__be64 eui64;
			__be64 extension;
		} srp_tpid;

		struct {
			__u8 name[224];
			__32 tpgt;
		} iscsi_tpid;

		struct {
			__be64 eui64;
			__be32 directory_id:24;
			/* SAM calls this mistakenly "Discovery ID" */
		} sbp_tpid;

		__be64 sas_tpid;
	} value;
    };

or something else.

The former two require that print functions reside in transport layer
implementations.  Note, the transport layer can easily make these print
functions available to mid-layer so that mid-layer can print TPIDs too,
without knowing what's in a TPID.  I.e. transport layer hands out string
representations of TPIDs to mid-layer.

The third variant allows to put the print function into mid-layer.
Before you call it a heresy:  SAM says how many bits or bytes are in the
identifiers, hence a generic SAM implementation can know how to print
them.  It only doesn't know how the values get in there.

PS:  Sorry that I continue to drag TPID into the discussion about LUN,
but in the end you need both to identify hardware.
-- 
Stefan Richter
-=====-=-=== -==- ===--
http://arcgraph.de/sr/

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Luben Tuikov]

--- Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> How about:
> 
> union scsi_lun {
> 	__u8	lun[8];
> 	__be64	lun64;
> 	__be16	lun16;
> };

I'd rather not even hint that a LUN is to be viewed
as anything integer-like.  Just use u8[8] aligned.

(I.e. it is u64 only at the time when printing it,
but no one really needs to know this. It is u8[8].
Not sure why this is hard to understand.)
 
> There are two things to consider:
>   - Is ->target representing a Target Device?  Then it could have more
>     than one port, each one with a different identifier.  Or is it
>     representing a Target Port?  Or is it representing a Target Device
>     with the twist that we instantiate as many ->target objects as the
>     device is showing ports to us?

Target port.  SCSI Core has no business in multi-pathing.  MP is another
layer on top of SCSI.

>   - If the identifier is stored in ->target, and is an object known to
>     mid-layer, then we need a datatype for ->target->tpid which is
>     flexible enough for all flavors of TPID.

This isn't object oriented.  See below.

> So, if tpid ends up in objects seen by mid-layer, the datatype of ->tpid
> could be either
> 
>     __u8 target_port_identifier[233];  /* enough for all */
> 
> or
>     __u8 target_port_identifier[0];  /* variable length */
> 
> or
>     struct scsi_target_port_identifier {
> 	enum transport_protocol {
> 		SCSI_TRANSPORT_UNKNOWN = 0,
> 		SCSI_TRANSPORT_SPI,
> 		SCSI_TRANSPORT_FCP,
> 		SCSI_TRANSPORT_SRP,
> 		SCSI_TRANSPORT_ISCSI,
> 		SCSI_TRANSPORT_SBP,
> 		SCSI_TRANSPORT_SAS,
> 	} format;
> 	union {
> 		unsigned spi_tpid:4;
> 
> 		__u8 fcp_tpid[3];  /* or __be32 fcp_tpid:24; ? */
> 
> 		struct {
> 			__be64 eui64;
> 			__be64 extension;
> 		} srp_tpid;
> 
> 		struct {
> 			__u8 name[224];
> 			__32 tpgt;
> 		} iscsi_tpid;
> 
> 		struct {
> 			__be64 eui64;
> 			__be32 directory_id:24;
> 			/* SAM calls this mistakenly "Discovery ID" */
> 		} sbp_tpid;
> 
> 		__be64 sas_tpid;

Is it possible to stop with the u64 and its derivatives?
__u8 sas_tpid[8] __aligned(8) would do just fine.

> 	} value;
>     };
> 
> or something else.

Neither. Inevitably a SCSI Core representation of a target
port would contain a transport's layer opaque token (void* for
example). That opaque token uniquely identifes a target's
representation in the transport layer (target port), whose
structure stores the tpid in protocol dependent form.

SCSI Core gives you /dev/sdXYZ, that's all. It needs to get
out of knowing particulars of protocols.  This is the object
oriented approach.

> The former two require that print functions reside in transport layer
> implementations.  Note, the transport layer can easily make these print
> functions available to mid-layer so that mid-layer can print TPIDs too,
> without knowing what's in a TPID.  I.e. transport layer hands out string
> representations of TPIDs to mid-layer.

Something like that.

Imagine you could say in SCSI Core:
transport->printf("I see a problem with %T:%016llx\n", sdev->target-><name of opaque token>,
                   dev->LUN);

Where %T will be "filled" with the tpid after the opaque token has been
resolved by the transport protocol layer.

> The third variant allows to put the print function into mid-layer.

Sorry, no.

> Before you call it a heresy:  SAM says how many bits or bytes are in the

No, it does NOT.  The transport protocol does. SAM merely reproduces
this information, in an _Annex_, for informational purposes to the
reader. I.e. so that you don't have to hunt out the transport protocol
spec and search for it there yourself.

> identifiers, hence a generic SAM implementation can know how to print
> them.  It only doesn't know how the values get in there.

Hmm, this is a weak argument: "... know how to print... doesn't know
how the values get in there."

Ask yourself these questions: "What does SCSI Core provide?", "What should
SCSI Core provide?", "What should SCSI Core not provide?", "Why?".
Give good justifications to your answers.

    Luben

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Stefan Richter]

Luben Tuikov wrote:
> --- Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>> How about:
>>
>> union scsi_lun {
>> 	__u8	lun[8];
>> 	__be64	lun64;
>> 	__be16	lun16;
>> };
> 
> I'd rather not even hint that a LUN is to be viewed
> as anything integer-like.  Just use u8[8] aligned.
> 
> (I.e. it is u64 only at the time when printing it,
> but no one really needs to know this. It is u8[8].
> Not sure why this is hard to understand.)

SAM itself speaks of the LUN as 8 bytes quantity as well as 64 bits
quantity (and 2 bytes quantity as well as 16 bits quantity).  Of course
whether this dualism should be exposed in an API is another question.

[...]
>> 		__be64 sas_tpid;
> 
> Is it possible to stop with the u64 and its derivatives?
> __u8 sas_tpid[8] __aligned(8) would do just fine.

SAS defines the SAS address as fitting into eight bytes.  It furthermore
defines the SAS address as a bitfield consisting of 4, 24, and 36 bits
wide parts.  (And it defines the SAM object Target port identifier to be
implemented as SAS address.)

[u8 tpid[MAX_SIZE] or u8 tpid[] or union tpid]
> Neither. Inevitably a SCSI Core representation of a target
> port would contain a transport's layer opaque token (void* for
> example). That opaque token uniquely identifes a target's
> representation in the transport layer (target port), whose
> structure stores the tpid in protocol dependent form.
> 
> SCSI Core gives you /dev/sdXYZ, that's all. It needs to get
> out of knowing particulars of protocols.  This is the object
> oriented approach.

Target Port Identifier is a property of Target Port.  This has nothing
to do with transport protocols.

Of course the size, source, deeper meaning, and hence potential
human-readable representations of Target Port Identifier are transport
protocol dependent.  I acknowledge that this is a practical problem
which can easily be solved by /not/ storing tpid in midlayer's struct
scsi_target, thus also eliminating the need for a tpid datatype in
midlayer's API.

[...]
>> SAM says how many bits or bytes are in the
[target port identifier]
> 
> No, it does NOT.  The transport protocol does. SAM merely reproduces
> this information,

Well, it reproduces it, hence "says" it; but you are of course right in
that it does not /define/ it.

[...]
> Ask yourself these questions: "What does SCSI Core provide?", "What should
> SCSI Core provide?", "What should SCSI Core not provide?", "Why?".
> Give good justifications to your answers.

Among else:  It should provide a framework which enables userspace to
find the unique object identifiers (that are required to uniquely and
persistently identify logical units) always in the same place.  (That's
just my opinion.  Current userspace tools like e.g. udev scripts are
adapted to hunt those identifiers down in various places, and this works
too.)

I acknowledge that this can be accomplished by only passing string
representations of identifiers through to midlayer.  I also agree that
midlayer should contain as little transport-dependent details as
possible, and that much of what I wrote in my previous post was contrary
to that.  (And now back to driver debugging... :-)
-- 
Stefan Richter
-=====-=-=== -==- ====-
http://arcgraph.de/sr/

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Luben Tuikov]

--- Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> SAM itself speaks of the LUN as 8 bytes quantity as well as 64 bits
> quantity (and 2 bytes quantity as well as 16 bits quantity).  Of course
> whether this dualism should be exposed in an API is another question.

"64 bit quantity", the keyword here is "quantity", _not_ "integer".
Any "u64" variation means "integer" since it dictates a BE or LE
interpretation.  A mere "64 bit quantity" does not expose a BE or LE
bit ordering, and thus does not represent itself as an integer.

It is though one such for human-readability.  Ideally, you'd not want
to expose this in the representation.

> SAS defines the SAS address as fitting into eight bytes.  It furthermore
> defines the SAS address as a bitfield consisting of 4, 24, and 36 bits
> wide parts.  (And it defines the SAM object Target port identifier to be
> implemented as SAS address.)
> 
> [u8 tpid[MAX_SIZE] or u8 tpid[] or union tpid]

Notice that you've already moved down to a protocol layer, in this
case SAS and have decomposed the tpid as defined in its protocol.
Thus, you've moved too "deep" into the internal choice of what
a SAS tpid is which is only interesting for people generating
such entities, deep into the details of the _protocol_.

I doubt that your intention is to expose the _composition_ itself
of the port identifier for _each_ protocol to SCSI Core.

> Target Port Identifier is a property of Target Port.  This has nothing
> to do with transport protocols.

Yes, in OOP(aradigm) this is a "virtual property".

> Of course the size, source, deeper meaning, and hence potential
> human-readable representations of Target Port Identifier are transport
> protocol dependent.

Classic OOP design.

> I acknowledge that this is a practical problem
> which can easily be solved by /not/ storing tpid in midlayer's struct
> scsi_target, thus also eliminating the need for a tpid datatype in
> midlayer's API.

> Well, it reproduces it, hence "says" it; but you are of course right in
> that it does not /define/ it.
> 
> [...]
> > Ask yourself these questions: "What does SCSI Core provide?", "What should
> > SCSI Core provide?", "What should SCSI Core not provide?", "Why?".
> > Give good justifications to your answers.
> 
> Among else:  It should provide a framework which enables userspace to
> find the unique object identifiers (that are required to uniquely and
> persistently identify logical units) always in the same place.  (That's
> just my opinion.  Current userspace tools like e.g. udev scripts are
> adapted to hunt those identifiers down in various places, and this works
> too.)

This information can be had by using default SPC commands, issued by
specialized userspace tools.  For example the SG tools maintained by Doug.

I'd rather have SCSI Core provide just _service_, i.e. means of getting
at those properties, but _not_ providing the properties themselves, i.e.
by trying to find them or figure them out on it's own. (I.e. by itself
issuing commands and what not.)

Of course, some of those identifiers and what not can be easily had
and seen simply by looking at the sysfs tree exported by the
respective protocol, just like my SAS Stack does. An example
of a 1-1 sysfs representation of a SAS domain can be seen here:
http://marc.info/?l=linux-scsi&m=112629509826900&w=2

But this is the exception.  I'd opt for using the SG tools exclusively.

> I acknowledge that this can be accomplished by only passing string
> representations of identifiers through to midlayer.

Tpid and such should only exist in the respective protocol layers.
It should NOT be exposed to SCSI Core.  The only thing that should be
is an opaque token by which the protocol layer identifies which target
port SCSI Core wants the command sent to, the LUN should be
identified in the SCSI Command struct!

There is a nice summary of this "opaque token" idea in the commentary to
sas_slave_alloc()::sas_scsi_host.c, and in
sas_do_lu_discovery()::sas_discover.c in my SAS stack (out of tree).

Also if you take a look at struct sas_task you'd see where the LUN
is.

> I also agree that
> midlayer should contain as little transport-dependent details as
> possible, and that much of what I wrote in my previous post was contrary
> to that.  (And now back to driver debugging... :-)

I think that from the beginning you had some great ideas which are
still current.  It also seems (and has always been the case) that
you have good understanding of the SCSI Architecture Model which is
a good thing.

   Luben

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Stefan Richter]

Luben Tuikov wrote:
> --- Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>> Among else:  It should provide a framework which enables userspace to
>> find the unique object identifiers (that are required to uniquely and
>> persistently identify logical units) always in the same place.  (That's
>> just my opinion.  Current userspace tools like e.g. udev scripts are
>> adapted to hunt those identifiers down in various places, and this works
>> too.)
> 
> This information can be had by using default SPC commands, issued by
> specialized userspace tools.  For example the SG tools maintained by Doug.

I did a quick test with twelve SBP-2 devices.  Only four of them
implement the mandatory VPD pages.  But they show only vendor specific
identifiers which are incidentally world wide unique, but are not
guaranteed to be.  The results in detail:

1.) an MMC device: does not support INQUIRY with EVPD=1  (sg_inq says:
invalid VPD response; probably a STANDARD INQUIRY response)

2.) an SBC device which I suspect is rather an RBC device: does not
support EVPD=1 either (returns: Invalid Request)

3.) another SBC device from a different vendor and with a different
controller, which I too suspect is really an RBC device:  Returns
corrupt INQUIRY data which look as if (only) VPD page 0x1f was
implemented; but it isn't. (if any non-zero VPD page code is specified,
sg_inq says: invalid VPD response; probably a STANDARD INQUIRY response)

4.) an RBC device from the same vendor as device 2 and with a newer
revision of controller and firmware of that of device 2:  behaves like
device 3, even though hardware and firmware are very different from device 3

5.-7.) three RBC devices from the same vendor but with different
controllers  and quite different years of production:   They all
implement VPD pages 0x80 = Unit Serial Number and 0x83 = Device
Identification, which fulfills the minimal requirement according to RBC.
 Alas the Device Identification page merely contains a Vendor Specific
Identifier which SPC says is not guaranteed to be globally unique.  In
these two cases it happens to be the same as the upper 64 bits of the
SBP-3 Target port identifier, the EUI-64.

8.) an MMC device from the same vendor as devices 5-7 bit with yet
another different controller:  does not support EVPD=1  (returns:
Invalid Request)

9.) yet one more RBC device, a Mac in target disk mode:  does not
support INQUIRY with EVPD=1  (returns: Invalid Request)

10.) an RBC device with the same bridge as one of the devices 5-7 but
from a different vendor:  Behaves like devices 5-7

11.) an MMC device with possibly same controller as device 1 but from a
different vendor:  behaves like device 1

12.) a dual LU device with a controller like device 3.  I plugged a HDD
and a DVD-ROM into it so that LU 00 00 is a SBC device (perhaps more
like an RBC device) and LU 00 01 is an MMC device.  LU 00 00 behaves
like device 3.  LU 00 01 behaves like device 1.

For the heck of it I also tried REPORT LUNS on device 12:
# sg_luns /dev/sdd
Report Luns command not supported (support mandatory in SPC-3)
# sg_luns /dev/sr0
REPORT LUNS command error: SCSI status: Check Condition
 Fixed format, current;  Sense key: Not Ready
 Additional sense: Medium not present

 Raw sense data (in hex):
        70 00 02 00 00 00 00 0a  00 00 00 00 3a 00 00 00
        00 00
plus...: Driver_status=0x08 [DRIVER_SENSE, SUGGEST_OK]

-- 
Stefan Richter
-=====-=-=== -=== ----=
http://arcgraph.de/sr/

Re: [PATCH] zfcp: Report FCP LUN to SCSI midlayer

[Luben Tuikov]

--- James Bottomley <James.Bottomley@SteelEye.com> wrote:
> On Thu, 2007-06-21 at 21:40 -0700, Mike Anderson wrote:
> > James Bottomley <James.Bottomley@SteelEye.com> wrote:
> > > A proposal to display the correct form of the LUN would be useful if you
> > > wish to make it? ...  The problem is really that SAM specifies a
> > > possible 4 level structure with 4 possible address methods per level.
> > > 
> > > The well known LUNs should be simple; there are only three: Report Lun,
> > > Access Controls and Target Log Pages.  The rest we really need input on.
> > > For instance, I could see the vendors wishing us to combine a
> > > multi-level flat addressing space into a single logical unit number,
> > > whereas I could see them wanting us to supply some sort of hierarchy for
> > > the peripheral and logical unit methods of addressing.
> > > 
> > > Since you're already using 2 level flat addressing, how do you want to
> > > see that represented?

Goodness gracious!

> > 
> > James, why would we would not want to display the lun per SAM4 4.6.2 as
> > suggested by Stefan in a previous comment.
> 
> Because in a two LUN system, what was LUN 1 would then become LUN
> 0x1000000000000 which looks a bit unpalatable.

To whom?  Who (as in entity) is the audience of the (structure of a) LUN?

    Luben