+James S.
Hi Roman,
On Tue, 18 Aug 2020, 6:24am, Roman Bolshakov wrote:
>
> On Tue, Aug 18, 2020 at 05:31:54AM -0700, Nilesh Javali wrote:
> > From: Arun Easi <aeasi@xxxxxxxxxxx>
> >
> > Add a remote port debugfs entry to get/set dev_loss_tmo for NVMe
> > devices.
> >
> > Signed-off-by: Arun Easi <aeasi@xxxxxxxxxxx>
> > Signed-off-by: Himanshu Madhani <hmadhani@xxxxxxxxxxx>
> > Signed-off-by: Nilesh Javali <njavali@xxxxxxxxxxx>
> > ---
> > drivers/scsi/qla2xxx/qla_dfs.c | 54 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 54 insertions(+)
> >
> > diff --git a/drivers/scsi/qla2xxx/qla_dfs.c b/drivers/scsi/qla2xxx/qla_dfs.c
> > index 3c4b9b549b17..7482e6bf7f7f 100644
> > --- a/drivers/scsi/qla2xxx/qla_dfs.c
> > +++ b/drivers/scsi/qla2xxx/qla_dfs.c
> > @@ -12,6 +12,57 @@
> > static struct dentry *qla2x00_dfs_root;
> > static atomic_t qla2x00_dfs_root_count;
> >
> > +#define QLA_DFS_RPORT_DEVLOSS_TMO 1
> > +
> > +static int
> > +qla_dfs_rport_get(struct fc_port *fp, int attr_id, u64 *val)
> > +{
> > + switch (attr_id) {
> > + case QLA_DFS_RPORT_DEVLOSS_TMO:
> > + /* Only supported for FC-NVMe devices that are registered. */
> > + if (!(fp->nvme_flag & NVME_FLAG_REGISTERED))
> > + return -EIO;
> > + *val = fp->nvme_remote_port->dev_loss_tmo;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
> > +
> > +static int
> > +qla_dfs_rport_set(struct fc_port *fp, int attr_id, u64 val)
> > +{
> > + switch (attr_id) {
> > + case QLA_DFS_RPORT_DEVLOSS_TMO:
> > + /* Only supported for FC-NVMe devices that are registered. */
> > + if (!(fp->nvme_flag & NVME_FLAG_REGISTERED))
> > + return -EIO;
> > + return nvme_fc_set_remoteport_devloss(fp->nvme_remote_port,
> > + val);
> > + default:
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
> > +
> > +#define DEFINE_QLA_DFS_RPORT_RW_ATTR(_attr_id, _attr) \
> > +static int qla_dfs_rport_##_attr##_get(void *data, u64 *val) \
> > +{ \
> > + struct fc_port *fp = data; \
> > + return qla_dfs_rport_get(fp, _attr_id, val); \
> > +} \
> > +static int qla_dfs_rport_##_attr##_set(void *data, u64 val) \
> > +{ \
> > + struct fc_port *fp = data; \
> > + return qla_dfs_rport_set(fp, _attr_id, val); \
> > +} \
> > +DEFINE_DEBUGFS_ATTRIBUTE(qla_dfs_rport_##_attr##_fops, \
> > + qla_dfs_rport_##_attr##_get, \
> > + qla_dfs_rport_##_attr##_set, "%llu\n")
> > +
> > +DEFINE_QLA_DFS_RPORT_RW_ATTR(QLA_DFS_RPORT_DEVLOSS_TMO, dev_loss_tmo);
> > +
> > void
> > qla2x00_dfs_create_rport(scsi_qla_host_t *vha, struct fc_port *fp)
> > {
> > @@ -24,6 +75,9 @@ qla2x00_dfs_create_rport(scsi_qla_host_t *vha, struct fc_port *fp)
> > fp->dfs_rport_dir = debugfs_create_dir(wwn, vha->dfs_rport_root);
> > if (!fp->dfs_rport_dir)
> > return;
> > + if (NVME_TARGET(vha->hw, fp))
> > + debugfs_create_file("dev_loss_tmo", 0600, fp->dfs_rport_dir,
> > + fp, &qla_dfs_rport_dev_loss_tmo_fops);
> > }
> >
> > void
> > --
> > 2.19.0.rc0
> >
>
> Hi Arun,
>
> I don't think that the setting should be in debugfs. IMO there should be
> separate fc_remote_ports entry for FCP and FC-NVMe, one per PRLI for
> multi-protocol targets.
>
> That'd be consistent with what exists for FCP and would allow
> similar configuration of dev_loss_tmo from multipath. It'd also provide
> semantics of per-rport PRLO to allow logout from either of the protocol
> but leaving second rport/session online.
>
Thanks for your comments. This debugfs way is sort of a crutch, but that
is something a user could use to set for FC-NVME dev_loss_tmo until a
FC-NVME sysfs node hierarchy for a FC initiator (similar to FCP) comes
into place.
BTW, the above setting is specific for FC-NVME, and is separate from FCP;
so PRLO semantics can still be met.
Copying James for his thoughts on the topic (on dev_loss_tmo setting).
James, would you mind sharing your ideas you'd considered?
Regards,
-Arun
