libfc, libfcoe, fcoe updates for 3.10-rc
Subsystems (2)
cover libfc, libfcoe, fcoe updates for 3.10-rc
The following series implements a few random fixes.
---
Krishna Mohan (1):
libfcoe: Fix Conflicting FCFs issue in the fabric
Mark Rustad (1):
libfc: Correct check for initiator role
Neil Horman (2):
libfc: extend ex_lock to protect all of fc_seq_send
MAINTAINERS: Fix fcoe mailing list
MAINTAINERS | 2 +- drivers/scsi/fcoe/fcoe_ctlr.c | 15 +++++---------- drivers/scsi/libfc/fc_exch.c | 37 ++++++++++++++++++++++++------------- drivers/scsi/libfc/fc_rport.c | 2 +- 4 files changed, 31 insertions(+), 25 deletions(-)
Signature · 1 line
Thanks, //Rob
PATCH 1/4 libfcoe: Fix Conflicting FCFs issue in the fabric
From: Krishna Mohan <[email protected]>
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.
--- drivers/scsi/fcoe/fcoe_ctlr.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
drivers/scsi/fcoe/fcoe_ctlr.c +5 -10
@@ -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) {
PATCH 2/4 libfc: Correct check for initiator role
In reply to Robert LoveFrom: Mark Rustad <[email protected]>
The service_params field is being checked against the symbol FC_RPORT_ROLE_FCP_INITIATOR where it really should be checked against FCP_SPPF_INIT_FCN.
--- drivers/scsi/libfc/fc_rport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/libfc/fc_rport.c +1 -1
@@ -1962,7 +1962,7 @@ static int fc_rport_fcp_prli(struct fc_rport_priv *rdata, u32 spp_len, rdata->flags |=FC_RP_FLAGS_RETRY ; rdata->supported_classes =FC_COS_CLASS3 ; -if (!(lport->service_params &FC_RPORT_ROLE_FCP_INITIATOR )) +if (!(lport->service_params &FCP_SPPF_INIT_FCN ))return 0 ; spp->spp_flags |= rspp->spp_flags &FC_SPP_EST_IMG_PAIR ;
PATCH 3/4 libfc: extend ex_lock to protect all of fc_seq_send
In reply to Robert LoveFrom: Neil Horman <[email protected]>
This warning was reported recently:
WARNING: at drivers/scsi/libfc/fc_exch.c:478 fc_seq_send+0x14f/0x160 [libfc]() (Not tainted) Hardware name: ProLiant DL120 G7 Modules linked in: tcm_fc target_core_iblock target_core_file target_core_pscsi
target_core_mod configfs dm_round_robin dm_multipath 8021q garp stp llc bnx2fc cnic uio fcoe libfcoe libfc scsi_transport_fc scsi_tgt autofs4 sunrpc pcc_cpufreq ipv6 hpilo hpwdt e1000e microcode iTCO_wdt iTCO_vendor_support serio_raw shpchp ixgbe dca mdio sg ext4 mbcache jbd2 sd_mod crc_t10dif pata_acpi ata_generic ata_piix hpsa dm_mirror dm_region_hash dm_log dm_mod [last unloaded:scsi_wait_scan]
Pid: 5464, comm: target_completi Not tainted 2.6.32-272.el6.x86_64 #1
Call Trace: [<ffffffff8106b747>] ? warn_slowpath_common+0x87/0xc0 [<ffffffff8106b79a>] ? warn_slowpath_null+0x1a/0x20 [<ffffffffa025f7df>] ? fc_seq_send+0x14f/0x160 [libfc] [<ffffffffa035cbce>] ? ft_queue_status+0x16e/0x210 [tcm_fc] [<ffffffffa030a660>] ? target_complete_ok_work+0x0/0x4b0 [target_core_mod] [<ffffffffa030a766>] ? target_complete_ok_work+0x106/0x4b0 [target_core_mod] [<ffffffffa030a660>] ? target_complete_ok_work+0x0/0x4b0 [target_core_mod] [<ffffffff8108c760>] ? worker_thread+0x170/0x2a0 [<ffffffff810920d0>] ? autoremove_wake_function+0x0/0x40 [<ffffffff8108c5f0>] ? worker_thread+0x0/0x2a0 [<ffffffff81091d66>] ? kthread+0x96/0xa0 [<ffffffff8100c14a>] ? child_rip+0xa/0x20 [<ffffffff81091cd0>] ? kthread+0x0/0xa0 [<ffffffff8100c140>] ? child_rip+0x0/0x20
It occurs because fc_seq_send can have multiple contexts executing within it at the same time, and fc_seq_send doesn't consistently use the ep->ex_lock that protects this structure. Because of that, its possible for one context to clear the INIT bit in the ep->esb_state field while another checks it, leading to the above stack trace generated by the WARN_ON in the function.
We should probably undertake the effort to convert access to the fc_exch structures to use rcu, but that a larger work item. To just fix this specific issue, we can just extend the ex_lock protection through the entire fc_seq_send path
--- drivers/scsi/libfc/fc_exch.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-)
drivers/scsi/libfc/fc_exch.c +24 -13
@@ -463,13 +463,7 @@ static void fc_exch_delete(struct fc_exch *ep)fc_exch_release (ep);/* drop hold for exch in mp */ } -/** -* fc_seq_send() - Send a frame using existing sequence/exchange pair -* @lport: The local port that the exchange will be sent on -* @sp: The sequence to be sent -* @fp: The frame to be sent on the exchange -*/ -static int fc_seq_send (struct fc_lport *lport,struct fc_seq *sp, +static int fc_seq_send_locked (struct fc_lport *lport,struct fc_seq *sp,struct fc_frame *fp) {struct fc_exch *ep;
@@ -479,7 +473,7 @@ static int fc_seq_send(struct fc_lport *lport, struct fc_seq *sp,u8 fh_type = fh->fh_type; ep =fc_seq_exch (sp); -WARN_ON ((ep->esb_stat &ESB_ST_SEQ_INIT ) !=ESB_ST_SEQ_INIT ); +WARN_ON (!(ep->esb_stat &ESB_ST_SEQ_INIT )); f_ctl =ntoh24 (fh->fh_f_ctl);fc_exch_setup_hdr (ep, fp, f_ctl);
@@ -502,17 +496,34 @@ static int fc_seq_send(struct fc_lport *lport, struct fc_seq *sp, error = lport->tt.frame_send (lport, fp);if (fh_type ==FC_TYPE_BLS ) -return error; + goto out;/* * Update the exchange and sequence flags, * assuming all frames for the sequence have been sent. * We can only be called to send once for each sequence. */ -spin_lock_bh (&ep->ex_lock); ep->f_ctl = f_ctl & ~FC_FC_FIRST_SEQ ;/* not first seq */ if (f_ctl &FC_FC_SEQ_INIT ) ep->esb_stat &= ~ESB_ST_SEQ_INIT ; +out: +return error; +} + +/** +* fc_seq_send() - Send a frame using existing sequence/exchange pair +* @lport: The local port that the exchange will be sent on +* @sp: The sequence to be sent +* @fp: The frame to be sent on the exchange +*/ +static int fc_seq_send (struct fc_lport *lport,struct fc_seq *sp, +struct fc_frame *fp) +{ +struct fc_exch *ep; +int error; + ep =fc_seq_exch (sp); +spin_lock_bh (&ep->ex_lock); + error =fc_seq_send_locked (lport, sp, fp);spin_unlock_bh (&ep->ex_lock);return error; }
@@ -629,7 +640,7 @@ static int fc_exch_abort_locked(struct fc_exch *ep,if (fp) {fc_fill_fc_hdr (fp,FC_RCTL_BA_ABTS , ep->did, ep->sid,FC_TYPE_BLS ,FC_FC_END_SEQ |FC_FC_SEQ_INIT ,0 ); - error =fc_seq_send (ep->lp, sp, fp); + error =fc_seq_send_locked (ep->lp, sp, fp); }else error = -ENOBUFS ;return error;
@@ -1132,7 +1143,7 @@ static void fc_seq_send_last(struct fc_seq *sp, struct fc_frame *fp, f_ctl =FC_FC_LAST_SEQ |FC_FC_END_SEQ |FC_FC_SEQ_INIT ; f_ctl |= ep->f_ctl;fc_fill_fc_hdr (fp, rctl, ep->did, ep->sid, fh_type, f_ctl,0 ); -fc_seq_send (ep->lp, sp, fp); +fc_seq_send_locked (ep->lp, sp, fp); }/**
@@ -1307,8 +1318,8 @@ static void fc_exch_recv_abts(struct fc_exch *ep, struct fc_frame *rx_fp) ap->ba_low_seq_cnt =htons (sp->cnt); } sp =fc_seq_start_next_locked (sp); -spin_unlock_bh (&ep->ex_lock);fc_seq_send_last (sp, fp,FC_RCTL_BA_ACC ,FC_TYPE_BLS ); +spin_unlock_bh (&ep->ex_lock);fc_frame_free (rx_fp);return ;
PATCH 4/4 MAINTAINERS: Fix fcoe mailing list
In reply to Robert LoveFrom: Neil Horman <[email protected]>
The FCoE mailing list has moved, updte it in the MAINTAINERS file
--- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
MAINTAINERS +1 -1
@@ -3165,7 +3165,7 @@ F: lib/fault-inject.c FCOE SUBSYSTEM (libfc, libfcoe, fcoe) M: Robert Love <[email protected]> -L: [email protected] +L: [email protected] W: www.Open-FCoE.org S: Supported F: drivers/scsi/libfc/