Patch to multipath-tools 0.2.4

Hi Christophe,

I've made a patch to multipath-tools 0.2.4 to correctly decipher SCSI EVPD page 0x83. The current code is totally ignorant of the fact that EVPD page 0x83 can return more than one identifier, some of which are either bogus (i.e. containing an empty string) or should be preferred to the other identifiers.

The attached patch
- correctly deciphers EVPD page 0x83
- Tries to select the 'best' candidate if several IDs are returned:
- ID should not be empty (i.e. either only spaces or Nulls)
- Order of preference:
- NAA (hightest subtype is preferrred)
- EUI-64
- T-10
- Vendor specific
- Map spaces to underscores if the serial number is ASCII; this is mainly to generate valid mapping names.

Would be great if this patch could be included, as without this the multipath tools do not work correctly with the IBM ESS.

Most of the decoding logic is in fact stolen from scsi_id (thanks Patrick), so we should probably add a copyright statement somewhere.

We should actually be using the scsi_id package directly as this would give us identical mappings for udev and multipath tools. But as this would mean a major restructuring of the program itself I took the cheap way out. Patrick, can you think of a clever way of integrating both?

Cheers,

Hannes

Dr. Hannes Reinecke			[email protected]
SuSE Linux AG				S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de

diff -u --recursive multipath-tools-0.2.4/multipath/devinfo.c multipath-tools-0.2.4.patched/multipath/devinfo.c
@@ -33,7 +33,8 @@
         if (evpd)
                 inqCmdBlk[1] |= 1;
         inqCmdBlk[2] = (unsigned char) pg_op;
-        inqCmdBlk[4] = (unsigned char) mx_resp_len;
+        inqCmdBlk[3] = (unsigned char)((mx_resp_len >> 8) & 0xff);
+        inqCmdBlk[4] = (unsigned char) (mx_resp_len & 0xff);
         memset(&io_hdr, 0, sizeof (struct sg_io_hdr));
         io_hdr.interface_id = 'S';
         io_hdr.cmd_len = sizeof (inqCmdBlk);

@@ -139,28 +140,231 @@
 	return 0;
 }
 
