On Mon, 2012-03-26 at 00:07 -0700, Bhanu Prakash Gollapudi wrote:
> On 3/22/2012 12:23 PM, Bhanu Prakash Gollapudi wrote:
> > On 3/22/2012 10:38 AM, Vasu Dev wrote:
> >> On Thu, 2012-03-22 at 11:03 -0700, Bhanu Prakash Gollapudi wrote:
> >>> On 2/14/2012 5:34 PM, Vasu Dev wrote:
> >>>> Currently fc_host mfs is not getting updated in
> >>>> case its changed during FLOGI and that leaves fc_host
> >>>> to show its initial old value in sysfs, so instead
> >>>> have fc_host mfs updated along with updating lport mfs
> >>>> during FLOGI.
> >>>>
> >>>> Also in case of bad mfs during flogi, error out
> >>>> instead of continuing with flogi.
> >>>>
> >>>> Signed-off-by: Vasu Dev<[email protected]>
> >>>> Tested-by: Ross Brattain<[email protected]>
> >>>> ---
> >>>>
> >>>> drivers/scsi/libfc/fc_lport.c | 9 ++++++++-
> >>>> 1 files changed, 8 insertions(+), 1 deletions(-)
> >>>>
> >>>> diff --git a/drivers/scsi/libfc/fc_lport.c
> >>>> b/drivers/scsi/libfc/fc_lport.c
> >>>> index 9a0b2a9..a992692 100644
> >>>> --- a/drivers/scsi/libfc/fc_lport.c
> >>>> +++ b/drivers/scsi/libfc/fc_lport.c
> >>>> @@ -1743,8 +1743,15 @@ void fc_lport_flogi_resp(struct fc_seq *sp,
> >>>> struct fc_frame *fp,
> >>>> mfs = ntohs(flp->fl_csp.sp_bb_data)&
> >>>> FC_SP_BB_DATA_MASK;
> >>>> if (mfs>= FC_SP_MIN_MAX_PAYLOAD&&
> >>>> - mfs< lport->mfs)
> >>>> + mfs< lport->mfs) {
> >>>> lport->mfs = mfs;
> >>>> + fc_host_maxframe_size(lport->host) = mfs;
> >>>> + } else {
> >>>> + FC_LPORT_DBG(lport, "FLOGI bad mfs:%hu response\n",
> >>>> mfs);
> >>>> + fc_lport_error(lport, fp);
> >>>> + goto err;
> >>>> + }
> >>>> +
> >>>
> >>> Vasu, sorry for not getting back to you on this earlier, but I found
> >>> that this check does not allow the FLOGI to succeed on Broadcom devices.
> >>> We have max frame size set to 2048 even if FLOGI resp indicates 2112.
> >>
> >> Failure with 2112 was due to existing wrong check prio to this check as
> >> mfs< lport->mfs and that triggered after else condition was added by
> >> this patch. The final updated patch fixed to allow flogi response with
> >> 2112 also by modifying check as mfs<= lport->mfs and that is the one
> >> applied to the tree by Rob at and that should work:
> >>
> >> http://www.open-fcoe.org/git/?p=fcoe/fcoe-next.git;a=commitdiff;h=4394b32e1728196407af14b4527abaada1b938c2;hp=355dff86cf1c0a7f054c17bf1f9dfed69ec5625e
> >
> > I saw the failure with latest code (with<= change).
> >
> >>
> >>
> >>>
> >>> Even for fcoe driver, with default MTU of 1500, FLOGI is not going to
> >>> succeed. Probably if we want to retry, we should check only for
> >>> MIN_MAX_PAYLOAD.
> >>
> >> Modified check ensure response not to exceed n-port mfs and that should
> >> work for this case also.
> >
> > During FLOGI we advertise our mfs (Receive data field size). But switch
> > advertised mfs in FLOGI ACC need not match with what we have. It
> > advertises what it supports. In our case, we advertise mfs as
> > 2K(0x800), and in FLOGI ACC switch sends 2112 (0x840). The check above
> > fails because of this, but this doesn't mean a bad response.
> >
> > Thanks,
> > Bhanu
>
> Vasu, This patch should do the checks you intended, and yet works with
> drivers that can work with mfs lower than the switch advertised value.
>
> diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
> index 4f7ef76..5ec556e 100644
> --- a/drivers/scsi/libfc/fc_lport.c
> +++ b/drivers/scsi/libfc/fc_lport.c
> @@ -1742,17 +1742,19 @@ void fc_lport_flogi_resp(struct fc_seq *sp,
> struct fc_fr
>
> mfs = ntohs(flp->fl_csp.sp_bb_data) &
> FC_SP_BB_DATA_MASK;
> - if (mfs >= FC_SP_MIN_MAX_PAYLOAD &&
> - mfs <= lport->mfs) {
> - lport->mfs = mfs;
> - fc_host_maxframe_size(lport->host) = mfs;
> - } else {
> +
> + if (mfs < FC_SP_MIN_MAX_PAYLOAD && mfs > FC_SP_MAX_MAX_PAYLOAD) {
This check comply to FC-FS-03 sec 15.6.2.6 for "The value shall be a
multiple of four bytes. Values less than 256 or greater than 2 112 are
invalid. An Fx_Port shall support a Data Field size of at least 256
bytes.", so its good though its kind of odd for having MFS 2112 in
response while N_PORT indicated 2048 in your case. I'd expect protocol
to proceed with minimum of two.
> FC_LPORT_DBG(lport, "FLOGI bad mfs:%hu response, "
> "lport->mfs:%hu\n", mfs, lport->mfs);
> fc_lport_error(lport, fp);
> goto err;
> }
>
> + if (mfs <= lport->mfs) {
> + lport->mfs = mfs;
> + fc_host_maxframe_size(lport->host) = mfs;
> + }
Yeap, this would update mfs in case switch sends 2048 in response to
2112 n_port mfs and that is what I wanted.
> +
> csp_flags = ntohs(flp->fl_csp.sp_features);
> r_a_tov = ntohl(flp->fl_csp.sp_r_a_tov);
> e_d_tov = ntohl(flp->fl_csp.sp_e_d_tov);
>
> Let me know if you have any comments. I'll send a formal patch if this
> is fine with you.
>
Good to go and sorry for late response.
Thanks
Vasu
> Thanks,
> Bhanu
>
> >
> >>
> >> Thanks
> >> Vasu
> >>
> >>
> >>
> >
> >
> > _______________________________________________
> > devel mailing list
> > [email protected]
> > https://lists.open-fcoe.org/mailman/listinfo/devel
> >
>
>
> _______________________________________________
> devel mailing list
> [email protected]
> https://lists.open-fcoe.org/mailman/listinfo/devel
_______________________________________________
devel mailing list
[email protected]
https://lists.open-fcoe.org/mailman/listinfo/devel
