[openib-general] Re: [PATCH] SRP : Use correct port identifier format according to target io_class

[Ramchandra K]

On Mon, 2006-05-29 at 10:07 -0700, Roland Dreier wrote:
> Overall seems OK.  Some comments:

I am resending the patch with the modifications you suggested.

>  > +#define SRP_REV10_IO_CLASS   0xFF00
>  > +#define SRP_REV16A_IO_CLASS  0x0100
> 
> I think these should be in an enum in <scsi/srp.h>, since they're
> generic constants from the SRP spec.
> 
I have defined the IO class values as an enum in <scsi/srp.h>. I am
sending this as a separate patch. I am not sure if those changes
are to be submitted here, since srp.h is not in the Open Fabrics
code base. But both the patches have to be applied together for
the SRP code to compile.


Signed-off-by: Ramachandra K <rkuchimanchi at silverstorm.com>

Index: infiniband/ulp/srp/ib_srp.c
===================================================================
--- infiniband/ulp/srp/ib_srp.c	(revision 7615)
+++ infiniband/ulp/srp/ib_srp.c	(working copy)
@@ -321,8 +321,33 @@
 	req->priv.req_it_iu_len = cpu_to_be32(srp_max_iu_len);
 	req->priv.req_buf_fmt 	= cpu_to_be16(SRP_BUF_FORMAT_DIRECT |
 					      SRP_BUF_FORMAT_INDIRECT);
-	memcpy(req->priv.initiator_port_id, target->srp_host->initiator_port_id, 16);
 	/*
+	 * Older targets conforming to Rev 10 of the SRP specification
+	 * use the port identifier format which is
+	 *
+	 * lower 8 bytes :  GUID
+	 * upper 8 bytes :  extension
+	 *
+	 * Where as according to the new SRP specification (Rev 16a), the 
+	 * port identifier format is
+	 *
+	 * lower 8 bytes :  extension
+	 * upper 8 bytes :  GUID
+	 *
+	 * So check the IO class of the target to decide which format to use.
+	 */
+
+	/* If its Rev 10, flip the initiator port id fields */
+	if (target->io_class == SRP_REV10_IO_CLASS) {
+		memcpy(req->priv.initiator_port_id,
+			target->srp_host->initiator_port_id + 8 , 8);
+		memcpy(req->priv.initiator_port_id + 8,
+			target->srp_host->initiator_port_id, 8);
+	} else {	
+		memcpy(req->priv.initiator_port_id,
+			 target->srp_host->initiator_port_id, 16);
+	}
+	/*
 	 * Topspin/Cisco SRP targets will reject our login unless we
 	 * zero out the first 8 bytes of our initiator port ID.  The
 	 * second 8 bytes must be our local node GUID, but we always
@@ -334,8 +359,13 @@
 		       (unsigned long long) be64_to_cpu(target->ioc_guid));
 		memset(req->priv.initiator_port_id, 0, 8);
 	}
-	memcpy(req->priv.target_port_id,     &target->id_ext, 8);
-	memcpy(req->priv.target_port_id + 8, &target->ioc_guid, 8);
+	if (target->io_class == SRP_REV10_IO_CLASS) {
+		memcpy(req->priv.target_port_id,     &target->ioc_guid, 8);
+		memcpy(req->priv.target_port_id + 8, &target->id_ext, 8);
+	} else {
+		memcpy(req->priv.target_port_id,     &target->id_ext, 8);
+		memcpy(req->priv.target_port_id + 8, &target->ioc_guid, 8);
+	}
 
 	status = ib_send_cm_req(target->cm_id, &req->param);
 
@@ -1513,6 +1543,7 @@
 	SRP_OPT_SERVICE_ID	= 1 << 4,
 	SRP_OPT_MAX_SECT	= 1 << 5,
 	SRP_OPT_MAX_CMD_PER_LUN	= 1 << 6,
+	SRP_OPT_IO_CLASS	= 1 << 7,
 	SRP_OPT_ALL		= (SRP_OPT_ID_EXT	|
 				   SRP_OPT_IOC_GUID	|
 				   SRP_OPT_DGID		|
@@ -1528,6 +1559,7 @@
 	{ SRP_OPT_SERVICE_ID,		"service_id=%s"		},
 	{ SRP_OPT_MAX_SECT,		"max_sect=%d" 		},
 	{ SRP_OPT_MAX_CMD_PER_LUN,	"max_cmd_per_lun=%d" 	},
+	{ SRP_OPT_IO_CLASS,		"io_class=%x"		},
 	{ SRP_OPT_ERR,			NULL 			}
 };
 
@@ -1611,7 +1643,19 @@
 			}
 			target->scsi_host->cmd_per_lun = min(token, SRP_SQ_SIZE);
 			break;
-
+		case SRP_OPT_IO_CLASS:
+			if (match_hex(args, &token)) {
+				printk(KERN_WARNING PFX "bad  IO class parameter '%s' \n", p);
+				goto out;
+			}
+			if (token == SRP_REV10_IO_CLASS || token == SRP_REV16A_IO_CLASS)
+				target->io_class = token;
+			else
+				printk(KERN_WARNING PFX "unknown IO class parameter value"
+				   " %x specified. Use %x or %x. Defaulting to IO class %x\n",
+				   token, SRP_REV10_IO_CLASS, SRP_REV16A_IO_CLASS,
+				   SRP_REV16A_IO_CLASS);
+			break;
 		default:
 			printk(KERN_WARNING PFX "unknown parameter or missing value "
 			       "'%s' in target creation request\n", p);
@@ -1654,6 +1698,7 @@
 	target = host_to_target(target_host);
 	memset(target, 0, sizeof *target);
 
+	target->io_class   = SRP_REV16A_IO_CLASS;
 	target->scsi_host  = target_host;
 	target->srp_host   = host;
 
Index: infiniband/ulp/srp/ib_srp.h
===================================================================
--- infiniband/ulp/srp/ib_srp.h	(revision 7615)
+++ infiniband/ulp/srp/ib_srp.h	(working copy)
@@ -122,6 +122,7 @@
 	__be64			id_ext;
 	__be64			ioc_guid;
 	__be64			service_id;
+	__be16			io_class;
 	struct srp_host	       *srp_host;
 	struct Scsi_Host       *scsi_host;
 	char			target_name[32];