[PATCH 0/5] bnx2fc: Update driver to 2.10.3.

[PATCH 0/5] bnx2fc: Update driver to 2.10.3.

[Chad Dupuis]

Hi James, Martin,

Please add the following patches to the scsi tree at your earliest
convenience.

Chad Dupuis (4):
  bnx2fc: Print when we send a fip keep alive.
  bnx2fc: Print netdev device name when FCoE is successfully
    initialized.
  bnx2fc: Check sc_cmd device and host pointer before returning the
    command to the mid-layer.
  bnx2fc: Update version number to 2.10.3.

Joe Carnuccio (1):
  bnx2fc: Add driver tunables.

 drivers/scsi/bnx2fc/bnx2fc.h      |  2 +-
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 59 ++++++++++++++++++++++++++++++++++++++-
 drivers/scsi/bnx2fc/bnx2fc_io.c   | 16 ++++++++++-
 3 files changed, 74 insertions(+), 3 deletions(-)

-- 
2.0.0.rc0.26.g779792a

[PATCH 1/5] bnx2fc: Add driver tunables.

[Chad Dupuis]

From: Joe Carnuccio <joe.carnuccio@qlogic.com>

Per customer request, add the following driver tunables:

o devloss_tmo
o max_luns
o queue_depth
o tm_timeout

Signed-off-by: Joe Carnuccio <joe.carnuccio@qlogic.com>
Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
---
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 38 +++++++++++++++++++++++++++++++++++++-
 drivers/scsi/bnx2fc/bnx2fc_io.c   |  4 +++-
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index d7029ea..600c29d 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -107,6 +107,26 @@ MODULE_PARM_DESC(debug_logging,
 		"\t\t0x10 - fcoe L2 fame related logs.\n"
 		"\t\t0xff - LOG all messages.");
 
+uint bnx2fc_devloss_tmo;
+module_param_named(devloss_tmo, bnx2fc_devloss_tmo, uint, S_IRUGO);
+MODULE_PARM_DESC(devloss_tmo, " Change devloss_tmo for the remote ports "
+	"attached via bnx2fc.");
+
+uint bnx2fc_max_luns = BNX2FC_MAX_LUN;
+module_param_named(max_luns, bnx2fc_max_luns, uint, S_IRUGO);
+MODULE_PARM_DESC(max_luns, " Change the default max_lun per SCSI host. Default "
+	"0xffff.");
+
+uint bnx2fc_queue_depth;
+module_param_named(queue_depth, bnx2fc_queue_depth, uint, S_IRUGO);
+MODULE_PARM_DESC(queue_depth, " Change the default queue depth of SCSI devices "
+	"attached via bnx2fc.");
+
+uint bnx2fc_tm_timeout = BNX2FC_TM_TIMEOUT;
+module_param_named(tm_timeout, bnx2fc_tm_timeout, uint, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(tm_timeout, " Change the default timeout for "
+	"task management commands. Default 60 seconds.");
+
 static int bnx2fc_cpu_callback(struct notifier_block *nfb,
 			     unsigned long action, void *hcpu);
 /* notification function for CPU hotplug events */
@@ -692,7 +712,7 @@ static int bnx2fc_shost_config(struct fc_lport *lport, struct device *dev)
 	int rc = 0;
 
 	shost->max_cmd_len = BNX2FC_MAX_CMD_LEN;
-	shost->max_lun = BNX2FC_MAX_LUN;
+	shost->max_lun = bnx2fc_max_luns;
 	shost->max_id = BNX2FC_MAX_FCP_TGT;
 	shost->max_channel = 0;
 	if (lport->vport)
@@ -1102,6 +1122,9 @@ static int bnx2fc_vport_create(struct fc_vport *vport, bool disabled)
 		return -EIO;
 	}
 
