Re: [PATCH 2/2] scsi: qla2xxx: print MAC via %pMR
On Thu, 2012-07-26 at 08:23 -0400, Chad Dupuis wrote: On Thu, 12 Jul 2012, Andy Shevchenko wrote: Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com --- drivers/scsi/qla2xxx/qla_attr.c |5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c index 5ab9530..095ba85 100644 --- a/drivers/scsi/qla2xxx/qla_attr.c +++ b/drivers/scsi/qla2xxx/qla_attr.c @@ -1193,10 +1193,7 @@ qla2x00_vn_port_mac_address_show(struct device *dev, if (!IS_CNA_CAPABLE(vha-hw)) return snprintf(buf, PAGE_SIZE, \n); - return snprintf(buf, PAGE_SIZE, %02x:%02x:%02x:%02x:%02x:%02x\n, - vha-fcoe_vn_port_mac[5], vha-fcoe_vn_port_mac[4], - vha-fcoe_vn_port_mac[3], vha-fcoe_vn_port_mac[2], - vha-fcoe_vn_port_mac[1], vha-fcoe_vn_port_mac[0]); + return snprintf(buf, PAGE_SIZE, %pMR\n, vha-fcoe_vn_port_mac); } static ssize_t Acked-by: Chad Dupuis chad.dup...@qlogic.com James, do you have any comments on this patch and patch 1/2? -- Andy Shevchenko andriy.shevche...@linux.intel.com Intel Finland Oy -- 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] fusion: print lan address via %pMR
On Thu, 2012-07-12 at 10:55 +0300, Andy Shevchenko wrote:
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
drivers/message/fusion/mptbase.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/message/fusion/mptbase.c
b/drivers/message/fusion/mptbase.c
index d99db56..d202f25 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -2562,10 +2562,7 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u32 reason, int
sleepFlag)
(void) GetLanConfigPages(ioc);
a =
(u8*)ioc-lan_cnfg_page1.HardwareAddressLow;
dprintk(ioc, printk(MYIOC_s_DEBUG_FMT
- LanAddr = %02X:%02X:%02X
- :%02X:%02X:%02X\n,
- ioc-name, a[5], a[4],
- a[3], a[2], a[1], a[0]));
+ LanAddr = %pMR\n, ioc-name, a));
}
break;
@@ -6785,8 +6782,7 @@ static int mpt_iocinfo_proc_show(struct seq_file *m,
void *v)
if (ioc-bus_type == FC) {
if (ioc-pfacts[p].ProtocolFlags
MPI_PORTFACTS_PROTOCOL_LAN) {
u8 *a =
(u8*)ioc-lan_cnfg_page1.HardwareAddressLow;
- seq_printf(m, LanAddr =
%02X:%02X:%02X:%02X:%02X:%02X\n,
- a[5], a[4], a[3], a[2], a[1],
a[0]);
+ seq_printf(m, LanAddr = %pMR\n, a);
}
seq_printf(m, WWN = %08X%08X:%08X%08X\n,
ioc-fc_port_page0[p].WWNN.High,
@@ -6863,8 +6859,7 @@ mpt_print_ioc_summary(MPT_ADAPTER *ioc, char *buffer,
int *size, int len, int sh
if (showlan (ioc-pfacts[0].ProtocolFlags
MPI_PORTFACTS_PROTOCOL_LAN)) {
u8 *a = (u8*)ioc-lan_cnfg_page1.HardwareAddressLow;
- y += sprintf(buffer+len+y, ,
LanAddr=%02X:%02X:%02X:%02X:%02X:%02X,
- a[5], a[4], a[3], a[2], a[1], a[0]);
+ y += sprintf(buffer+len+y, , LanAddr=%pMR, a);
}
y += sprintf(buffer+len+y, , IRQ=%d, ioc-pci_irq);
@@ -6897,8 +6892,7 @@ static void seq_mpt_print_ioc_summary(MPT_ADAPTER *ioc,
struct seq_file *m, int
if (showlan (ioc-pfacts[0].ProtocolFlags
MPI_PORTFACTS_PROTOCOL_LAN)) {
u8 *a = (u8*)ioc-lan_cnfg_page1.HardwareAddressLow;
- seq_printf(m, , LanAddr=%02X:%02X:%02X:%02X:%02X:%02X,
- a[5], a[4], a[3], a[2], a[1], a[0]);
+ seq_printf(m, , LanAddr=%pMR, a);
}
seq_printf(m, , IRQ=%d, ioc-pci_irq);
James, do you have any comments on this one?
--
Andy Shevchenko andriy.shevche...@linux.intel.com
Intel Finland Oy
--
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 2/8] iscsi_transport: Additional parameters for net settings
Come on man, same comments as last patches like this :) Could you use the same names that we currently use for existing params? Check for the inorder ones and the ones where we use en instead of enabled for the postfix. Also check the others. Also what is up with isns. If we support passing the addr/port/enabled then will it work completely in fw? You do not need any driver or userspace changes for that? On 05/09/2013 05:02 AM, vikas.chaudh...@qlogic.com wrote: From: Harish Zunjarrao harish.zunjar...@qlogic.com Added support to display and update additional network parameters through iscsiadm Signed-off-by: Harish Zunjarrao harish.zunjar...@qlogic.com Signed-off-by: Vikas Chaudhary vikas.chaudh...@qlogic.com --- drivers/scsi/scsi_transport_iscsi.c | 331 +++- include/scsi/iscsi_if.h | 78 + include/scsi/scsi_transport_iscsi.h | 3 + 3 files changed, 411 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c index 133926b..315c8b6 100644 --- a/drivers/scsi/scsi_transport_iscsi.c +++ b/drivers/scsi/scsi_transport_iscsi.c @@ -306,11 +306,38 @@ show_##type##_##name(struct device *dev, struct device_attribute *attr, \ iscsi_iface_attr_show(type, name, ISCSI_NET_PARAM, param) \ static ISCSI_IFACE_ATTR(type, name, S_IRUGO, show_##type##_##name, NULL); -/* generic read only ipvi4 attribute */ +/* generic read only ipv4 attribute */ iscsi_iface_net_attr(ipv4_iface, ipaddress, ISCSI_NET_PARAM_IPV4_ADDR); iscsi_iface_net_attr(ipv4_iface, gateway, ISCSI_NET_PARAM_IPV4_GW); iscsi_iface_net_attr(ipv4_iface, subnet, ISCSI_NET_PARAM_IPV4_SUBNET); iscsi_iface_net_attr(ipv4_iface, bootproto, ISCSI_NET_PARAM_IPV4_BOOTPROTO); +iscsi_iface_net_attr(ipv4_iface, dhcp_dns_address_enabled, + ISCSI_NET_PARAM_IPV4_DHCP_DNS_ADDR_EN); +iscsi_iface_net_attr(ipv4_iface, dhcp_slp_da_info_enabled, + ISCSI_NET_PARAM_IPV4_DHCP_SLP_DA_EN); +iscsi_iface_net_attr(ipv4_iface, dhcp_req_isns_info_enabled, + ISCSI_NET_PARAM_IPV4_DHCP_REQ_ISNS_INFO_EN); +iscsi_iface_net_attr(ipv4_iface, tos_enabled, ISCSI_NET_PARAM_IPV4_TOS_EN); +iscsi_iface_net_attr(ipv4_iface, tos, ISCSI_NET_PARAM_IPV4_TOS); +iscsi_iface_net_attr(ipv4_iface, grat_arp_enabled, + ISCSI_NET_PARAM_IPV4_GRAT_ARP_EN); +iscsi_iface_net_attr(ipv4_iface, dhcp_alt_client_id_enabled, + ISCSI_NET_PARAM_IPV4_DHCP_ALT_CLIENT_ID_EN); +iscsi_iface_net_attr(ipv4_iface, dhcp_alt_client_id, + ISCSI_NET_PARAM_IPV4_DHCP_ALT_CLIENT_ID); +iscsi_iface_net_attr(ipv4_iface, dhcp_req_vendor_id_enabled, + ISCSI_NET_PARAM_IPV4_DHCP_REQ_VENDOR_ID_EN); +iscsi_iface_net_attr(ipv4_iface, dhcp_use_vendor_id_enabled, + ISCSI_NET_PARAM_IPV4_DHCP_USE_VENDOR_ID_EN); +iscsi_iface_net_attr(ipv4_iface, dhcp_vendor_id, + ISCSI_NET_PARAM_IPV4_DHCP_VENDOR_ID); +iscsi_iface_net_attr(ipv4_iface, dhcp_learn_iqn_enabled, + ISCSI_NET_PARAM_IPV4_DHCP_LEARN_IQN_EN); +iscsi_iface_net_attr(ipv4_iface, fragmentation_enabled, + ISCSI_NET_PARAM_IPV4_FRAGMENT_EN); +iscsi_iface_net_attr(ipv4_iface, incoming_forwarding_enabled, + ISCSI_NET_PARAM_IPV4_IN_FORWARD_EN); +iscsi_iface_net_attr(ipv4_iface, ttl, ISCSI_NET_PARAM_IPV4_TTL); /* generic read only ipv6 attribute */ iscsi_iface_net_attr(ipv6_iface, ipaddress, ISCSI_NET_PARAM_IPV6_ADDR); @@ -320,6 +347,27 @@ iscsi_iface_net_attr(ipv6_iface, ipaddr_autocfg, ISCSI_NET_PARAM_IPV6_ADDR_AUTOCFG); iscsi_iface_net_attr(ipv6_iface, link_local_autocfg, ISCSI_NET_PARAM_IPV6_LINKLOCAL_AUTOCFG); +iscsi_iface_net_attr(ipv6_iface, link_local_state, + ISCSI_NET_PARAM_IPV6_LINKLOCAL_STATE); +iscsi_iface_net_attr(ipv6_iface, router_state, + ISCSI_NET_PARAM_IPV6_ROUTER_STATE); +iscsi_iface_net_attr(ipv6_iface, grat_neighbor_adv_enabled, + ISCSI_NET_PARAM_IPV6_GRAT_NEIGHBOR_ADV_EN); +iscsi_iface_net_attr(ipv6_iface, mld_enabled, ISCSI_NET_PARAM_IPV6_MLD_EN); +iscsi_iface_net_attr(ipv6_iface, flow_label, ISCSI_NET_PARAM_IPV6_FLOW_LABEL); +iscsi_iface_net_attr(ipv6_iface, traffic_class, + ISCSI_NET_PARAM_IPV6_TRAFFIC_CLASS); +iscsi_iface_net_attr(ipv6_iface, hop_limit, ISCSI_NET_PARAM_IPV6_HOP_LIMIT); +iscsi_iface_net_attr(ipv6_iface, nd_reachable_tmo, + ISCSI_NET_PARAM_IPV6_ND_REACHABLE_TMO); +iscsi_iface_net_attr(ipv6_iface, nd_rexmit_time, + ISCSI_NET_PARAM_IPV6_ND_REXMIT_TIME); +iscsi_iface_net_attr(ipv6_iface, nd_stale_tmo, + ISCSI_NET_PARAM_IPV6_ND_STALE_TMO); +iscsi_iface_net_attr(ipv6_iface, dup_addr_det_cnt, + ISCSI_NET_PARAM_IPV6_DUP_ADDR_DET_CNT); +iscsi_iface_net_attr(ipv6_iface,
Re: [PATCH 2/8] iscsi_transport: Additional parameters for net settings
On 05/28/2013 03:49 AM, Mike Christie wrote: Come on man, same comments as last patches like this :) Could you use the same names that we currently use for existing params? Check for the inorder ones and the ones where we use en instead of enabled for the postfix. Also check the others. Also what is up with isns. If we support passing the addr/port/enabled then will it work completely in fw? You do not need any driver or userspace changes for that? For isns support does what you are adding work for flash mode only? -- 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 4/8] scsi_transport_iscsi: Exporting new attrs for iscsi session and connection in sysfs
On 05/09/2013 05:02 AM, vikas.chaudh...@qlogic.com wrote: + ISCSI_PARAM_DEF_TASKMGMT_TMO, We currently have: ISCSI_PARAM_ABORT_TMO, ISCSI_PARAM_LU_RESET_TMO, ISCSI_PARAM_HOST_RESET_TMO, Does your card just have the one timeout for all types of tmfs? I think the interface to set timers for the different eh operations should be generic. -- 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 1/1] bfa: Fix for possible null pointer dereference
-Original Message- From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- ow...@vger.kernel.org] On Behalf Of Jakob Normark Sent: Monday, May 27, 2013 1:21 AM To: Anil Gurumurthy; Vijaya Mohan Guvva Guvva; James E.J. Bottomley Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org; Jakob Normark Subject: [PATCH 1/1] bfa: Fix for possible null pointer dereference Fix for cppcheck error in bfa_fcs_lport.c Kernel version: v3.10-rc2 Signed-off-by: Jakob Normark jakobnorm...@gmail.com --- drivers/scsi/bfa/bfa_fcs_lport.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/bfa/bfa_fcs_lport.c b/drivers/scsi/bfa/bfa_fcs_lport.c index 1224d04..a37c45a 100644 --- a/drivers/scsi/bfa/bfa_fcs_lport.c +++ b/drivers/scsi/bfa/bfa_fcs_lport.c @@ -5615,13 +5615,14 @@ bfa_fcs_lport_get_rport_max_speed(bfa_fcs_lport_t *port) bfa_port_speed_t max_speed = 0; struct bfa_port_attr_s port_attr; bfa_port_speed_t port_speed, rport_speed; - bfa_boolean_t trl_enabled = bfa_fcport_is_ratelim(port-fcs-bfa); + bfa_boolean_t trl_enabled; if (port == NULL) return 0; fcs = port-fcs; + trl_enabled = bfa_fcport_is_ratelim(port-fcs-bfa); /* Get Physical port's current speed */ bfa_fcport_get_attr(port-fcs-bfa, port_attr); -- 1.7.9.5 -- Changes looks good. Thanks for the patch. Acked-by: Vijay Mohan Guvva vmo...@brocade.com -- 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 2/8] iscsi_transport: Additional parameters for net settings
-Original Message- From: Mike Christie micha...@cs.wisc.edu Date: Tuesday 28 May 2013 2:19 PM To: Vikas vikas.chaudh...@qlogic.com Cc: jbottom...@parallels.com jbottom...@parallels.com, scsi linux-scsi@vger.kernel.org, Lalit Chandivade lalit.chandiv...@qlogic.com, Ravi Anand ravi.an...@qlogic.com, Harish Zunjarrao harish.zunjar...@qlogic.com Subject: Re: [PATCH 2/8] iscsi_transport: Additional parameters for net settings Come on man, same comments as last patches like this :) Could you use the same names that we currently use for existing params? Check for the inorder ones and the ones where we use en instead of enabled for the postfix. Also check the others. If we understand correctly you are suggesting to change macro postfix from EN to ENABLED as in attached patch iscsi_net_param-fix1.patch. But we think changing old macro postfix from ENABLED to EN is better way to fix it. It will make iscsi_if.h consistent for iscsi_net_param, iscsi_param and iscsi_flashnode_param macros as defined in attached patch iscsi_net_param-fix2.patch. Let us know what you think? Also what is up with isns. If we support passing the addr/port/enabled then will it work completely in fw? You do not need any driver or userspace changes for that? For now we are just adding support to enable or disable iSNS. However we need to modify driver and userspace to support iSNS. attachment: winmail.dat
Re: [PATCH 4/8] scsi_transport_iscsi: Exporting new attrs for iscsi session and connection in sysfs
-Original Message- From: Mike Christie micha...@cs.wisc.edu Date: Tuesday 28 May 2013 2:27 PM To: Vikas vikas.chaudh...@qlogic.com Cc: jbottom...@parallels.com jbottom...@parallels.com, scsi linux-scsi@vger.kernel.org, Lalit Chandivade lalit.chandiv...@qlogic.com, Ravi Anand ravi.an...@qlogic.com, Adheer Chandravanshi adheer.chandravan...@qlogic.com Subject: Re: [PATCH 4/8] scsi_transport_iscsi: Exporting new attrs for iscsi session and connection in sysfs On 05/09/2013 05:02 AM, vikas.chaudh...@qlogic.com wrote: +ISCSI_PARAM_DEF_TASKMGMT_TMO, We currently have: ISCSI_PARAM_ABORT_TMO, ISCSI_PARAM_LU_RESET_TMO, ISCSI_PARAM_HOST_RESET_TMO, Does your card just have the one timeout for all types of tmfs? I think the interface to set timers for the different eh operations should be generic. We have added TASKMGMT_TMO as our adapter sets single TMO for all task management commands. Let us know if its ok or not? attachment: winmail.dat
Re: SCSI error handling -- one error blocks the whole SCSI host
On 5/27/2013 8:32 PM, Baruch Even wrote: necessary but the command itself if it is already actively handled continues in its path. The abort only cancels those commands that are in the queue and if there really was a problem and the disk is engaging in error recovery of its own you'll just have no response from it and it will seem dead (abort may timeout). Yes, the abort seems to be handled more like a hint in many cases. Having coded a couple targets, abort handling is often _REALLY_ hard to get 100% right. Especially, when its an actual error that is causing the delay, rather than a correctly functional long running command. That said, I've seen devices actually respond to aborts on tape ERASE and similar commands by actually aborting the command as one would expect. So it does sometimes work.. Besides abort timeouts (which is major bad karma) the abort may be accepted, and the next non inquiry/tur type command that gets queued simply blocks waiting for the abort to internally complete. From the target device perspective, if you don't send a response for ABTS out in 2*RA_TOV then your problems start to multiply. So it encourages the target devices to treat aborts in an async manner. As you said, the device simply finds the indicated command on a queue, marks it as being aborted and hopes whatever is processing the command notices and terminates its operation. On subsequent commands the nicer devices will notice the abort hasn't completed and return becoming ready or similar in response to TUR/etc for some number of minutes. This view of aborts also means that reducing timeouts for commands and TMFs is mostly useless and sometimes even a really bad idea. I prefer to just let the device go on with its error recovery and just forget about the command. I want to forget about the DMA so I issue an abort but anything higher than that means a link is dead to me. Well, invariably the manufactures have timeouts that are really long and based on internal error recovery logic. See http://www-01.ibm.com/support/docview.wss?uid=ssg1S7003556aid=1 page 468. Notice the timeouts are specified in minutes, not seconds. Furthermore, the commands that normally complete in fractions of a second have actual timeouts that can be tens of minutes (READ/WRITE for example). So, doing anything before that timeout has expired is a good way to knock the device offline. Some of the newer disks have mode page options to shorten their read/write error recovery, but short error recovery can still be many tens of seconds rather than a couple minutes. Plus, it doesn't help compound commands like SYNCHRONIZE CACHE which may take multiple errors during operation. This is another part of what formed my opinions about error isolation. If one of your devices goes out to lunch and isn't recovering via abort/lun reset. Its done! Wrecking the rest of the SAN doing bus resets and HBA resets is a good way to take a serious problem and turn it into a full blown catastrophe. -- 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 4/8] scsi_transport_iscsi: Exporting new attrs for iscsi session and connection in sysfs
On 05/28/2013 06:55 AM, Vikas Chaudhary wrote: -Original Message- From: Mike Christie micha...@cs.wisc.edu Date: Tuesday 28 May 2013 2:27 PM To: Vikas vikas.chaudh...@qlogic.com Cc: jbottom...@parallels.com jbottom...@parallels.com, scsi linux-scsi@vger.kernel.org, Lalit Chandivade lalit.chandiv...@qlogic.com, Ravi Anand ravi.an...@qlogic.com, Adheer Chandravanshi adheer.chandravan...@qlogic.com Subject: Re: [PATCH 4/8] scsi_transport_iscsi: Exporting new attrs for iscsi session and connection in sysfs On 05/09/2013 05:02 AM, vikas.chaudh...@qlogic.com wrote: + ISCSI_PARAM_DEF_TASKMGMT_TMO, We currently have: ISCSI_PARAM_ABORT_TMO, ISCSI_PARAM_LU_RESET_TMO, ISCSI_PARAM_HOST_RESET_TMO, Does your card just have the one timeout for all types of tmfs? I think the interface to set timers for the different eh operations should be generic. We have added TASKMGMT_TMO as our adapter sets single TMO for all task management commands. Let us know if its ok or not? I was thinking maybe a better name to reflect that it is for all tmfs, but I think it is ok. -- 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: SCSI error handling -- one error blocks the whole SCSI host
On Tue, May 28, 2013 at 5:38 PM, Jeremy Linton jlin...@tributary.com wrote: This is another part of what formed my opinions about error isolation. If one of your devices goes out to lunch and isn't recovering via abort/lun reset. Its done! Wrecking the rest of the SAN doing bus resets and HBA resets is a good way to take a serious problem and turn it into a full blown catastrophe. This is the gist of the issue, once you got to an abort you are screwed already. You need the abort but anything else should be reserved to when things are really dead (the HBA might still recover on a host reset, but only do it if the host is really unresponsive). That's why I prefer to have a long timeout for the command and a long timeout for the abort. The application above should handle itself with its own timeout once the abort was sent (the buffer remains locked until the abort returns). The device itself is likely stuck in error recovery and it will come out of it when its own internal timeouts are exhausted which can be infinite and will generally be very large. Baruch -- 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 v3 0/6] scsi_debug: bug fixes and cleanups for data integrity support
Perhaps I'm missing something but I can never get DIF stuff to do what I want in scsi_debug. It was like this before your patches as well. Assume this set up: # lsscsi -gp [1:0:0:0]diskATA ST3320620AS 3.AA /dev/sda /dev/sg0 - - [6:0:9:0]diskSEAGATE ST33000650SS 0002 /dev/sdb /dev/sg2 DIF/Type1 - [7:0:0:0]diskLinuxscsi_debug 0004 /dev/sdc /dev/sg3 DIF/Type1 - So 6:0:9:0 is a real disk formatted with protection type 1 and 7:0:0:0 is simulating the same thing (with scsi_debug). Now I try to read the first block with RDPROTECT=1 so in the data-out buffer I expect the LB plus the protection info. That as 512+8 bytes. So now I use ddpt to read the first block plus PI from the Seagate disk: # ddpt if=/dev/sg2 bs=512 verbose=3 protect=1 count=1 Output file not specified so no copy, just reading input READ cdb: 28 20 00 00 00 00 00 00 01 00 1+0 records in 0+0 records out time to read data: 0.000574 secs at 892.0 KB/sec Good, asked for 520 bytes and got them. But when I try that with scsi_debug disk: # ddpt if=/dev/sg3 bs=512 verbose=3 protect=1 count=1 ... Output file not specified so no copy, just reading input READ cdb: 28 20 00 00 00 00 00 00 01 00 READ: pass-through requested 520 bytes but got 512 bytes 1+0 records in 0+0 records out Non-zero sum of residual counts=8 time to read data: 0.000833 secs at 614.6 KB/sec No PI, why not? This was tested on lk 3.9.4 with your patches applied. Doug Gilbert On 13-05-26 04:01 AM, Akinobu Mita wrote: This patch set includes bug fixes which I hit when I was tried testing the data integrity support in scsi_debug on x86_32. And it also includes cleanups which helps increasing readability and further bug fixing in data integrity support. * Changes from v2 - Add new bug fix patch for UNMAP command support - Change the way to fix for the patch fix invalid address passed to kunmap_atomic() - Reduce more lines of code for the patch reduce duplication between prot_verify_read and prot_verify_writ * Changes from v1 - Split the patch fix data integrity support on highmem machine into two separate patches. - Add new cleanup patch reduce duplication between prot_verify_read and prot_verify_write. Cc: James E.J. Bottomley jbottom...@parallels.com Cc: Douglas Gilbert dgilb...@interlog.com Cc: Martin K. Petersen martin.peter...@oracle.com Cc: linux-scsi@vger.kernel.org Akinobu Mita (6): scsi_debug: fix invalid address passed to kunmap_atomic() scsi_debug: fix incorrectly nested kmap_atomic() scsi_debug: fix NULL pointer dereference with parameters dif=0 dix=1 scsi_debug: invalidate protection info for unmapped region scsi_debug: simplify offset calculation for dif_storep scsi_debug: reduce duplication between prot_verify_read and prot_verify_write drivers/scsi/scsi_debug.c | 176 +++--- 1 file changed, 72 insertions(+), 104 deletions(-) -- 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 v3 0/6] scsi_debug: bug fixes and cleanups for data integrity support
Doug == Douglas Gilbert dgilb...@interlog.com writes: Doug No PI, why not? This was tested on lk 3.9.4 with your patches Doug applied. I've been communicating with Akinobu offline about this. The reason is that I really only implemented DIX support in scsi_debug. The dif module parameter is only there to select the reported protection type. The main problem with scsi_debug is that it's both initiator and target in one. And there isn't any notion of a data transfer between the initiator and the target. We just fill out the scatterlists with stuff from the in-memory buffer. To support DIX/DIF correctly we'd have to have the notion of a handoff between the front and back of scsi_debug. And we'd have to support all combinations of DIX and DIF with the relevant slicing and dicing of data and PI buffers. I have some patches pending as part of my next DIF/DIX update that makes some of these things more palatable at the block/SCSI level. Akinobu voiced interest in finishing the scsi_debug work on top of my code. -- Martin K. Petersen Oracle Linux Engineering -- 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: PING^7 (was Re: [PATCH v2 00/14] Corrections and customization of the SG_IO command whitelist (CVE-2012-4542))
Vladislav == Vladislav Bolkhovitin v...@vlnb.net writes: Vladislav Linux block layer is purely artificial creature slowly Vladislav reinventing wheel creating more problems, than solving. On the contrary. I do think we solve a whole bunch of problems. Vladislav It enforces approach, where often impossible means Vladislav impossible in this interface. I agree we have limitations. I do not agree that all limitations are bad. Sometimes it's OK to say no. Vladislav For instance, how about copy offload? How about atomic Vladislav writes? I'm actively working on copy offload. Nobody appears to be interested in atomic writes. Otherwise I'd work on those as well. Vladislav Why was it needed to create special blk integrity interface Vladislav with the only end user - SCSI? Simple. Because we did not want to interleave data and PI 512+8+512+8 neither in memory, nor at DMA time. Furthermore, the ATA EPP proposal was still on the table so I also needed to support ATA. And finally, NVM Express uses the blk_integrity interface as well. Vladislav The block layer keeps repeating SCSI. So, maybe, after all, Vladislav it's better to acknowledge that direct usage of SCSI without Vladislav any intermediate layers and translations is more productive? Vladislav And for those minors not using SCSI internally, translate Vladislav from SCSI to their internal commands? Creating and filling Vladislav CDB fields for most cases isn't anyhow harder, than creating Vladislav and feeling bio fields. This is quite possibly the worst idea I have heard all week. As it stands it's a headache for the disk ULD driver to figure out which of the bazillion READ/WRITE variants to send to a SCSI/ATA device. What makes you think that an application or filesystem would be better equipped to make that call? See also: WRITE SAME w/ zeroes vs. WRITE SAME w/ UNMAP vs. UNMAP See also: EXTENDED COPY vs. the PROXY command set See also: USB-ATA bridge chips You make it sound like all the block layer does is filling out CDBs. Which it doesn't in fact have anything to do with at all. When you are talking about CDBs we're down in the SBC/SSC territory. Which is such a tiny bit of what's going on. We have transports, we have SAM, we have HBA controller DMA constraints, system DMA constraints, buffer bouncing, etc. There's a ton of stuff that needs to happen before the CDB and the data physically reach the storage. You seem to be advocating that everything up to the point where the device receives the command is in the way. Well, by all means. Why limit ourselves to the confines of SCSI? Why not get rid of POSIX read()/write(), page cache, filesystems and let applications speak ST-506 directly? I know we're doing different things. My job is to make a general purpose operating system with interfaces that make sense to normal applications. That does not preclude special cases where it may make sense to poke at the device directly. For testing purposes, for instance. But I consider it a failure when we start having applications that know about hardware intricacies, cylinders/heads/sectors, etc. That road leads straight to the 1980s... -- Martin K. Petersen Oracle Linux Engineering -- 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 0/2] Minor scsi_dh_emc error path fixes
These two patches spun out of the blk_get_request return type changes I've been working on [1]. [1] http://thread.gmane.org/gmane.linux.scsi/80934 Should that patch go in, there are a few other device handler changes possible for 3.11 [2], but the following bugfixes are unrelated, so I thought I'd post them in case they could apply for 3.10. [2] http://thread.gmane.org/gmane.linux.scsi/81130 The first bug was spotted by Mike in thread [2], the second when I traced the call path of clariion_send_inquiry back out through clariion_set_params and its caller parse_path. The latter fix could have been made in parse_path, but it didn't seem appropriate for that file to sort through SCSI_DH enums. Comments welcome, unfortunately I don't have the HW to verify, so these are only compile tested. Thanks, Joe Lawrence (2): scsi_dh_emc: handle zero-senselen send_inquiry_cmd errors scsi_dh_emc: set_params callback should consistently return errno drivers/scsi/device_handler/scsi_dh_emc.c | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) -- 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
scsi_dh_emc: handle zero-senselen send_inquiry_cmd errors
From c6645f639f7e7551c7a6f0aacee78e57ddd37bc1 Mon Sep 17 00:00:00 2001
From: Joe Lawrence joe.lawre...@stratus.com
Date: Tue, 28 May 2013 14:49:37 -0400
Subject: [PATCH 1/2] scsi_dh_emc: handle zero-senselen send_inquiry_cmd
errors
The send_inquiry_cmd function may exit without setting senselen if
get_req fails. Callers shouldn't assume senselen is set non-zero in
their failure checking.
Signed-off-by: Joe Lawrence joe.lawre...@stratus.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Hannes Reinecke h...@suse.de
Cc: Mike Christie micha...@cs.wisc.edu
---
drivers/scsi/device_handler/scsi_dh_emc.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c
b/drivers/scsi/device_handler/scsi_dh_emc.c
index e1c8be0..0438ed6 100644
--- a/drivers/scsi/device_handler/scsi_dh_emc.c
+++ b/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -468,9 +468,12 @@ static int clariion_std_inquiry(struct scsi_device *sdev,
char *sp_model;
err = send_inquiry_cmd(sdev, 0, csdev);
- if (err != SCSI_DH_OK csdev-senselen) {
+ if (err != SCSI_DH_OK) {
struct scsi_sense_hdr sshdr;
+ if (!csdev-senselen)
+ goto out;
+
if (scsi_normalize_sense(csdev-sense, SCSI_SENSE_BUFFERSIZE,
sshdr)) {
sdev_printk(KERN_ERR, sdev, %s: INQUIRY sense code
@@ -507,9 +510,12 @@ static int clariion_send_inquiry(struct scsi_device *sdev,
retry:
err = send_inquiry_cmd(sdev, 0xC0, csdev);
- if (err != SCSI_DH_OK csdev-senselen) {
+ if (err != SCSI_DH_OK) {
struct scsi_sense_hdr sshdr;
+ if (!csdev-senselen)
+ goto out;
+
err = scsi_normalize_sense(csdev-sense, SCSI_SENSE_BUFFERSIZE,
sshdr);
if (!err)
@@ -527,6 +533,7 @@ retry:
} else {
err = parse_sp_info_reply(sdev, csdev);
}
+out:
return err;
}
--
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
scsi_dh_emc: set_params callback should consistently return errno
From db6c2d4406732585fd7a658fc89b14fa26e7d1d4 Mon Sep 17 00:00:00 2001
From: Joe Lawrence joe.lawre...@stratus.com
Date: Tue, 28 May 2013 15:47:20 -0400
Subject: [PATCH 2/2] scsi_dh_emc: set_params callback should consistently
return errno
A SCSI scsi_device_handler set_params routine should return a negative
errno value on failure. The scsi_dh_emc driver may return not only
-EINVAL but also also positive SCSI_DH enum values. Fix the
clariion_set_params implementation to return a negative value in all
error scenarios. To that end, make sure send_trespass_cmd consistently
returns a SCSI_DH enum.
Signed-off-by: Joe Lawrence joe.lawre...@stratus.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Hannes Reinecke h...@suse.de
Cc: Mike Christie micha...@cs.wisc.edu
---
drivers/scsi/device_handler/scsi_dh_emc.c |6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c
b/drivers/scsi/device_handler/scsi_dh_emc.c
index 0438ed6..6ca9e84 100644
--- a/drivers/scsi/device_handler/scsi_dh_emc.c
+++ b/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -389,6 +389,7 @@ static int send_trespass_cmd(struct scsi_device *sdev,
if (rq-sense_len) {
err = trespass_endio(sdev, csdev-sense);
} else {
+ err = SCSI_DH_IO;
sdev_printk(KERN_INFO, sdev,
%s: failed to send MODE SELECT: %x\n,
CLARIION_NAME, rq-errors);
@@ -626,7 +627,10 @@ static int clariion_set_params(struct scsi_device *sdev,
const char *params)
result = clariion_send_inquiry(sdev, csdev);
done:
- return result;
+ if (result != SCSI_DH_OK)
+ return -EIO;
+
+ return 0;
}
static const struct scsi_dh_devlist clariion_dev_list[] = {
--
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
