Patchwork [Open-FCoE] libfcoe: Fix Conflicting FCFs issue in the fabric

login
register
mail settings
Submitter Krishna Mohan
Date March 1, 2013, 3:35 a.m.
Message ID <1362108931-18863-1-git-send-email-krmohan@cisco.com>
Download mbox | patch
Permalink /patch/1/
State Accepted
Headers show

Comments

Krishna Mohan - March 1, 2013, 3:35 a.m.
When multiple FCFs in use, and first FIP Advertisement received is
with "Available for Login" i.e A bit set to 0, FCF selection will fail.
The fix is to remove the assumption in the code that first FCF is only
allowed selectable FCF.
Consider the scenario fip->fcfs contains FCF1(fabricname X, marked A=0)
FCF2(fabricname Y, marked A=1). list_first_entry(first) points to FCF1
and 1st iteration we ignore the FCF and on 2nd iteration we compare
FCF1 & FCF2 fabric name and we fails to perform FCF selection.

Signed-off-by: Krishna Mohan <krmohan@cisco.com>
---
 drivers/scsi/fcoe/fcoe_ctlr.c |   15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)
Bhanu Prakash Gollapudi - March 16, 2013, 3:35 a.m.
On 02/28/2013 07:35 PM, Krishna Mohan wrote:
> When multiple FCFs in use, and first FIP Advertisement received is
> with "Available for Login" i.e A bit set to 0, FCF selection will fail.
> The fix is to remove the assumption in the code that first FCF is only
> allowed selectable FCF.
> Consider the scenario fip->fcfs contains FCF1(fabricname X, marked A=0)
> FCF2(fabricname Y, marked A=1). list_first_entry(first) points to FCF1
> and 1st iteration we ignore the FCF and on 2nd iteration we compare
> FCF1 & FCF2 fabric name and we fails to perform FCF selection.
>
> Signed-off-by: Krishna Mohan <krmohan@cisco.com>
> ---
>   drivers/scsi/fcoe/fcoe_ctlr.c |   15 +++++----------
>   1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
> index 08c3bc3..ec35b0c 100644
> --- a/drivers/scsi/fcoe/fcoe_ctlr.c
> +++ b/drivers/scsi/fcoe/fcoe_ctlr.c
> @@ -1548,9 +1548,6 @@ static struct fcoe_fcf *fcoe_ctlr_select(struct fcoe_ctlr *fip)
>   {
>   	struct fcoe_fcf *fcf;
>   	struct fcoe_fcf *best = fip->sel_fcf;
> -	struct fcoe_fcf *first;
> -
> -	first = list_first_entry(&fip->fcfs, struct fcoe_fcf, list);
>   
>   	list_for_each_entry(fcf, &fip->fcfs, list) {
>   		LIBFCOE_FIP_DBG(fip, "consider FCF fab %16.16llx "
> @@ -1568,17 +1565,15 @@ static struct fcoe_fcf *fcoe_ctlr_select(struct fcoe_ctlr *fip)
>   					"" : "un");
>   			continue;
>   		}
> -		if (fcf->fabric_name != first->fabric_name ||
> -		    fcf->vfid != first->vfid ||
> -		    fcf->fc_map != first->fc_map) {
> +		if (!best || fcf->pri < best->pri || best->flogi_sent)
> +			best = fcf;
> +		if (fcf->fabric_name != best->fabric_name ||
> +		    fcf->vfid != best->vfid ||
> +		    fcf->fc_map != best->fc_map) {
>   			LIBFCOE_FIP_DBG(fip, "Conflicting fabric, VFID, "
>   					"or FC-MAP\n");
>   			return NULL;
>   		}
> -		if (fcf->flogi_sent)
> -			continue;
> -		if (!best || fcf->pri < best->pri || best->flogi_sent)
> -			best = fcf;
>   	}
>   	fip->sel_fcf = best;
>   	if (best) {
Changes look good to me.

Reviewed-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com>

Patch

diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 08c3bc3..ec35b0c 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -1548,9 +1548,6 @@  static struct fcoe_fcf *fcoe_ctlr_select(struct fcoe_ctlr *fip)
 {
 	struct fcoe_fcf *fcf;
 	struct fcoe_fcf *best = fip->sel_fcf;
-	struct fcoe_fcf *first;
-
-	first = list_first_entry(&fip->fcfs, struct fcoe_fcf, list);
 
 	list_for_each_entry(fcf, &fip->fcfs, list) {
 		LIBFCOE_FIP_DBG(fip, "consider FCF fab %16.16llx "
@@ -1568,17 +1565,15 @@  static struct fcoe_fcf *fcoe_ctlr_select(struct fcoe_ctlr *fip)
 					"" : "un");
 			continue;
 		}
-		if (fcf->fabric_name != first->fabric_name ||
-		    fcf->vfid != first->vfid ||
-		    fcf->fc_map != first->fc_map) {
+		if (!best || fcf->pri < best->pri || best->flogi_sent)
+			best = fcf;
+		if (fcf->fabric_name != best->fabric_name ||
+		    fcf->vfid != best->vfid ||
+		    fcf->fc_map != best->fc_map) {
 			LIBFCOE_FIP_DBG(fip, "Conflicting fabric, VFID, "
 					"or FC-MAP\n");
 			return NULL;
 		}
-		if (fcf->flogi_sent)
-			continue;
-		if (!best || fcf->pri < best->pri || best->flogi_sent)
-			best = fcf;
 	}
 	fip->sel_fcf = best;
 	if (best) {