+	if (bnx2fc_devloss_tmo)
+		fc_host_dev_loss_tmo(vn_port->host) = bnx2fc_devloss_tmo;
+
 	if (disabled) {
 		fc_vport_set_state(vport, FC_VPORT_DISABLED);
 	} else {
@@ -1495,6 +1518,9 @@ static struct fc_lport *bnx2fc_if_create(struct bnx2fc_interface *interface,
 	}
 	fc_host_port_type(lport->host) = FC_PORTTYPE_UNKNOWN;
 
+	if (bnx2fc_devloss_tmo)
+		fc_host_dev_loss_tmo(shost) = bnx2fc_devloss_tmo;
+
 	/* Allocate exchange manager */
 	if (!npiv)
 		rc = bnx2fc_em_config(lport, hba);
@@ -2612,6 +2638,15 @@ static int bnx2fc_cpu_callback(struct notifier_block *nfb,
 	return NOTIFY_OK;
 }
 
+static int bnx2fc_slave_configure(struct scsi_device *sdev)
+{
+	if (!bnx2fc_queue_depth)
+		return 0;
+
+	scsi_change_queue_depth(sdev, bnx2fc_queue_depth);
+	return 0;
+}
+
 /**
  * bnx2fc_mod_init - module init entry point
  *
@@ -2877,6 +2912,7 @@ static struct scsi_host_template bnx2fc_shost_template = {
 	.sg_tablesize		= BNX2FC_MAX_BDS_PER_CMD,
 	.max_sectors		= 1024,
 	.track_queue_depth	= 1,
+	.slave_configure = bnx2fc_slave_configure,
 };
 
 static struct libfc_function_template bnx2fc_libfc_fcn_templ = {
diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 0002caf..0f60e22 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -15,6 +15,8 @@
 
 #define RESERVE_FREE_LIST_INDEX num_possible_cpus()
 
+extern uint bnx2fc_tm_timeout;
+
 static int bnx2fc_split_bd(struct bnx2fc_cmd *io_req, u64 addr, int sg_len,
 			   int bd_index);
 static int bnx2fc_map_sg(struct bnx2fc_cmd *io_req);
@@ -770,7 +772,7 @@ retry_tmf:
 	spin_unlock_bh(&tgt->tgt_lock);
 
 	rc = wait_for_completion_timeout(&io_req->tm_done,
-					 BNX2FC_TM_TIMEOUT * HZ);
+					 bnx2fc_tm_timeout * HZ);
 	spin_lock_bh(&tgt->tgt_lock);
 
 	io_req->wait_for_comp = 0;
-- 
2.0.0.rc0.26.g779792a

[PATCH 2/5] bnx2fc: Print when we send a fip keep alive.

[Chad Dupuis]

Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
---
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 600c29d..d95eee6 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -127,6 +127,11 @@ module_param_named(tm_timeout, bnx2fc_tm_timeout, uint, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(tm_timeout, " Change the default timeout for "
 	"task management commands. Default 60 seconds.");
 
+uint bnx2fc_log_fka;
+module_param_named(log_fka, bnx2fc_log_fka, uint, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(log_fka, " Print message to kernel log when fcoe is "
+	"initiating a FIP keep alive when debug logging is enabled.");
+
 static int bnx2fc_cpu_callback(struct notifier_block *nfb,
 			     unsigned long action, void *hcpu);
 /* notification function for CPU hotplug events */
@@ -1081,6 +1086,20 @@ static u8 *bnx2fc_get_src_mac(struct fc_lport *lport)
  */
 static void bnx2fc_fip_send(struct fcoe_ctlr *fip, struct sk_buff *skb)
 {
+	struct fip_header *fiph;
+	struct ethhdr *eth_hdr;
+	u16 op;
+	u8 sub;
+
+	fiph = (struct fip_header *) ((void *)skb->data + 2 * ETH_ALEN + 2);
+	eth_hdr = (struct ethhdr *)skb_mac_header(skb);
+	op = ntohs(fiph->fip_op);
+	sub = fiph->fip_subcode;
+
+	if (op == FIP_OP_CTRL && sub == FIP_SC_SOL && bnx2fc_log_fka)
+		BNX2FC_MISC_DBG("Sending FKA from %pM to %pM.\n",
+		    eth_hdr->h_source, eth_hdr->h_dest);
+
 	skb->dev = bnx2fc_from_ctlr(fip)->netdev;
 	dev_queue_xmit(skb);
 }
-- 
2.0.0.rc0.26.g779792a

[PATCH 3/5] bnx2fc: Print netdev device name when FCoE is successfully initialized.

[Chad Dupuis]

Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
---
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index d95eee6..60423cf 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -2044,6 +2044,8 @@ static void bnx2fc_ulp_init(struct cnic_dev *dev)
 		return;
 	}
 
