-----Original Message----- From: Mike Christie [mailto:micha...@cs.wisc.edu] Sent: Tuesday, September 17, 2013 2:16 PM To: Jayamohan Kallickal Cc: Jayamohan Kallickal; jbottom...@parallels.com; linux-scsi@vger.kernel.org; Sony John-N Subject: Re: [PATCH 04/22] be2iscsi: Fix negotiated parameters upload to FW
On 09/16/2013 11:28 PM, Jayamohan Kallickal wrote: > > > -----Original Message----- > From: Mike Christie [mailto:micha...@cs.wisc.edu] > Sent: Monday, September 16, 2013 7:59 PM > To: Jayamohan Kallickal > Cc: jbottom...@parallels.com; linux-scsi@vger.kernel.org; Jayamohan > Kallickal; Sony John-N > Subject: Re: [PATCH 04/22] be2iscsi: Fix negotiated parameters upload > to FW > > On 09/13/2013 12:09 AM, Jayamohan Kallickal wrote: >> - If target does not send MaxRecvDSL in login repsonse, then >> initiator should consider the MaxRecvDSL for target is 8K. >> In this scenario driver was setting the value to 64K and this >> caused target to close cxn as data xfer was more than the >> MaxRecvDSL >> - Update connection offload data structure for SKH-R adapters. >> >> Signed-off-by: John Soni Jose <sony.joh...@emulex.com> >> Signed-off-by: Jayamohan Kallickal <jayamohan.kallic...@emulex.com> >> --- >> drivers/scsi/be2iscsi/be_iscsi.c | 9 +++++++-- >> drivers/scsi/be2iscsi/be_main.h | 29 ++++++++++++++++------------- >> drivers/scsi/be2iscsi/be_mgmt.c | 8 +++----- >> 3 files changed, 26 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/scsi/be2iscsi/be_iscsi.c >> b/drivers/scsi/be2iscsi/be_iscsi.c >> index 2496ea7..60c1dff 100644 >> --- a/drivers/scsi/be2iscsi/be_iscsi.c >> +++ b/drivers/scsi/be2iscsi/be_iscsi.c >> @@ -672,9 +672,10 @@ int beiscsi_set_param(struct iscsi_cls_conn *cls_conn, >> session->max_burst = 262144; >> break; >> case ISCSI_PARAM_MAX_XMIT_DLENGTH: >> - if ((conn->max_xmit_dlength > 65536) || >> - (conn->max_xmit_dlength == 0)) >> + if (conn->max_xmit_dlength > 65536) >> conn->max_xmit_dlength = 65536; >> + else if (conn->max_xmit_dlength == 0) >> + conn->max_xmit_dlength = 8192; > >> Was the target sending 0 or not sending anything at all? Userspace should >> not be sending 0 if the target did not send >MaxRecvDSL. It looks like it >> should be sending 8k for that case. It looks like there is a bug in the >> tools where it will pass 0 >if the target sent 0. > >> It seems other drivers would be hitting this bug too and we should fix >> everyone. > > This was an IET target that did not send any value and we were > defaulting to 64K > > Here is a small userspace fix that should go along > > diff --git a/usr/be2iscsi.c b/usr/be2iscsi.c index ce8b719..ba4c29f > 100644 > --- a/usr/be2iscsi.c > +++ b/usr/be2iscsi.c > @@ -33,10 +33,6 @@ void be2iscsi_create_conn(struct iscsi_conn *conn) > if (conn->max_xmit_dlength > 65536) > conn->max_xmit_dlength = 65536; > > - if (!conn_rec->iscsi.MaxXmitDataSegmentLength || > - conn_rec->iscsi.MaxXmitDataSegmentLength > 65536) > - conn_rec->iscsi.MaxXmitDataSegmentLength = 65536; > - > session->erl = 0; > session->initial_r2t_en = 1; > } >For the case you are trying to fix are you getting 0 or 64K from userspace? >In the kernel code you changed to set the <value to 8K it looks like you got 0 >from userspace. Is that right? Before this fix, in the usespace we will hit (!conn_rec->iscsi.MaxXmitDataSegmentLength) and will set conn_rec->iscsi.MaxXmitDataSegmentLength = 65536. The userspace patch would prevent that from happening. So, in the Kernel space we would get 0 and which would be set to 8K. >Are you setting node.conn[0].iscsi.MaxXmitDataSegmentLength in iscsif.conf to >64K or did you leave it as the default? >Just to make sure we are on the same >page I really mean did you set it in iscsid.conf or with iscsiadm. I am not >talking >about setting above in the be2iscsi create_conn callout. No changes were made. It was default values. >If it is the default of 0, then I think when the code above is called >iscsi_copy_operational_params will have set conn->>max_xmit_dlength to >ISCSI_DEF_MAX_RECV_SEG_LEN (8k). The conn->max_xmit_dlength value is the one >we use for >final negotiated value so that is what gets passed to the kernel. >If the target does not negotiate for MRDSL then it should stay 8k. >iscsi_session_set_params will then pass down conn->max_xmit_dlength when login >is done. >If I am looking at the code right, the only way we can get 0 in the kernel is >if the target sends 0 for MRDSL. iscsid was not >expecting that and will just >set conn->max_xmit_dlength to 0 and that will get passed down to all drivers >incorrectly. If I >am right then we need to do a fix for all drivers. As an experiment, we tried applying just the userspace patch I sent earlier and we got the max_xmit_dlength as 8K. So, the basic issue was that the user space code in usr/be2iscsi was setting the conn->max_xmit_dlength to 64K when we hit (!conn_rec->iscsi.MaxXmitDataSegmentLength) I will send out a separate patch for the userspace code, but feel we can still keep the driver changes since the current code in Kernel sets conn->max_xmit_dlength=64K if it cam down to driver as zero. I would like to keep the check in driver as the user may apply different version of the userspace code (Distro version, Top of your Tree). -- 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
