Re: [PATCH] scsi/ibmvscsi: /sys/class/scsi_host/hostX/config doesn't show any information
On Sun, Jul 29, 2012 at 8:33 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: scsi/ibmvscsi: Fix host config length field overflow The length field in the host config packet is only 16-bit long, so passing it 0x1 (64K which is our standard PAGE_SIZE) doesn't work and result in an empty config from the server. Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org CC: sta...@vger.kernel.org James, can this be added to your for-next branch so that we can also get this to the stable trees? Thanks. Acked-by: Robert Jennings r...@linux.vnet.ibm.com --- diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 3a6c474..337e8b3 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -1541,6 +1541,9 @@ static int ibmvscsi_do_host_config(struct ibmvscsi_host_data *hostdata, host_config = evt_struct-iu.mad.host_config; + /* The transport length field is only 16-bit */ + length = min(0x, length); + /* Set up a lun reset SRP command */ memset(host_config, 0x00, sizeof(*host_config)); host_config-common.type = VIOSRP_HOST_CONFIG_TYPE; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi/ibmvscsi: /sys/class/scsi_host/hostX/config doesn't show any information
On Sun, Jul 29, 2012 at 8:33 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: n Wed, 2012-07-18 at 18:49 +0200, o...@aepfle.de wrote: From: Linda Xie lx...@us.ibm.com Expected result: It should show something like this: x1521p4:~ # cat /sys/class/scsi_host/host1/config PARTITIONNAME='x1521p4' NWSDNAME='X1521P4' HOSTNAME='X1521P4' DOMAINNAME='RCHLAND.IBM.COM' NAMESERVERS='9.10.244.100 9.10.244.200' Actual result: x1521p4:~ # cat /sys/class/scsi_host/host0/config x1521p4:~ # This patch changes the size of the buffer used for transfering config data to 4K. It was tested against 2.6.19-rc2 tree. Reported by IBM during SLES11 beta testing: So this patch just seems to blindly replace all occurrences of PAGE_SIZE with HOST_PAGE_SIZE which is utterly wrong. Only one of those needs to be changed, the one passed to ibmvscsi_do_host_config() which is what's visible to the server, all the rest is just sysfs attributes and should remain as-is. Additionally (not even mentioning that there is no explanation as to what the real problem is anywhere in the changeset) I don't like the fix. The root of the problem is that the MAD header has a 16-bit length field, so writing 0x1 (64K PAGE_SIZE) into it doesn't quite work. So in addition to a better comment, I would suggest a fix more like this: scsi/ibmvscsi: Fix host config length field overflow The length field in the host config packet is only 16-bit long, so passing it 0x1 (64K which is our standard PAGE_SIZE) doesn't work and result in an empty config from the server. Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org CC: sta...@vger.kernel.org Acked-by: Robert Jennings r...@linux.vnet.ibm.com Tested with an IBM i host and confirmed the fix. --- diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 3a6c474..337e8b3 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -1541,6 +1541,9 @@ static int ibmvscsi_do_host_config(struct ibmvscsi_host_data *hostdata, host_config = evt_struct-iu.mad.host_config; + /* The transport length field is only 16-bit */ + length = min(0x, length); + /* Set up a lun reset SRP command */ memset(host_config, 0x00, sizeof(*host_config)); host_config-common.type = VIOSRP_HOST_CONFIG_TYPE; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi/ibmvscsi: /sys/class/scsi_host/hostX/config doesn't show any information
n Wed, 2012-07-18 at 18:49 +0200, o...@aepfle.de wrote: From: Linda Xie lx...@us.ibm.com Expected result: It should show something like this: x1521p4:~ # cat /sys/class/scsi_host/host1/config PARTITIONNAME='x1521p4' NWSDNAME='X1521P4' HOSTNAME='X1521P4' DOMAINNAME='RCHLAND.IBM.COM' NAMESERVERS='9.10.244.100 9.10.244.200' Actual result: x1521p4:~ # cat /sys/class/scsi_host/host0/config x1521p4:~ # This patch changes the size of the buffer used for transfering config data to 4K. It was tested against 2.6.19-rc2 tree. Reported by IBM during SLES11 beta testing: So this patch just seems to blindly replace all occurrences of PAGE_SIZE with HOST_PAGE_SIZE which is utterly wrong. Only one of those needs to be changed, the one passed to ibmvscsi_do_host_config() which is what's visible to the server, all the rest is just sysfs attributes and should remain as-is. Additionally (not even mentioning that there is no explanation as to what the real problem is anywhere in the changeset) I don't like the fix. The root of the problem is that the MAD header has a 16-bit length field, so writing 0x1 (64K PAGE_SIZE) into it doesn't quite work. So in addition to a better comment, I would suggest a fix more like this: scsi/ibmvscsi: Fix host config length field overflow The length field in the host config packet is only 16-bit long, so passing it 0x1 (64K which is our standard PAGE_SIZE) doesn't work and result in an empty config from the server. Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org CC: sta...@vger.kernel.org --- diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 3a6c474..337e8b3 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -1541,6 +1541,9 @@ static int ibmvscsi_do_host_config(struct ibmvscsi_host_data *hostdata, host_config = evt_struct-iu.mad.host_config; + /* The transport length field is only 16-bit */ + length = min(0x, length); + /* Set up a lun reset SRP command */ memset(host_config, 0x00, sizeof(*host_config)); host_config-common.type = VIOSRP_HOST_CONFIG_TYPE; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi/ibmvscsi: /sys/class/scsi_host/hostX/config doesn't show any information
On Fri, 2012-07-27 at 15:19 +1000, Benjamin Herrenschmidt wrote: On Wed, 2012-07-18 at 18:49 +0200, o...@aepfle.de wrote: From: Linda Xie lx...@us.ibm.com James, can I assume you're picking up those two ? If they get acked by the maintiners ... James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi/ibmvscsi: /sys/class/scsi_host/hostX/config doesn't show any information
On Wed, 2012-07-18 at 18:49 +0200, o...@aepfle.de wrote: From: Linda Xie lx...@us.ibm.com James, can I assume you're picking up those two ? Cheers, Ben. Expected result: It should show something like this: x1521p4:~ # cat /sys/class/scsi_host/host1/config PARTITIONNAME='x1521p4' NWSDNAME='X1521P4' HOSTNAME='X1521P4' DOMAINNAME='RCHLAND.IBM.COM' NAMESERVERS='9.10.244.100 9.10.244.200' Actual result: x1521p4:~ # cat /sys/class/scsi_host/host0/config x1521p4:~ # This patch changes the size of the buffer used for transfering config data to 4K. It was tested against 2.6.19-rc2 tree. Reported by IBM during SLES11 beta testing: https://bugzilla.novell.com/show_bug.cgi?id=439970 LTC49349 Signed-off-by: Olaf Hering o...@aepfle.de diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index e580aa4..1513ca8 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -93,6 +93,8 @@ static int max_requests = IBMVSCSI_MAX_REQUESTS_DEFAULT; static int max_events = IBMVSCSI_MAX_REQUESTS_DEFAULT + 2; static int fast_fail = 1; static int client_reserve = 1; +/* host data buffer size */ +#define HOST_BUFFER_SIZE 4096 static struct scsi_transport_template *ibmvscsi_transport_template; @@ -1666,7 +1668,7 @@ static ssize_t show_host_srp_version(struct device *dev, struct ibmvscsi_host_data *hostdata = shost_priv(shost); int len; - len = snprintf(buf, PAGE_SIZE, %s\n, + len = snprintf(buf, HOST_BUFFER_SIZE, %s\n, hostdata-madapter_info.srp_version); return len; } @@ -1687,7 +1689,7 @@ static ssize_t show_host_partition_name(struct device *dev, struct ibmvscsi_host_data *hostdata = shost_priv(shost); int len; - len = snprintf(buf, PAGE_SIZE, %s\n, + len = snprintf(buf, HOST_BUFFER_SIZE, %s\n, hostdata-madapter_info.partition_name); return len; } @@ -1708,7 +1710,7 @@ static ssize_t show_host_partition_number(struct device *dev, struct ibmvscsi_host_data *hostdata = shost_priv(shost); int len; - len = snprintf(buf, PAGE_SIZE, %d\n, + len = snprintf(buf, HOST_BUFFER_SIZE, %d\n, hostdata-madapter_info.partition_number); return len; } @@ -1728,7 +1730,7 @@ static ssize_t show_host_mad_version(struct device *dev, struct ibmvscsi_host_data *hostdata = shost_priv(shost); int len; - len = snprintf(buf, PAGE_SIZE, %d\n, + len = snprintf(buf, HOST_BUFFER_SIZE, %d\n, hostdata-madapter_info.mad_version); return len; } @@ -1748,7 +1750,7 @@ static ssize_t show_host_os_type(struct device *dev, struct ibmvscsi_host_data *hostdata = shost_priv(shost); int len; - len = snprintf(buf, PAGE_SIZE, %d\n, hostdata-madapter_info.os_type); + len = snprintf(buf, HOST_BUFFER_SIZE, %d\n, hostdata-madapter_info.os_type); return len; } @@ -1767,7 +1769,7 @@ static ssize_t show_host_config(struct device *dev, struct ibmvscsi_host_data *hostdata = shost_priv(shost); /* returns null-terminated host config data */ - if (ibmvscsi_do_host_config(hostdata, buf, PAGE_SIZE) == 0) + if (ibmvscsi_do_host_config(hostdata, buf, HOST_BUFFER_SIZE) == 0) return strlen(buf); else return 0; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi/ibmvscsi: /sys/class/scsi_host/hostX/config doesn't show any information
From: Linda Xie lx...@us.ibm.com Expected result: It should show something like this: x1521p4:~ # cat /sys/class/scsi_host/host1/config PARTITIONNAME='x1521p4' NWSDNAME='X1521P4' HOSTNAME='X1521P4' DOMAINNAME='RCHLAND.IBM.COM' NAMESERVERS='9.10.244.100 9.10.244.200' Actual result: x1521p4:~ # cat /sys/class/scsi_host/host0/config x1521p4:~ # This patch changes the size of the buffer used for transfering config data to 4K. It was tested against 2.6.19-rc2 tree. Reported by IBM during SLES11 beta testing: https://bugzilla.novell.com/show_bug.cgi?id=439970 LTC49349 Signed-off-by: Olaf Hering o...@aepfle.de diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index e580aa4..1513ca8 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -93,6 +93,8 @@ static int max_requests = IBMVSCSI_MAX_REQUESTS_DEFAULT; static int max_events = IBMVSCSI_MAX_REQUESTS_DEFAULT + 2; static int fast_fail = 1; static int client_reserve = 1; +/* host data buffer size */ +#define HOST_BUFFER_SIZE 4096 static struct scsi_transport_template *ibmvscsi_transport_template; @@ -1666,7 +1668,7 @@ static ssize_t show_host_srp_version(struct device *dev, struct ibmvscsi_host_data *hostdata = shost_priv(shost); int len; - len = snprintf(buf, PAGE_SIZE, %s\n, + len = snprintf(buf, HOST_BUFFER_SIZE, %s\n, hostdata-madapter_info.srp_version); return len; } @@ -1687,7 +1689,7 @@ static ssize_t show_host_partition_name(struct device *dev, struct ibmvscsi_host_data *hostdata = shost_priv(shost); int len; - len = snprintf(buf, PAGE_SIZE, %s\n, + len = snprintf(buf, HOST_BUFFER_SIZE, %s\n, hostdata-madapter_info.partition_name); return len; } @@ -1708,7 +1710,7 @@ static ssize_t show_host_partition_number(struct device *dev, struct ibmvscsi_host_data *hostdata = shost_priv(shost); int len; - len = snprintf(buf, PAGE_SIZE, %d\n, + len = snprintf(buf, HOST_BUFFER_SIZE, %d\n, hostdata-madapter_info.partition_number); return len; } @@ -1728,7 +1730,7 @@ static ssize_t show_host_mad_version(struct device *dev, struct ibmvscsi_host_data *hostdata = shost_priv(shost); int len; - len = snprintf(buf, PAGE_SIZE, %d\n, + len = snprintf(buf, HOST_BUFFER_SIZE, %d\n, hostdata-madapter_info.mad_version); return len; } @@ -1748,7 +1750,7 @@ static ssize_t show_host_os_type(struct device *dev, struct ibmvscsi_host_data *hostdata = shost_priv(shost); int len; - len = snprintf(buf, PAGE_SIZE, %d\n, hostdata-madapter_info.os_type); + len = snprintf(buf, HOST_BUFFER_SIZE, %d\n, hostdata-madapter_info.os_type); return len; } @@ -1767,7 +1769,7 @@ static ssize_t show_host_config(struct device *dev, struct ibmvscsi_host_data *hostdata = shost_priv(shost); /* returns null-terminated host config data */ - if (ibmvscsi_do_host_config(hostdata, buf, PAGE_SIZE) == 0) + if (ibmvscsi_do_host_config(hostdata, buf, HOST_BUFFER_SIZE) == 0) return strlen(buf); else return 0; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