+	pr_err(PFX "FCoE initialized for %s.\n", dev->netdev->name);
+
 	/* Add HBA to the adapter list */
 	mutex_lock(&bnx2fc_dev_lock);
 	list_add_tail(&hba->list, &adapter_list);
-- 
2.0.0.rc0.26.g779792a

[PATCH 4/5] bnx2fc: Check sc_cmd device and host pointer before returning the command to the mid-layer.

[Chad Dupuis]

When we are in connection recovery and the internal command timer on a request
pops, either the scsi_cmnd->device or scsi_cmnd->device->host back pointers may
be NULL as the device that the command that the request was submitted on may
have been subsequently reaped due to the connection recovery. This can cause
one or both of the pointers above to be NULL and cause a system crash if we try
to return the command to the midlayer.

Instead, double check the pointers before the return to the midlayer so as to
prevent the crash and let the upper layers finish the session recovery and
rediscover the device.

Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
---
 drivers/scsi/bnx2fc/bnx2fc_io.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 0f60e22..33e519d 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -181,12 +181,24 @@ static void bnx2fc_scsi_done(struct bnx2fc_cmd *io_req, int err_code)
 
 	bnx2fc_unmap_sg_list(io_req);
 	io_req->sc_cmd = NULL;
+
+	/* Sanity checks before returning command to mid-layer */
 	if (!sc_cmd) {
 		printk(KERN_ERR PFX "scsi_done - sc_cmd NULL. "
 				    "IO(0x%x) already cleaned up\n",
 		       io_req->xid);
 		return;
 	}
+	if (!sc_cmd->device) {
+		pr_err(PFX "0x%x: sc_cmd->device is NULL.\n", io_req->xid);
+		return;
+	}
+	if (!sc_cmd->device->host) {
+		pr_err(PFX "0x%x: sc_cmd->device->host is NULL.\n",
+		    io_req->xid);
+		return;
+	}
+
 	sc_cmd->result = err_code << 16;
 
 	BNX2FC_IO_DBG(io_req, "sc=%p, result=0x%x, retries=%d, allowed=%d\n",
-- 
2.0.0.rc0.26.g779792a

[PATCH 5/5] bnx2fc: Update version number to 2.10.3.

[Chad Dupuis]

Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
---
 drivers/scsi/bnx2fc/bnx2fc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc.h b/drivers/scsi/bnx2fc/bnx2fc.h
index 499e369..9ce361a 100644
--- a/drivers/scsi/bnx2fc/bnx2fc.h
+++ b/drivers/scsi/bnx2fc/bnx2fc.h
@@ -65,7 +65,7 @@
 #include "bnx2fc_constants.h"
 
 #define BNX2FC_NAME		"bnx2fc"
-#define BNX2FC_VERSION		"2.9.6"
+#define BNX2FC_VERSION		"2.10.3"
 
 #define PFX			"bnx2fc: "
 
-- 
2.0.0.rc0.26.g779792a

Re: [PATCH 1/5] bnx2fc: Add driver tunables.

[Johannes Thumshirn]

On 2016-04-01 15:10, Chad Dupuis wrote:
> From: Joe Carnuccio <joe.carnuccio@qlogic.com>
> 
> Per customer request, add the following driver tunables:
> 
> o devloss_tmo
> o max_luns
> o queue_depth
> o tm_timeout
> 
> Signed-off-by: Joe Carnuccio <joe.carnuccio@qlogic.com>
> Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
> ---
>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 38 
> +++++++++++++++++++++++++++++++++++++-
>  drivers/scsi/bnx2fc/bnx2fc_io.c   |  4 +++-
>  2 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> index d7029ea..600c29d 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> @@ -107,6 +107,26 @@ MODULE_PARM_DESC(debug_logging,
>  		"\t\t0x10 - fcoe L2 fame related logs.\n"
>  		"\t\t0xff - LOG all messages.");
> 
> +uint bnx2fc_devloss_tmo;
> +module_param_named(devloss_tmo, bnx2fc_devloss_tmo, uint, S_IRUGO);
> +MODULE_PARM_DESC(devloss_tmo, " Change devloss_tmo for the remote 
> ports "
> +	"attached via bnx2fc.");
> +
> +uint bnx2fc_max_luns = BNX2FC_MAX_LUN;
> +module_param_named(max_luns, bnx2fc_max_luns, uint, S_IRUGO);
> +MODULE_PARM_DESC(max_luns, " Change the default max_lun per SCSI
> host. Default "
> +	"0xffff.");
> +
> +uint bnx2fc_queue_depth;
> +module_param_named(queue_depth, bnx2fc_queue_depth, uint, S_IRUGO);
> +MODULE_PARM_DESC(queue_depth, " Change the default queue depth of
> SCSI devices "
> +	"attached via bnx2fc.");
> +
> +uint bnx2fc_tm_timeout = BNX2FC_TM_TIMEOUT;
> +module_param_named(tm_timeout, bnx2fc_tm_timeout, uint, 
> S_IRUGO|S_IWUSR);
> +MODULE_PARM_DESC(tm_timeout, " Change the default timeout for "
> +	"task management commands. Default 60 seconds.");
> +

Just a question, can't this be made dynamically adjustable via sysfs 
instead of a module parameter?

Re: [PATCH 2/5] bnx2fc: Print when we send a fip keep alive.

[Johannes Thumshirn]

On 2016-04-01 15:10, Chad Dupuis wrote:
> Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
> ---

Looks like a nice debug feature,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

Re: [PATCH 3/5] bnx2fc: Print netdev device name when FCoE is successfully initialized.

[Johannes Thumshirn]

On 2016-04-01 15:10, Chad Dupuis wrote:
> Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
> ---
>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> index d95eee6..60423cf 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> @@ -2044,6 +2044,8 @@ static void bnx2fc_ulp_init(struct cnic_dev *dev)
>  		return;
>  	}
> 
> +	pr_err(PFX "FCoE initialized for %s.\n", dev->netdev->name);
> +
>  	/* Add HBA to the adapter list */
>  	mutex_lock(&bnx2fc_dev_lock);
>  	list_add_tail(&hba->list, &adapter_list);