-static void
-sprint_wwid(char * buff, const char * str)
+/*
+ * Supported Code Set values.
+ */
+#define	CODESET_BINARY	1
+#define	CODESET_ASCII	2
+
+static const char hex_str[]="0123456789abcdef";
+
+/**
+ * check_fill_0x83_id - check the page 0x83 id, if OK allocate and fill
+ * serial number.
+ **/
+static int check_0x83_id(char *scsi_dev, char *page_83, int max_len)
+{
+	int i, j, len, idweight;
+
+	/*
+	 * ASSOCIATION must be with the device (value 0)
+	 */
+#ifdef DEBUG
+	fprintf(stdout,"%s: Association: %X\n", scsi_dev, page_83[1] & 0x30);
+#endif
+	if ((page_83[1] & 0x30) != 0)
+		return -1;
+
+	/*
+	 * Check for code set - ASCII or BINARY.
+	 */
+#ifdef DEBUG
+	fprintf(stdout,"%s: Codeset: %X\n", scsi_dev, page_83[0] & 0x0f);
+#endif
+	if (((page_83[0] & 0x0f) != CODESET_ASCII) &&
+	    ((page_83[0] & 0x0f) != CODESET_BINARY))
+		return -1;
+
+	/*
+	 * Check for ID type
+	 */
+	idweight = page_83[1] & 0x0f;
+#ifdef DEBUG
+	fprintf(stdout,"%s: ID type: ", scsi_dev);
+	switch (page_83[1] & 0x0f) {
+	case 0x0:
+		printf("Vendor specific\n");
+		break;
+	case 0x1:
+		printf("T-10 vendor identification\n");
+		break;
+	case 0x2:
+		printf("EUI-64\n");
+		break;
+	case 0x3:
+		printf("NAA ");
+		switch((page_83[4] >> 4) & 0x0f) {
+		case 0x02:
+			printf("IEEE Extended\n");
+			break;
+		case 0x05:
+			printf("IEEE Registered\n");
+			break;
+		case 0x06:
+			printf("IEEE Registered Extended\n");
+			break;
+		default:
+			printf("Reserved (%X)\n", (page_83[4] >> 4) & 0x0f);
+			break;
+		}
+		break;
+	case 0x4:
+		printf("Relative target port\n");
+		break;
+	case 0x5:
+		printf("Target port group\n");
+		break;
+	case 0x6:
+		printf("Logical unit group\n");
+		break;
+	case 0x7:
+		printf("MD5 logical unit identifier\n");
+		break;
+	default:
+		printf("Reserved\n");
+		break;
+	}
+#endif
+	/*
+	 * Only allow vendor specific, T-10 Vendor Identifcation,
+	 * EUI-64 or NAA identificators
+	 */
+	if ((page_83[1] & 0x0f) > 0x3)
+		return -1;
+
+	/*
+	 * Prefer NAA Registered Extended to NAA Registered and
+	 * NAA Extended.
+	 * Not checking for actual NAA types as some devices
+	 * ( e.g. Compaq iSCSI disks via CISCO routers) return
+	 * add NAA subtypes (0x3).
+	 */
+	if ((page_83[1] & 0x0f) == 0x03) {
+		idweight += (page_83[4] >> 4) & 0x0f;
+	}
+	 
+	/*
+	 * page_83[3]: identifier length
+	 */
+	len = page_83[3];
+	if ((page_83[0] & 0x0f) != CODESET_ASCII)
+		/*
+		 * If not ASCII, use two bytes for each binary value.
+		 */
+		len *= 2;
+
+
+       	/*
+	 * Add one byte for the NUL termination.
+	 */
+	if (max_len < len + 1) {
+		fprintf(stderr, "%s: length %d too short - need %d\n",
+			scsi_dev, max_len, len);
+		return 1;
+	}
+
+	i = 4; /* offset to the start of the identifier */
+	j = 0;
+	if ((page_83[0] & 0x0f) == CODESET_ASCII) {
+		/*
+		 * ASCII descriptor.
+		 *
+		 * Check whether the descriptor contains anything useful
+		 * i.e. anything apart from ' ' or '0'.
+		 */
+#ifdef DEBUG
+		fprintf(stdout,"%s: string '", scsi_dev);
+#endif
+		while (i < (4 + page_83[3])) {
+			if (page_83[i] == ' ' || page_83[i] == '0')
+				len--;
+#ifdef DEBUG		
+			fputc(page_83[i],stdout);
+#endif
+			i++;
+		}
+#ifdef DEBUG
+		fprintf(stdout,"'");
+#endif
+	} else {
+		/*
+		 * Binary descriptor, convert to ASCII, using two bytes of
+		 * ASCII for each byte in the page_83.
+		 *
+		 * Again, check whether the descriptor contains anything
+		 * useful; in this case, anything > 0.
+		 */
+#ifdef DEBUG
+		fprintf(stdout,"%s: binary ", scsi_dev);
+#endif
+		while (i < (4 + page_83[3])) {
+			if (page_83[i] == 0)
+				len-=2;
+#ifdef DEBUG
+			fputc(hex_str[(page_83[i] & 0xf0) >> 4],stdout);
+			fputc(hex_str[page_83[i] & 0x0f],stdout);
+#endif
+			i++;
+		}
+	}
+#ifdef DEBUG
+	fprintf(stdout," (len %d)\n", len);
+#endif
+
+	if (len <= 0)
+		return -1;
+
+	return idweight;
+}
+
+/**
+ * fill_0x83_id - fill serial number.
+ **/
+static int fill_0x83_id(char *page_83, char *serial)
 {
-	int i;
-	const char *p;
-	char *cursor;
-	unsigned char c;
-	char empty_buff[WWID_SIZE];
-
-	memset(empty_buff, 0, WWID_SIZE);
-
-	if (0 == memcmp(empty_buff, str, WWID_SIZE))
-		return;
-
-	p = str;
-	cursor = buff;
-	for (i = 0; i <= WWID_SIZE / 2 - 1; i++) {
-		c = *p++;
-		sprintf(cursor, "%.2x", (int) (unsigned char) c);
-		cursor += 2;
+	int i, j, len;
+
+	/*
+	 * page_83[3]: identifier length
+	 */
+	len = page_83[3];
+	if ((page_83[0] & 0x0f) != CODESET_ASCII)
+		/*
+		 * If not ASCII, use two bytes for each binary value.
+		 */
+		len *= 2;
+
+       	/*
+	 * Add one byte for the NUL termination.
+	 */
+	len += 1;
+
+	i = 4; /* offset to the start of the identifier */
+	j = 0;
+	if ((page_83[0] & 0x0f) == CODESET_ASCII) {
+		/*
+		 * ASCII descriptor.
+		 */
+		while (i < (4 + page_83[3])) {
+			/* Map ' ' -> '_' */
+			if (page_83[i] == ' ')
+				serial[j] = '_';
+			else
+				serial[j] = page_83[i];
+			j++; i++;
+		}
+	} else {
+		/*
+		 * Binary descriptor, convert to ASCII, using two bytes of
+		 * ASCII for each byte in the page_83.
+		 */
+		while (i < (4 + page_83[3])) {
+			serial[j++] = hex_str[(page_83[i] & 0xf0) >> 4];
+			serial[j++] = hex_str[page_83[i] & 0x0f];
+			i++;
+		}
 	}
-	buff[WWID_SIZE - 1] = '\0';
+	return 0;
 }
 
 long

@@ -246,18 +450,39 @@
 int
 get_evpd_wwid (char * devname, char * wwid)
 {
-        int fd;
+        int fd, j, weight, weight_cur, offset_cur, retval = 0;
         char buff[MX_ALLOC_LEN + 1];
 
         if ((fd = open(devname, O_RDONLY)) < 0)
                         return 1;
 
+	weight_cur = -1;
+	offset_cur = -1;
         if (0 == do_inq(fd, 0, 1, 0x83, buff, MX_ALLOC_LEN, 1)) {
-                sprint_wwid(wwid, &buff[8]);
-                close(fd);
-                return 0;
+		/*
+		 * Examine each descriptor returned. There is normally only
+		 * one or a small number of descriptors.
+		 */
+		for (j = 4; j <= buff[3] + 3; j += buff[j + 3] + 4) {
+#ifdef DEBUG
+			fprintf(stdout,"%s: ID descriptor at %d:\n", 
+				devname, j);
+#endif
+			weight = check_0x83_id(devname, &buff[j], 
+					       MX_ALLOC_LEN);
+			if (weight >= 0 && weight > weight_cur) {
+				weight_cur = weight;
+				offset_cur = j;
+			}
+		}
+		if (weight_cur >= 0)
+			fill_0x83_id(&buff[offset_cur], wwid);
+		else
+			retval = 1;
+	} else {
+		retval = 1;
         }
 
         close(fd);
-        return 1;
+        return retval;
 }

Thanks,

merged in multipath-tools-0.2.5
it works here on StorageWorks :

xa-s03:~/multipath/multipath-tools-0.2.5/multipath# ./multipath -v
/dev/sda: ID descriptor at 4:
/dev/sda: Association: 0
/dev/sda: Codeset: 1
/dev/sda: ID type: NAA IEEE Registered Extended
/dev/sda: binary 600508b4000156d700012000000b0000 (len 20)
/dev/sdb: ID descriptor at 4:
/dev/sdb: Association: 0
/dev/sdb: Codeset: 1
/dev/sdb: ID type: NAA IEEE Registered Extended
/dev/sdb: binary 600508b4000156d700012000000b0000 (len 20)
# all paths :
600508b4000156d700012000000b0000 (3 0 0 1) /dev/sda [HSV110 (C)COMPAQ]
600508b4000156d700012000000b0000 (3 0 1 1) /dev/sdb [HSV110 (C)COMPAQ]
# all multipaths :
600508b4000156d700012000000b0000
 \_(3 0 0 1) /dev/sda [HSV110 (C)COMPAQ]
 \_(3 0 1 1) /dev/sdb [HSV110 (C)COMPAQ]
# device maps :
U:600508b4000156d700012000000b0000:0 62914560 multipath 2 round-robin 1 0
/dev/sda round-robin 1 0 /dev/sdb

If you want the IBM ESS strings in the default config file and core defaults, please send the corresponding patch.

regards,
cvaroqui

Selon Hannes Reinecke <[email protected]>:

-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training.
Attend Black Hat Briefings & Training, Las Vegas July 24-29 - 
digital self defense, top technical experts, no vendor pitches, 
unmatched networking opportunities. Visit www.blackhat.com
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Christophe -

How about a callout line, similiar to that in udev, so any program can be specified to get the id of the device?

Does this still need to work with 2.4? If so, the current get_evpd_wwid can be left in place.

The corresponding config file syntax could be:

getuid_function exec /sbin/scsi_id

-- Patrick Mansfield

-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training. Attend Black Hat Briefings & Training, Las Vegas July 24-29 - digital self defense, top technical experts, no vendor pitches, unmatched networking opportunities. Visit www.blackhat.com
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

That, or a libscsiid ...
Do you plan to produce such a lib ?

regards,
cvaroqui

-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training. Attend Black Hat Briefings & Training, Las Vegas July 24-29 - digital self defense, top technical experts, no vendor pitches, unmatched networking opportunities. Visit www.blackhat.com
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

No plans, but have thought about it. A callout is simpler and more flexible, maybe slower but that should not be a issue.

-- Patrick Mansfield

-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training. Attend Black Hat Briefings & Training, Las Vegas July 24-29 - digital self defense, top technical experts, no vendor pitches, unmatched networking opportunities. Visit www.blackhat.com
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

ok, I got the callout working in my current tree.

before I release, I would like to report that scsi_id presents a strange leading digit in its output.
In my case a wwid like 6005xxxxxxxxxxxx (32 char) becomes 36005xxxxxxxxxxxx (33 char).

Note it's a bit annoying because the fallback get_evpd_wwid (in case scsi_id is absent) rightfully reports 64 bit uuid, which is not consistant with scsi_id reporting.

Can you explain this and give guidance ?

regards,
cvaroqui

-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training. Attend Black Hat Briefings & Training, Las Vegas July 24-29 - digital self defense, top technical experts, no vendor pitches, unmatched networking opportunities. Visit www.blackhat.com
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Thanks for doing that work.

The first digit is the value of the "identifier type", looking at the SCSI primary commands draft spc3r19, the values in table 284, section 7.6.4.1, page numbered 312.

The different types of identifiers could in theory - though perhaps unlikely or maybe even impossible at this time - have overlapping name spaces.

For example, the NAA IEEE extended has an 8 byte value (including the NAA type), as does the EUI-64 8 byte value (there are non 64 bit EUI-64 values!?).

The page 0x83 "identifier type" is 8 bits, and so can never overlap with the scsi_id page 0x80 prefix of 0x53 ('S'), so the scsi_id page 0x80 values can never overlap with page 0x83 values.

-- Patrick Mansfield

-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training. Attend Black Hat Briefings & Training, Las Vegas July 24-29 - digital self defense, top technical experts, no vendor pitches, unmatched networking opportunities. Visit www.blackhat.com
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

It sounds fair for scsi_id to try as hard as possible to generate a *really unique* uid, thus the prefix.

In the context of multipath, do you suggest I introduce this prefix in the fallback internal method or should I trim the scsi_id output ?

Subsidiary question : should I merge scsi_id in the multipath-tools tree for klibc build ?

regards,
cvaroqui

Selon Patrick Mansfield <[email protected]>:

-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training.
Attend Black Hat Briefings & Training, Las Vegas July 24-29 - 
digital self defense, top technical experts, no vendor pitches, 
unmatched networking opportunities. Visit www.blackhat.com
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

multipath should also use the prefix.

I don't think so, as it makes it harder to keep trees in sync.

It would be nice to have a klibc build environment (maybe klibc is already setup that way, I haven't looked closely), instead of carrying klibc in multiple places. So you could build multiple and perhaps separate tools, and then install them under their own root (i.e. initramfs) directory.

-- Patrick Mansfield

-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training. Attend Black Hat Briefings & Training, Las Vegas July 24-29 - digital self defense, top technical experts, no vendor pitches, unmatched networking opportunities. Visit www.blackhat.com
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Hannes, care to feed me a patch for this ?

regards,
cvaroqui

-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training. Attend Black Hat Briefings & Training, Las Vegas July 24-29 - digital self defense, top technical experts, no vendor pitches, unmatched networking opportunities. Visit www.blackhat.com
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel