On 09/16/2013 11:28 PM, Jayamohan Kallickal wrote: > > > -----Original Message----- > From: Mike Christie [mailto:michaelc@xxxxxxxxxxx] > Sent: Monday, September 16, 2013 7:59 PM > To: Jayamohan Kallickal > Cc: jbottomley@xxxxxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx; 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.john-n@xxxxxxxxxx> >> Signed-off-by: Jayamohan Kallickal <jayamohan.kallickal@xxxxxxxxxx> >> --- >> 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? 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. 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. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