I don't think KERN_ERR is the appropriate log level for this, can you 
change it to pr_info()?

Re: [PATCH 4/5] bnx2fc: Check sc_cmd device and host pointer before returning the command to the mid-layer.

[Johannes Thumshirn]

On 2016-04-01 15:10, Chad Dupuis wrote:
> When we are in connection recovery and the internal command timer on a 
> request
> pops, either the scsi_cmnd->device or scsi_cmnd->device->host back 
> pointers may
> be NULL as the device that the command that the request was submitted 
> on may
> have been subsequently reaped due to the connection recovery. This can 
> cause
> one or both of the pointers above to be NULL and cause a system crash 
> if we try
> to return the command to the midlayer.
> 
> Instead, double check the pointers before the return to the midlayer so 
> as to
> prevent the crash and let the upper layers finish the session recovery 
> and
> rediscover the device.
> 
> Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
> ---

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

Re: [PATCH 5/5] bnx2fc: Update version number to 2.10.3.

[Johannes Thumshirn]

On 2016-04-01 15:10, Chad Dupuis wrote:
> Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
> ---

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

Re: [PATCH 1/5] bnx2fc: Add driver tunables.

[Chad Dupuis]

On Fri, 1 Apr 2016, Johannes Thumshirn wrote:

> On 2016-04-01 15:10, Chad Dupuis wrote:
>> From: Joe Carnuccio <joe.carnuccio@qlogic.com>
>> 
>> Per customer request, add the following driver tunables:
>> 
>> o devloss_tmo
>> o max_luns
>> o queue_depth
>> o tm_timeout
>> 
>> Signed-off-by: Joe Carnuccio <joe.carnuccio@qlogic.com>
>> Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
>> ---
>>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 38 
>> +++++++++++++++++++++++++++++++++++++-
>>  drivers/scsi/bnx2fc/bnx2fc_io.c   |  4 +++-
>>  2 files changed, 40 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>> index d7029ea..600c29d 100644
>> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>> @@ -107,6 +107,26 @@ MODULE_PARM_DESC(debug_logging,
>>  		"\t\t0x10 - fcoe L2 fame related logs.\n"
>>  		"\t\t0xff - LOG all messages.");
>> 
>> +uint bnx2fc_devloss_tmo;
>> +module_param_named(devloss_tmo, bnx2fc_devloss_tmo, uint, S_IRUGO);
>> +MODULE_PARM_DESC(devloss_tmo, " Change devloss_tmo for the remote ports "
>> +	"attached via bnx2fc.");
>> +
>> +uint bnx2fc_max_luns = BNX2FC_MAX_LUN;
>> +module_param_named(max_luns, bnx2fc_max_luns, uint, S_IRUGO);
>> +MODULE_PARM_DESC(max_luns, " Change the default max_lun per SCSI
>> host. Default "
>> +	"0xffff.");
>> +
>> +uint bnx2fc_queue_depth;
>> +module_param_named(queue_depth, bnx2fc_queue_depth, uint, S_IRUGO);
>> +MODULE_PARM_DESC(queue_depth, " Change the default queue depth of
>> SCSI devices "
>> +	"attached via bnx2fc.");
>> +
>> +uint bnx2fc_tm_timeout = BNX2FC_TM_TIMEOUT;
>> +module_param_named(tm_timeout, bnx2fc_tm_timeout, uint, S_IRUGO|S_IWUSR);
>> +MODULE_PARM_DESC(tm_timeout, " Change the default timeout for "
>> +	"task management commands. Default 60 seconds.");
>> +
>
> Just a question, can't this be made dynamically adjustable via sysfs instead 
> of a module parameter?
>

I presume you're talking about something like a 
/sys/class/scsi_host/hostX/tm_timeout sysfs node?

Re: [PATCH 3/5] bnx2fc: Print netdev device name when FCoE is successfully initialized.

[Chad Dupuis]

On Fri, 1 Apr 2016, Johannes Thumshirn wrote:

> On 2016-04-01 15:10, Chad Dupuis wrote:
>> Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
>> ---
>>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>> index d95eee6..60423cf 100644
>> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>> @@ -2044,6 +2044,8 @@ static void bnx2fc_ulp_init(struct cnic_dev *dev)
>>  		return;
>>  	}
>> 
>> +	pr_err(PFX "FCoE initialized for %s.\n", dev->netdev->name);
>> +
>>  	/* Add HBA to the adapter list */
>>  	mutex_lock(&bnx2fc_dev_lock);
>>  	list_add_tail(&hba->list, &adapter_list);
>
> I don't think KERN_ERR is the appropriate log level for this, can you change 
> it to pr_info()?
>

Yes, I'll change it to a pr_info in a V2 set.

Re: [PATCH 1/5] bnx2fc: Add driver tunables.

[James Bottomley]

On Fri, 2016-04-01 at 10:06 -0400, Chad Dupuis wrote:
> On Fri, 1 Apr 2016, Johannes Thumshirn wrote:
> 
> > On 2016-04-01 15:10, Chad Dupuis wrote:
> > > From: Joe Carnuccio <joe.carnuccio@qlogic.com>
> > > 
> > > Per customer request, add the following driver tunables:
> > > 
> > > o devloss_tmo
> > > o max_luns
> > > o queue_depth
> > > o tm_timeout
> > > 
> > > Signed-off-by: Joe Carnuccio <joe.carnuccio@qlogic.com>
> > > Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
> > > ---
> > >  drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 38 
> > > +++++++++++++++++++++++++++++++++++++-
> > >  drivers/scsi/bnx2fc/bnx2fc_io.c   |  4 +++-
> > >  2 files changed, 40 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> > > b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> > > index d7029ea..600c29d 100644
> > > --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> > > +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> > > @@ -107,6 +107,26 @@ MODULE_PARM_DESC(debug_logging,
> > >  		"\t\t0x10 - fcoe L2 fame related logs.\n"
> > >  		"\t\t0xff - LOG all messages.");
> > > 
> > > +uint bnx2fc_devloss_tmo;
> > > +module_param_named(devloss_tmo, bnx2fc_devloss_tmo, uint,
> > > S_IRUGO);
> > > +MODULE_PARM_DESC(devloss_tmo, " Change devloss_tmo for the
> > > remote ports "
> > > +	"attached via bnx2fc.");
> > > +
> > > +uint bnx2fc_max_luns = BNX2FC_MAX_LUN;
> > > +module_param_named(max_luns, bnx2fc_max_luns, uint, S_IRUGO);
> > > +MODULE_PARM_DESC(max_luns, " Change the default max_lun per SCSI
> > > host. Default "
> > > +	"0xffff.");
> > > +
> > > +uint bnx2fc_queue_depth;
> > > +module_param_named(queue_depth, bnx2fc_queue_depth, uint,
> > > S_IRUGO);
> > > +MODULE_PARM_DESC(queue_depth, " Change the default queue depth
> > > of
> > > SCSI devices "
> > > +	"attached via bnx2fc.");
> > > +
> > > +uint bnx2fc_tm_timeout = BNX2FC_TM_TIMEOUT;
> > > +module_param_named(tm_timeout, bnx2fc_tm_timeout, uint,
> > > S_IRUGO|S_IWUSR);
> > > +MODULE_PARM_DESC(tm_timeout, " Change the default timeout for "
> > > +	"task management commands. Default 60 seconds.");
> > > +
> > 
> > Just a question, can't this be made dynamically adjustable via
> > sysfs instead 
> > of a module parameter?
> > 
> 
> I presume you're talking about something like a 
> /sys/class/scsi_host/hostX/tm_timeout sysfs node?

Yes, but there's also the question of whether they should be generic
rather than bnx2fc specific.  At least queue_depth, max_luns and
possibly tm_timeout would seem to belong to the SCSI host itself. 
 devloss_tmo looks like it should be a host parameter within the fc
transport class.

James

Re: [PATCH 1/5] bnx2fc: Add driver tunables.

[Johannes Thumshirn]

On 2016-04-01 16:06, Chad Dupuis wrote:
> On Fri, 1 Apr 2016, Johannes Thumshirn wrote:
> 
>> On 2016-04-01 15:10, Chad Dupuis wrote:
>>> From: Joe Carnuccio <joe.carnuccio@qlogic.com>
>>> 
>>> Per customer request, add the following driver tunables:
>>> 
>>> o devloss_tmo
>>> o max_luns
>>> o queue_depth
>>> o tm_timeout
>>> 
>>> Signed-off-by: Joe Carnuccio <joe.carnuccio@qlogic.com>
>>> Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
>>> ---
>>>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 38 
>>> +++++++++++++++++++++++++++++++++++++-
>>>  drivers/scsi/bnx2fc/bnx2fc_io.c   |  4 +++-
>>>  2 files changed, 40 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>>> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>>> index d7029ea..600c29d 100644
>>> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>>> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>>> @@ -107,6 +107,26 @@ MODULE_PARM_DESC(debug_logging,
>>>  		"\t\t0x10 - fcoe L2 fame related logs.\n"
>>>  		"\t\t0xff - LOG all messages.");
>>> 
>>> +uint bnx2fc_devloss_tmo;
>>> +module_param_named(devloss_tmo, bnx2fc_devloss_tmo, uint, S_IRUGO);
>>> +MODULE_PARM_DESC(devloss_tmo, " Change devloss_tmo for the remote 
>>> ports "
>>> +	"attached via bnx2fc.");
>>> +
>>> +uint bnx2fc_max_luns = BNX2FC_MAX_LUN;
>>> +module_param_named(max_luns, bnx2fc_max_luns, uint, S_IRUGO);
>>> +MODULE_PARM_DESC(max_luns, " Change the default max_lun per SCSI
>>> host. Default "
>>> +	"0xffff.");
>>> +
>>> +uint bnx2fc_queue_depth;
>>> +module_param_named(queue_depth, bnx2fc_queue_depth, uint, S_IRUGO);
>>> +MODULE_PARM_DESC(queue_depth, " Change the default queue depth of
>>> SCSI devices "
>>> +	"attached via bnx2fc.");
>>> +
>>> +uint bnx2fc_tm_timeout = BNX2FC_TM_TIMEOUT;
>>> +module_param_named(tm_timeout, bnx2fc_tm_timeout, uint, 
>>> S_IRUGO|S_IWUSR);
>>> +MODULE_PARM_DESC(tm_timeout, " Change the default timeout for "
>>> +	"task management commands. Default 60 seconds.");
>>> +
>> 
>> Just a question, can't this be made dynamically adjustable via sysfs 
>> instead of a module parameter?
>> 
> 
> I presume you're talking about something like a
> /sys/class/scsi_host/hostX/tm_timeout sysfs node?

