RE: [PATCH V1 00/17] Update the driver version to 3.2.21.1
-Original Message- From: Vijay Mohan Guvva Sent: Monday, May 13, 2013 3:03 PM To: linux-scsi@vger.kernel.org Cc: jbottom...@parallels.com; Adapter Linux Open SRC Team; Vijay Mohan Guvva Subject: [PATCH V1 00/17] Update the driver version to 3.2.21.1 Hi James, Re-submitting the patch sets, Update the driver version to 3.2.21.0 and Update the driver version to 3.2.21.1. It includes bug-fixes, changes to use firmware version 3.2.1.0, adds new features such as Forward Error Correction, FC BB credit recovery, dynamic diagnostic Port (D-port), enabled support for new hardware - chinook quad and updates the Brocade BFA driver to v3.2.21.1. Thanks, Vijaya Mohan Guvva Vijaya Mohan Guvva (17): bfa: Support for FC BB credit recovery bfa: Forward Error Correction status query bfa: Add dynamic diagnostic port support bfa: Fix WARN_ON condition check bfa: FDMI enhancements bfa: Fix 1860 port initialize when ATC is enabled bfa: Fix FDISC timeout handling bfa: kdump fix on 815 and 825 adapters bfa: driver compatibility with 32bit libs bfa: fru vpd date update changes bfa: firmware statistics update bfa: Allow rsp queue process during ioc disable bfa: Fix bug_on condition in RPSC rsp handling bfa: fix endianess issue for firmware stats bfa: Support for chinook-quad port card bfa: dis-associate bfa path_tov with dev_loss_tmo bfa: Update the driver version to 3.2.21.1 drivers/scsi/bfa/bfa_core.c | 3 +- drivers/scsi/bfa/bfa_defs.h | 103 +- drivers/scsi/bfa/bfa_defs_svc.h | 77 - drivers/scsi/bfa/bfa_fc.h| 15 + drivers/scsi/bfa/bfa_fcpim.c | 2 +- drivers/scsi/bfa/bfa_fcs.c | 62 +--- drivers/scsi/bfa/bfa_fcs.h | 34 +- drivers/scsi/bfa/bfa_fcs_lport.c | 209 +++- drivers/scsi/bfa/bfa_fcs_rport.c | 7 +- drivers/scsi/bfa/bfa_ioc.c | 74 +++-- drivers/scsi/bfa/bfa_ioc.h | 9 +- drivers/scsi/bfa/bfa_ioc_cb.c| 86 - drivers/scsi/bfa/bfa_ioc_ct.c| 46 +++ drivers/scsi/bfa/bfa_svc.c | 698 --- drivers/scsi/bfa/bfa_svc.h | 34 +- drivers/scsi/bfa/bfad.c | 14 +- drivers/scsi/bfa/bfad_attr.c | 33 +- drivers/scsi/bfa/bfad_bsg.c | 137 +--- drivers/scsi/bfa/bfad_bsg.h | 52 ++- drivers/scsi/bfa/bfad_drv.h | 2 +- drivers/scsi/bfa/bfi.h | 78 - drivers/scsi/bfa/bfi_ms.h| 5 +- 22 files changed, 1498 insertions(+), 282 deletions(-) -- 1.7.12 The patch you're replying to didn't make the list, which is also a necessary precursor to getting stuff into the tree. Hi James, I have resubmitted the patch set and verified them in MARC. Please pull the set if you don't have any comments. Thanks, Vijay -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/1] bfa: Fixes for 0-terminated strncpy and possible null pointer dereference
-Original Message-
From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
ow...@vger.kernel.org] On Behalf Of Jakob Normark
Sent: Thursday, May 16, 2013 1:12 AM
To: Anil Gurumurthy; Vijay Mohan Guvva; James E.J. Bottomley
Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org; Jakob Normark
Subject: [PATCH 1/1] bfa: Fixes for 0-terminated strncpy and possible null
pointer dereference
This patch fixes two cppcheck errors in drivers/scsi/bfa/bfad_im.c Kernel
version: v3.10-rc1
Signed-off-by: Jakob Normark jakobnorm...@gmail.com
---
drivers/scsi/bfa/bfad_im.c |9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/bfa/bfad_im.c b/drivers/scsi/bfa/bfad_im.c index
5864f98..9489c56 100644
--- a/drivers/scsi/bfa/bfad_im.c
+++ b/drivers/scsi/bfa/bfad_im.c
@@ -944,13 +944,15 @@ static int
bfad_im_slave_alloc(struct scsi_device *sdev) {
struct fc_rport *rport = starget_to_rport(scsi_target(sdev));
- struct bfad_itnim_data_s *itnim_data =
- (struct bfad_itnim_data_s *) rport-dd_data;
- struct bfa_s *bfa = itnim_data-itnim-bfa_itnim-bfa;
+ struct bfad_itnim_data_s *itnim_data;
+ struct bfa_s *bfa;
if (!rport || fc_remote_port_chkready(rport))
return -ENXIO;
+ itnim_data = (struct bfad_itnim_data_s *) rport-dd_data;
+ bfa = itnim_data-itnim-bfa_itnim-bfa;
+
if (bfa_get_lun_mask_status(bfa) == BFA_LUNMASK_ENABLED) {
/*
* We should not mask LUN 0 - since this will translate @@ -
1037,6 +1039,7 @@ bfad_fc_host_init(struct bfad_im_port_s *im_port)
strncpy(symname, bfad-
bfa_fcs.fabric.bport.port_cfg.sym_name.symname,
BFA_SYMNAME_MAXLEN);
+ symname[BFA_SYMNAME_MAXLEN - 1] = '\0';
sprintf(fc_host_symbolic_name(host), %s, symname);
fc_host_supported_speeds(host) =
bfad_im_supported_speeds(bfad-bfa);
--
1.7.9.5
Changes looks good. Thanks for the patch.
Acked-by: Vijay Mohan Guvva vmo...@brocade.com
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V1] bfa: fix faulty handling of events in lps sm
Resending the patch as it didn't make the link-scsi list.
When a switch disable/enable or a reboot is done, the HBA port gets an
offline and a subsequent online notification. When the port comes up a
link up notification is sent to bfa from the firmware. The bfa then send
an FLOGI to the firmware which is sent out on the wire.
The switch port meanwhile goes offline (presumably for diagnostics)
which causes the switch not to respond to the FLOGI.
The link down notification is sent to the HBA driver. However owing to a
bug in the lps state machine handling the lps state machine does not
move to sm_init state (it remains in sm_login state and send a login
complete message to fcs). This results in a zero PID assignment as the
login is not really complete.
This fix is to correctly handle the events in lps state machine.
Signed-off-by: Vijaya Mohan Guvva vmo...@brocade.com
---
drivers/scsi/bfa/bfa_svc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/bfa/bfa_svc.c b/drivers/scsi/bfa/bfa_svc.c
index 299c1c8..7ed2c57 100644
--- a/drivers/scsi/bfa/bfa_svc.c
+++ b/drivers/scsi/bfa/bfa_svc.c
@@ -1276,7 +1276,6 @@ bfa_lps_sm_login(struct bfa_lps_s *lps, enum
bfa_lps_event event)
switch (event) {
case BFA_LPS_SM_FWRSP:
- case BFA_LPS_SM_OFFLINE:
if (lps-status == BFA_STATUS_OK) {
bfa_sm_set_state(lps, bfa_lps_sm_online);
if (lps-fdisc)
@@ -1305,6 +1304,7 @@ bfa_lps_sm_login(struct bfa_lps_s *lps, enum
bfa_lps_event event)
bfa_lps_login_comp(lps);
break;
+ case BFA_LPS_SM_OFFLINE:
case BFA_LPS_SM_DELETE:
bfa_sm_set_state(lps, bfa_lps_sm_init);
break;
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] bfa: fix for FC Direct Attach LUN discovery failure
Resending the patch as it didn't make the linux-scsi list. This patch fixes fcs rport state machine to address ocassional Brocade FC Direct Attach LUN discovery failure due to not sending PLOGI accept to the target. Signed-off-by: Vijaya Mohan Guvva vmo...@brocade.com --- drivers/scsi/bfa/bfa_fcs_rport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/bfa/bfa_fcs_rport.c b/drivers/scsi/bfa/bfa_fcs_rport.c index 58ac643..610ca95 100644 --- a/drivers/scsi/bfa/bfa_fcs_rport.c +++ b/drivers/scsi/bfa/bfa_fcs_rport.c @@ -189,8 +189,8 @@ bfa_fcs_rport_sm_uninit(struct bfa_fcs_rport_s *rport, enum rport_event event) break; case RPSM_EVENT_PLOGI_RCVD: - bfa_sm_set_state(rport, bfa_fcs_rport_sm_fc4_fcs_online); - bfa_fcs_rport_fcs_online_action(rport); + bfa_sm_set_state(rport, bfa_fcs_rport_sm_plogiacc_sending); + bfa_fcs_rport_send_plogiacc(rport, NULL); break; case RPSM_EVENT_PLOGI_COMP: -- 1.8.2.1 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [patch] [SCSI] bfa: Use GFP_ATOMIC under spin_lock
-Original Message-
From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
Sent: Friday, March 08, 2013 4:01 AM
To: Anil Gurumurthy
Cc: Vijay Mohan Guvva; James E.J. Bottomley; linux-scsi@vger.kernel.org;
linux-ker...@vger.kernel.org; kernel-janit...@vger.kernel.org
Subject: [patch] [SCSI] bfa: Use GFP_ATOMIC under spin_lock
This is always called with spinlocks held so it should use GFP_ATOMIC. The
call tree is:
- bfad_drv_start()
Takes spin_lock_irqsave(bfad-bfad_lock, flags);
- bfa_fcs_pbc_vport_init()
- bfa_fcb_pbc_vport_create()
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c index
a5f7690..d144a06 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -491,7 +491,7 @@ bfa_fcb_pbc_vport_create(struct bfad_s *bfad,
struct bfi_pbc_vport_s pbc_vport)
struct bfad_vport_s *vport;
int rc;
- vport = kzalloc(sizeof(struct bfad_vport_s), GFP_KERNEL);
+ vport = kzalloc(sizeof(struct bfad_vport_s), GFP_ATOMIC);
if (!vport) {
bfa_trc(bfad, 0);
return;
Changes looks good. Thanks for the patch.
Acked-by: Vijay Mohan Guvva vmo...@brocade.com
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2][RFC] scsi_transport_fc: Implement I_T nexus reset
-Original Message-
From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
ow...@vger.kernel.org] On Behalf Of James Smart
Sent: Monday, March 11, 2013 11:04 AM
To: Hannes Reinecke
Cc: Jeremy Linton; Mike Christie; linux-scsi@vger.kernel.org; Andrew
Vasquez; Chad Dupuis; Robert Elliot; Smart, James
Subject: Re: [PATCH v2][RFC] scsi_transport_fc: Implement I_T nexus reset
On 3/11/2013 1:05 PM, Hannes Reinecke wrote:
On 03/07/2013 09:35 PM, Jeremy Linton wrote:
On 3/7/2013 2:20 PM, Mike Christie wrote:
On 03/07/2013 02:13 PM, Jeremy Linton wrote:
For lpfc, you never get to the code. Or rather when I was
testing it, I couldn't find any way to propagate an error beyond
the initial
lpfc_reset_flush_io_context() call in lpfc_device_reset_handler().
That call pretty much always returns success indpependent of
the remote device because the firmware acks the context clear
aborts, resulting in the outstanding iocb count being zero
(independent of both the mid layer status and the actual device
state).
Your lpfc patch fixes that right?
Yes. It allows the device reset to fail if the device doesn't
respond to the task mgmt request, or rejects it, etc.
It doesn't unjam the commands that get aborted by the
flush_io_context() call.
Those have to depend on their timeouts. That is another patch...
It's actually worse than that.
lpfc_terminate_rport_io() calls lpfc_sli_abort_iocb(), which has this:
if (lpfc_is_link_up(phba))
abtsiocb-iocb.ulpCommand = CMD_ABORT_XRI_CN;
else
abtsiocb-iocb.ulpCommand = CMD_CLOSE_XRI_CN;
/* Setup callback routine and issue the command. */
abtsiocb-iocb_cmpl = lpfc_sli_abort_fcp_cmpl;
ret_val = lpfc_sli_issue_iocb(phba, pring-ringno,
abtsiocb, 0);
if (ret_val == IOCB_ERROR) {
lpfc_sli_release_iocbq(phba, abtsiocb);
errcnt++;
continue;
}
Ie we're calling into firmware and waiting for an async event telling
us that the command has been aborted (ideally).
What I would like is some kind of synchronous call here, which would
guarantee us that we won't run into use-after-free issues.
Also 'lpfc_is_link_up' is clearly deficient here as the link itself
most likely is up, it's the I_T Nexus which is not.
James, is it safe to use 'CMD_CLOSE_XRI_CN' even when the link is up?
No, it's not safe. The ABORT, which sends an ABTS, is mandated so that the
other end and ourselves maintain proper (unique) exchange id
state. CLOSE sends no link traffic - but can only be used if the login
is broken (e.g. there's a different mechanism that communicated
termination of exchange states). I don't believe I can trust the logic
in the OS about frames laying in wait in the fabric (maybe sent earlier,
delayed at a switch, delivered after os thinks nexus is gone), so driver needs
to terminate them properly.
Which makes me wonder, how _exactly_ is I_T nexus reset supposed to
work? After all, we're trying to tell the target port that we cannot
talk to it anymore, right?
Which has some hurdles, conceptually ...
So from my POV I_T nexus reset can only be implemented on the
_initiator_ side, disregarding any target implementation.
(which would be pointless anyway).
Hmm. Probably have to ask T10 for clarification. Robert, any insights?
The I_T nexus reset should be a FC transport implicit logout call to the LLDD.
E.g. this becomes a transport-specific action on what it means to
break the I_T nexus, which for FC, is to terminate the login. This
logout call allows the driver to do all the implicit work to kill exchange
contexts and allows it to adjust the state of the target in
it's FC discovery engine. Question is - should the driver re-login ?
Typically, this would be driven by a RSCN, which I'm guessing for this
scenario, would not be occurring. If you knew it would, you could let
the driver respond to the RSCN and re-login later. If there's no RSCN,
then I would assume we put a heartbeat into the transport to retry login (to a
WWPN/WWNN basis - remembered from the I_T nexus reset) with the LLDD
- a new interface as well - call it establish I_T_nexus.
In lpfc's case - the Logout would allow the driver to take the CLOSE_XRI case,
giving you the speed/asynchronicity you desire. Reuse of scsi job structures
still can't occur until the driver returns then via the completion routines
(as
DMA related to them must be cancelled within the card by the ABORT/CLOSE
commands - even if we know there shouldn't be something to DMA).
-- james s
Cheers,
Hannes
Adding BROCADE BFA FC SCSI DRIVER maintainer Anil to the CC.
Thanks,
Vijay
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More
RE: [PATCH] bfa: Fix possible NULL pointer dereference magic no
-Original Message- From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- ow...@vger.kernel.org] On Behalf Of Syam Sidhardhan Sent: Sunday, February 24, 2013 3:08 PM To: linux-scsi@vger.kernel.org Cc: syamsidha...@gmail.com; Anil Gurumurthy; Vijay Mohan Guvva; jbottom...@parallels.com Subject: [PATCH] bfa: Fix possible NULL pointer dereference magic no Check for (port == NULL) has to be done before accessing port. Also fixes the magic number. Signed-off-by: Syam Sidhardhan s.s...@samsung.com --- drivers/scsi/bfa/bfa_fcs_lport.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/bfa/bfa_fcs_lport.c b/drivers/scsi/bfa/bfa_fcs_lport.c index 1224d04..69cf977 100644 --- a/drivers/scsi/bfa/bfa_fcs_lport.c +++ b/drivers/scsi/bfa/bfa_fcs_lport.c @@ -5615,11 +5615,13 @@ bfa_fcs_lport_get_rport_max_speed(bfa_fcs_lport_t *port) bfa_port_speed_t max_speed = 0; struct bfa_port_attr_s port_attr; bfa_port_speed_t port_speed, rport_speed; - bfa_boolean_t trl_enabled = bfa_fcport_is_ratelim(port-fcs-bfa); + bfa_boolean_t trl_enabled; if (port == NULL) - return 0; + return BFA_PORT_SPEED_UNKNOWN; + + trl_enabled = bfa_fcport_is_ratelim(port-fcs-bfa); fcs = port-fcs; Thanks for the patch Shyam. Acked-by: Vijay Mohan Guvva vmo...@brocade.com -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] bfa: avoid buffer overrun for 12-byte model name
Hi Jim,
Due to BFA_FCS_PORT_SYMBNAME_MODEL_SZ macro value of 12, we are missing some
part of the model name in port/node symbolic name and seeing issues related to
null termination. Mismatch between the actual model size and number of bytes
copied to symbolic name is a bug. Can you please fix this by changing
BFA_FCS_PORT_SYMBNAME_MODEL_SZ to 16 and reduce os_name macro
(BFA_FCS_PORT_SYMBNAME_OSINFO_SZ) to 44, so that both the issues i.e symbolic
name and null termination will be fixed.
Thanks,
Vijay
-Original Message-
From: linux-scsi-ow...@vger.kernel.org
[mailto:linux-scsi-ow...@vger.kernel.org] On Behalf Of Jim Meyering
Sent: Sunday, October 14, 2012 1:51 PM
To: Krishna Gudipati
Cc: Andi Kleen; linux-ker...@vger.kernel.org; James E.J. Bottomley;
linux-scsi@vger.kernel.org
Subject: Re: [PATCH] bfa: avoid buffer overrun for 12-byte model name
Jim Meyering wrote:
Jim Meyering wrote:
Krishna Gudipati wrote:
-Original Message-
From: Jim Meyering [mailto:j...@meyering.net]
Sent: Monday, August 20, 2012 9:55 AM
To: linux-ker...@vger.kernel.org
Cc: Jim Meyering; Jing Huang; Krishna Gudipati; James E.J.
Bottomley; linux- s...@vger.kernel.org
Subject: [PATCH] bfa: avoid buffer overrun for 12-byte model name
From: Jim Meyering meyer...@redhat.com
we use strncpy to copy a model name of length up to 15 (16, if you
count the NUL), into a buffer of size 12 (BFA_FCS_PORT_SYMBNAME_MODEL_SZ).
However, strncpy does not always NUL-terminate, so whenever the
original model string has strlen = 12, the following strncat reads
beyond end of the -
sym_name buffer as it attempts to find end of string.
bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s *fabric) {
bfa_ioc_get_adapter_model(fabric-fcs-bfa-ioc, model);
...
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
strncat((char *)port_cfg-sym_name,
BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
...
bfa_ioc_get_adapter_model(struct bfa_ioc_s *ioc, char *model) {
struct bfi_ioc_attr_s *ioc_attr;
WARN_ON(!model);
memset((void *)model, 0, BFA_ADAPTER_MODEL_NAME_LEN);
BFA_ADAPTER_MODEL_NAME_LEN = 16
Signed-off-by: Jim Meyering meyer...@redhat.com
---
drivers/scsi/bfa/bfa_fcs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/bfa/bfa_fcs.c
b/drivers/scsi/bfa/bfa_fcs.c index
eaac57e..3329493 100644
--- a/drivers/scsi/bfa/bfa_fcs.c
+++ b/drivers/scsi/bfa/bfa_fcs.c
@@ -713,6 +713,7 @@ bfa_fcs_fabric_psymb_init(struct
bfa_fcs_fabric_s
*fabric)
/* Model name/number */
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
+ port_cfg-sym_name[BFA_FCS_PORT_SYMBNAME_MODEL_SZ - 1]
= 0;
strncat((char *)port_cfg-sym_name,
BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
Nacked-by: Krishna Gudipati kgudi...@brocade.com
Hi Jim,
This model number is of length 12 bytes and the logic added here
will reset the model last byte.
In addition strncat does not need the src to be null terminated, the
change does not compile even.
NACK to this change.
Hi Krishna,
Thanks for the quick feedback and sorry the patch wasn't quite right.
However, the log is accurate: there is at least a theoretical problem
when the string in model (a buffer of size 16 bytes) has strlen = 12.
While strncat does not require that its second argument be
NUL-terminated, the first one (the destination) must be. Otherwise,
it has no way to determine the end of the string to which it must append the
source bytes.
Ping?
In case it wasn't clear, there *is* a risk of buffer overflow, which
happens when strncpy makes it so strncat's destination is not NUL
terminated.
If you require support for 12-byte model numbers, then you'll have to
increase the length of that buffer
(BFA_FCS_PORT_SYMBNAME_MODEL_SZ) to at least 13.
I've just rebased, and thus confirmed that the patches still apply.
Here is a v2 patch to which I've added the requisite (char*) cast.
However, this whole function is rather unreadable due to the
repetition (12 times!) of (char *)port_cfg-sym_name.
In case someone prefers to factor out that repetition, I've appended
a larger, v3 patch to do that.
Taking Andi's advice, I've made the offending code use strlcpy in place of
strncpy. More importantly, I've fixed the same bug also in the following,
nearly identical function.
-- 8 --
Two functions have this problem:
bfa_fcs_fabric_psymb_init
bfa_fcs_fabric_nsymb_init
They use strncpy to copy a model name of length up to 15 (16, if you count the
NUL), into a buffer of size 12 (BFA_FCS_PORT_SYMBNAME_MODEL_SZ).
However, strncpy does not always NUL-terminate, so whenever the original model
string has strlen = 12, the following strncat reads beyond end of the
-sym_name buffer as it attempts to find end of string.
Instead, use strlcpy, which
