From mboxrd@z Thu Jan 1 00:00:00 1970 From: Douglas Gilbert Subject: Re: [PATCH 4/5] target: Skip non hex characters for EVPD=0x83 NAA IEEE Registered Extended Date: Fri, 16 Sep 2011 10:19:49 -0400 Message-ID: <4E735B05.4000307@interlog.com> References: <1316169506-4441-1-git-send-email-nab@linux-iscsi.org> <1316169506-4441-5-git-send-email-nab@linux-iscsi.org> <4E73562D.9060205@zoner.cz> Reply-To: dgilbert@interlog.com Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-2; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from smtp.infotech.no ([82.134.31.41]:38082 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751077Ab1IPOUB (ORCPT ); Fri, 16 Sep 2011 10:20:01 -0400 In-Reply-To: <4E73562D.9060205@zoner.cz> Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: Martin Svec Cc: "Nicholas A. Bellinger" , target-devel , linux-scsi , Linus Torvalds , James Bottomley A documentation note: it should be "VPD=0x83" not "EVPD=0x83" in the subject line and elsewhere. The "E" stands for "enable" and EVPD refers to a bit field in a INQUIRY cdb. When it is set the response should be a VPD page corresponding to the "page code" field in the cdb. I made the same mistake in my tools but I hope that has all been cleaned up now. Doug Gilbert On 11-09-16 09:59 AM, Martin Svec wrote: > Tested, looks fine to me. (For next == 0, maybe use assignment instead of > bitwise OR in "buf[cnt] |= hex_to_bin(*p++) << 4;"?) > > Martin > > Dne 16.9.2011 12:38, Nicholas A. Bellinger napsal(a): >> From: Nicholas Bellinger >> >> This patch adds target_parse_naa_6h_vendor_specific() to address a bug where the >> conversion of PRODUCT SERIAL NUMBER to use hex2bin() in target_emulate_evpd_83() >> was not doing proper isxdigit() checking. This conversion of the vpd_unit_serial >> configifs attribute is done while generating a EVPD=0x83 NAA IEEE Registered >> Extended DESIGNATOR format's 100 bits of unique VENDOR SPECIFIC IDENTIFIER + >> VENDOR SPECIFIC IDENTIFIER EXTENSION area. >> >> This patch allows vpd_unit_serial (EVPD=0x80) and the T10 Vendor ID DESIGNATOR >> format (EVPD=0x83) to continue to use free-form variable length ASCII values, >> and now skips any non hex characters for fixed length NAA IEEE Registered >> Extended >> DESIGNATOR format (EVPD=0x83) requring the binary conversion. >> >> This was originally reported by Martin after the v3.1-rc1 change to use hex2bin() >> in commit 11650b859681e03fdbf26277fcfc5f1f62186703 where the use of non hex >> characters in vpd_unit_serial generated different values than the original >> v3.0 internal hex -> binary code. This v3.1 change caused a problem with >> filesystems who write a NAA DESIGNATOR onto it's ondisk metadata, and this patch >> will (again) change existing values to ensure that non hex characters are not >> included in the fixed length NAA DESIGNATOR. >> >> Note this patch still expects vpd_unit_serial to be set via existing userspace >> methods of uuid generation, and does not do strict formatting via configfs input. >> >> The original bug report and thread can be found here: >> >> NAA breakage >> http://www.spinics.net/lists/target-devel/msg00477.html >> >> The v3.1-rc1 formatting of EVPD=0x83 w/o this patch: >> >> VPD INQUIRY: Device Identification page >> Designation descriptor number 1, descriptor length: 20 >> designator_type: NAA, code_set: Binary >> associated with the addressed logical unit >> NAA 6, IEEE Company_id: 0x1405 >> Vendor Specific Identifier: 0xffde35ebf >> Vendor Specific Identifier Extension: 0x3092f498ffa820f9 >> [0x6001405ffde35ebf3092f498ffa820f9] >> Designation descriptor number 2, descriptor length: 56 >> designator_type: T10 vendor identification, code_set: ASCII >> associated with the addressed logical unit >> vendor id: LIO-ORG >> vendor specific: IBLOCK:ffde35ec-3092-4980-a820-917636ca54f1 >> >> The v3.1-final formatting of EVPD=0x83 w/ this patch: >> >> VPD INQUIRY: Device Identification page >> Designation descriptor number 1, descriptor length: 20 >> designator_type: NAA, code_set: Binary >> associated with the addressed logical unit >> NAA 6, IEEE Company_id: 0x1405 >> Vendor Specific Identifier: 0xffde35ec3 >> Vendor Specific Identifier Extension: 0x924980a82091763 >> [0x6001405ffde35ec30924980a82091763] >> Designation descriptor number 2, descriptor length: 56 >> designator_type: T10 vendor identification, code_set: ASCII >> associated with the addressed logical unit >> vendor id: LIO-ORG >> vendor specific: IBLOCK:ffde35ec-3092-4980-a820-917636ca54f1 >> >> Reported-by: Martin Svec >> Cc: Martin Svec >> Cc: Douglas Gilbert >> Cc: James Bottomley >> Signed-off-by: Nicholas Bellinger >> --- >> drivers/target/target_core_cdb.c | 35 +++++++++++++++++++++++++++++++++-- >> 1 files changed, 33 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/target/target_core_cdb.c b/drivers/target/target_core_cdb.c >> index 89ae923..08bda0c 100644 >> --- a/drivers/target/target_core_cdb.c >> +++ b/drivers/target/target_core_cdb.c >> @@ -24,6 +24,7 @@ >> */ >> >> #include >> +#include >> #include >> #include >> >> @@ -154,6 +155,37 @@ target_emulate_evpd_80(struct se_cmd *cmd, unsigned char >> *buf) >> return 0; >> } >> >> +static void >> +target_parse_naa_6h_vendor_specific(struct se_device *dev, unsigned char >> *buf_off) >> +{ >> + unsigned char *p =&dev->se_sub_dev->t10_wwn.unit_serial[0]; >> + unsigned char *buf = buf_off; >> + int cnt = 0, next = 1; >> + /* >> + * Generate up to 36 bits of VENDOR SPECIFIC IDENTIFIER starting on >> + * byte 3 bit 4 for NAA IEEE Registered Extended DESIGNATOR field >> + * format, followed by 64 bits of VENDOR SPECIFIC IDENTIFIER EXTENSION >> + * to complete the payload. These are based from VPD=0x80 PRODUCT SERIAL >> + * NUMBER set via vpd_unit_serial in target_core_configfs.c to to ensure >> + * per device uniqeness. >> + */ >> + while (p != NULL) { >> + if (cnt>= 13) >> + break; >> + if (!isxdigit(*p)) { >> + p++; >> + continue; >> + } >> + if (next != 0) { >> + buf[cnt++] |= hex_to_bin(*p++); >> + next = 0; >> + } else { >> + buf[cnt] |= hex_to_bin(*p++)<< 4; >> + next = 1; >> + } >> + } >> +} >> + >> /* >> * Device identification VPD, for a complete list of >> * DESIGNATOR TYPEs see spc4r17 Table 459. >> @@ -219,8 +251,7 @@ target_emulate_evpd_83(struct se_cmd *cmd, unsigned char >> *buf) >> * VENDOR_SPECIFIC_IDENTIFIER and >> * VENDOR_SPECIFIC_IDENTIFIER_EXTENTION >> */ >> - buf[off++] |= hex_to_bin(dev->se_sub_dev->t10_wwn.unit_serial[0]); >> - hex2bin(&buf[off],&dev->se_sub_dev->t10_wwn.unit_serial[1], 12); >> + target_parse_naa_6h_vendor_specific(dev,&buf[off]); >> >> len = 20; >> off = (len + 4); > >