Yeah, something like that

Re: [PATCH 1/5] bnx2fc: Add driver tunables.

[Chad Dupuis]

On Fri, 1 Apr 2016, James Bottomley wrote:

> On Fri, 2016-04-01 at 10:06 -0400, Chad Dupuis wrote:
>> On Fri, 1 Apr 2016, Johannes Thumshirn wrote:
>>
>>> On 2016-04-01 15:10, Chad Dupuis wrote:
>>>> From: Joe Carnuccio <joe.carnuccio@qlogic.com>
>>>>
>>>> Per customer request, add the following driver tunables:
>>>>
>>>> o devloss_tmo
>>>> o max_luns
>>>> o queue_depth
>>>> o tm_timeout
>>>>
>>>> Signed-off-by: Joe Carnuccio <joe.carnuccio@qlogic.com>
>>>> Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
>>>> ---
>>>>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 38
>>>> +++++++++++++++++++++++++++++++++++++-
>>>>  drivers/scsi/bnx2fc/bnx2fc_io.c   |  4 +++-
>>>>  2 files changed, 40 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>>>> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>>>> index d7029ea..600c29d 100644
>>>> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>>>> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>>>> @@ -107,6 +107,26 @@ MODULE_PARM_DESC(debug_logging,
>>>>  		"\t\t0x10 - fcoe L2 fame related logs.\n"
>>>>  		"\t\t0xff - LOG all messages.");
>>>>
>>>> +uint bnx2fc_devloss_tmo;
>>>> +module_param_named(devloss_tmo, bnx2fc_devloss_tmo, uint,
>>>> S_IRUGO);
>>>> +MODULE_PARM_DESC(devloss_tmo, " Change devloss_tmo for the
>>>> remote ports "
>>>> +	"attached via bnx2fc.");
>>>> +
>>>> +uint bnx2fc_max_luns = BNX2FC_MAX_LUN;
>>>> +module_param_named(max_luns, bnx2fc_max_luns, uint, S_IRUGO);
>>>> +MODULE_PARM_DESC(max_luns, " Change the default max_lun per SCSI
>>>> host. Default "
>>>> +	"0xffff.");
>>>> +
>>>> +uint bnx2fc_queue_depth;
>>>> +module_param_named(queue_depth, bnx2fc_queue_depth, uint,
>>>> S_IRUGO);
>>>> +MODULE_PARM_DESC(queue_depth, " Change the default queue depth
>>>> of
>>>> SCSI devices "
>>>> +	"attached via bnx2fc.");
>>>> +
>>>> +uint bnx2fc_tm_timeout = BNX2FC_TM_TIMEOUT;
>>>> +module_param_named(tm_timeout, bnx2fc_tm_timeout, uint,
>>>> S_IRUGO|S_IWUSR);
>>>> +MODULE_PARM_DESC(tm_timeout, " Change the default timeout for "
>>>> +	"task management commands. Default 60 seconds.");
>>>> +
>>>
>>> Just a question, can't this be made dynamically adjustable via
>>> sysfs instead
>>> of a module parameter?
>>>
>>
>> I presume you're talking about something like a
>> /sys/class/scsi_host/hostX/tm_timeout sysfs node?
>
> Yes, but there's also the question of whether they should be generic
> rather than bnx2fc specific.  At least queue_depth, max_luns and
> possibly tm_timeout would seem to belong to the SCSI host itself.
> devloss_tmo looks like it should be a host parameter within the fc
> transport class.
>
> James
>

The use case for this is so that someone can administratively set these 
parameters on driver load so that all devices discovered via our driver 
will get these parameters set automatically usually via the 
slave_configure callback.  So functionally that's what is trying to 
be accomplished with this patch and what we would be looking for in a 
generic solution.  Would the scsi_host_template and/or fc_host_template 
be the appropriate place to set this then so the values could be 
initialized during scsi_host/fc_host discovery?