Re: [PATCH v2 01/20] lpfc: Fix plogi collision that causes illegal state transition

[Johannes Thumshirn <jthumshirn@suse.de>]

On Fri, Aug 04, 2017 at 05:47:12PM -0700, James Smart wrote:
> From: Dick Kennedy <dick.kennedy@broadcom.com>
> 
> Message "0271 Illegal State Transition: node" seen in logs, all
> luns are unuseable for that target.
> 
> A window exists in the rcv_plogi path where if the state is plogi
> issue but the driver has not issued a plogi, then two reglogins will
> be sent for the same RPI. The first one to complete will advance
> the state to prli issue the second one will be detected as an
> illegal state, and leave the node in an unusable state.
> 
> Correct the completion routine for the PLOGI ACC that detects the
> state change when the driver starts discovery on the node again
> and drop the REGLOGIN mailbox command.
> 
> Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
> Signed-off-by: James Smart <james.smart@broadcom.com>
> ---
>  drivers/scsi/lpfc/lpfc_els.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
> index 6d1d6f691df4..9e9aa8bb7001 100644
> --- a/drivers/scsi/lpfc/lpfc_els.c
> +++ b/drivers/scsi/lpfc/lpfc_els.c
> @@ -3930,7 +3930,31 @@ lpfc_cmpl_els_rsp(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
>  	if (mbox) {
>  		if ((rspiocb->iocb.ulpStatus == 0)
>  		    && (ndlp->nlp_flag & NLP_ACC_REGLOGIN)) {
> -			lpfc_unreg_rpi(vport, ndlp);
> +			if (!lpfc_unreg_rpi(vport, ndlp)) {
> +				if (ndlp->nlp_state ==  NLP_STE_PLOGI_ISSUE ||
> +				    ndlp->nlp_state ==
> +				     NLP_STE_REG_LOGIN_ISSUE) {


			if (!lpfc_unreg_rpi(vport, ndlp) &&
			    (ndlp->nlp_state == NLP_STE_PLOGI_ISSUE ||
			     ndlp->nlp_state == NLP_STE_REG_LOGIN_ISSUE)) {

This saves you one level of indention and given that we already have two
before we reach the 'if' it's highly desirable.

> +					lpfc_printf_vlog(vport, KERN_INFO,
> +							 LOG_DISCOVERY,
> +							 "0314 PLOGI recov "
> +							 "DID x%x "
> +							 "Data: x%x x%x x%x\n",
> +							 ndlp->nlp_DID,
> +							 ndlp->nlp_state,
> +							 ndlp->nlp_rpi,
> +							 ndlp->nlp_flag);
> +					mp = (struct lpfc_dmabuf *)
> +					      mbox->context1;

No need for a cast.

Otherwise,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850