| Message ID | 1454604319-27947-4-git-send-email-himanshu.madhani@qlogic.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 02/04/2016 08:45 AM, Himanshu Madhani wrote: > From: Quinn Tran <quinn.tran@qlogic.com> > > #cat /sys/kernel/debug/qla2xxx/qla2xxx_31/tgt_sess > qla2xxx_31 > Port ID Port Name Handle > ff:fc:01 21:fd:00:05:33:c7:ec:16 0 > 01:0e:00 21:00:00:24:ff:7b:8a:e4 1 > 01:0f:00 21:00:00:24:ff:7b:8a:e5 2 > .... Hello Quinn and Himanshu, The above information is not only useful to people who are debugging the QLogic target driver but also to end users who want to check which initiator ports have already logged in to a target port. Hence my proposal to move this information from debugfs to another location (e.g. configfs or sysfs). Users of other target drivers are probably also interested in seeing which sessions are active. How about adding the functionality for reporting session information per target port in the LIO core in such a way that some attributes are available for all target drivers (e.g. initiator port name, SCSI command statistics) and such that target drivers can define additional attributes to exported for each session (e.g. port ID, handle, ...). Thanks, Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bart, On 2/4/16, 10:16 AM, "Bart Van Assche" <bart.vanassche@sandisk.com> wrote: >On 02/04/2016 08:45 AM, Himanshu Madhani wrote: >> From: Quinn Tran <quinn.tran@qlogic.com> >> >> #cat /sys/kernel/debug/qla2xxx/qla2xxx_31/tgt_sess >> qla2xxx_31 >> Port ID Port Name Handle >> ff:fc:01 21:fd:00:05:33:c7:ec:16 0 >> 01:0e:00 21:00:00:24:ff:7b:8a:e4 1 >> 01:0f:00 21:00:00:24:ff:7b:8a:e5 2 >> .... > >Hello Quinn and Himanshu, > >The above information is not only useful to people who are debugging the >QLogic target driver but also to end users who want to check which >initiator ports have already logged in to a target port. Hence my >proposal to move this information from debugfs to another location (e.g. >configfs or sysfs). Users of other target drivers are probably also >interested in seeing which sessions are active. How about adding the >functionality for reporting session information per target port in the >LIO core in such a way that some attributes are available for all target >drivers (e.g. initiator port name, SCSI command statistics) and such >that target drivers can define additional attributes to exported for >each session (e.g. port ID, handle, ...). We had initially implemented this as a sysfs hook, but knowing that sysfs is not encouraged, we decided to put this information via debugfs. Would it make more sense if we send a sysfs patch? > >Thanks, > >Bart. Thanks, - Himanshu
On 02/05/2016 09:26 AM, Himanshu Madhani wrote: > On 2/4/16, 10:16 AM, "Bart Van Assche" <bart.vanassche@sandisk.com> wrote: >> On 02/04/2016 08:45 AM, Himanshu Madhani wrote: >>> From: Quinn Tran <quinn.tran@qlogic.com> >>> >>> #cat /sys/kernel/debug/qla2xxx/qla2xxx_31/tgt_sess >>> qla2xxx_31 >>> Port ID Port Name Handle >>> ff:fc:01 21:fd:00:05:33:c7:ec:16 0 >>> 01:0e:00 21:00:00:24:ff:7b:8a:e4 1 >>> 01:0f:00 21:00:00:24:ff:7b:8a:e5 2 >>> .... >> >> Hello Quinn and Himanshu, >> >> The above information is not only useful to people who are debugging the >> QLogic target driver but also to end users who want to check which >> initiator ports have already logged in to a target port. Hence my >> proposal to move this information from debugfs to another location (e.g. >> configfs or sysfs). Users of other target drivers are probably also >> interested in seeing which sessions are active. How about adding the >> functionality for reporting session information per target port in the >> LIO core in such a way that some attributes are available for all target >> drivers (e.g. initiator port name, SCSI command statistics) and such >> that target drivers can define additional attributes to exported for >> each session (e.g. port ID, handle, ...). > > We had initially implemented this as a sysfs hook, but knowing that sysfs > is not encouraged, we decided to put this information via debugfs. Would it > make more sense if we send a sysfs patch? Hello Himanshu, Let's try to reach agreement about the approach first before starting to rework this patch. Five years ago I explained in a message that I posted on the linux-scsi mailing list why LIO should use sysfs to export information that changes dynamically (see also http://thread.gmane.org/gmane.linux.scsi/65615/). I think this patch shows that there is a real need to have detailed session information from LIO target drivers in user space. We need one directory per session instead of exporting all session information through a single file. sysfs is the right filesystem to export such information because configfs directories should be created by the user and not from inside the kernel. If no agreement can be reached about this over e-mail my proposal is to discuss this further at the 2016 LSF/MM summit. Thanks, Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2/5/16, 12:00 PM, "Bart Van Assche" <bart.vanassche@sandisk.com> wrote: >On 02/05/2016 09:26 AM, Himanshu Madhani wrote: >> On 2/4/16, 10:16 AM, "Bart Van Assche" <bart.vanassche@sandisk.com> >>wrote: >>> On 02/04/2016 08:45 AM, Himanshu Madhani wrote: >>>> From: Quinn Tran <quinn.tran@qlogic.com> >>>> >>>> #cat /sys/kernel/debug/qla2xxx/qla2xxx_31/tgt_sess >>>> qla2xxx_31 >>>> Port ID Port Name Handle >>>> ff:fc:01 21:fd:00:05:33:c7:ec:16 0 >>>> 01:0e:00 21:00:00:24:ff:7b:8a:e4 1 >>>> 01:0f:00 21:00:00:24:ff:7b:8a:e5 2 >>>> .... >>> >>> Hello Quinn and Himanshu, >>> >>> The above information is not only useful to people who are debugging >>>the >>> QLogic target driver but also to end users who want to check which >>> initiator ports have already logged in to a target port. Hence my >>> proposal to move this information from debugfs to another location >>>(e.g. >>> configfs or sysfs). Users of other target drivers are probably also >>> interested in seeing which sessions are active. How about adding the >>> functionality for reporting session information per target port in the >>> LIO core in such a way that some attributes are available for all >>>target >>> drivers (e.g. initiator port name, SCSI command statistics) and such >>> that target drivers can define additional attributes to exported for >>> each session (e.g. port ID, handle, ...). >> >> We had initially implemented this as a sysfs hook, but knowing that >>sysfs >> is not encouraged, we decided to put this information via debugfs. >>Would it >> make more sense if we send a sysfs patch? > >Hello Himanshu, > >Let's try to reach agreement about the approach first before starting to >rework this patch. > >Five years ago I explained in a message that I posted on the linux-scsi >mailing list why LIO should use sysfs to export information that changes >dynamically (see also http://thread.gmane.org/gmane.linux.scsi/65615/). >I think this patch shows that there is a real need to have detailed >session information from LIO target drivers in user space. We need one >directory per session instead of exporting all session information >through a single file. sysfs is the right filesystem to export such >information because configfs directories should be created by the user >and not from inside the kernel. If no agreement can be reached about >this over e-mail my proposal is to discuss this further at the 2016 >LSF/MM summit. Bart, I see lot of target customers requesting for such information and having something that can be created inside kernel space will be helpful. Let us discuss further at 2016 LSF/MM summit. ‹ Giri > >Thanks, > >Bart.
Hi Himanshu & Quinn, On Thu, 2016-02-04 at 11:45 -0500, Himanshu Madhani wrote: > From: Quinn Tran <quinn.tran@qlogic.com> > > #cat /sys/kernel/debug/qla2xxx/qla2xxx_31/tgt_sess > qla2xxx_31 > Port ID Port Name Handle > ff:fc:01 21:fd:00:05:33:c7:ec:16 0 > 01:0e:00 21:00:00:24:ff:7b:8a:e4 1 > 01:0f:00 21:00:00:24:ff:7b:8a:e5 2 > .... > > Signed-off-by: Quinn Tran <quinn.tran@qlogic.com> > Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com> > --- > drivers/scsi/qla2xxx/qla_def.h | 1 + > drivers/scsi/qla2xxx/qla_dfs.c | 55 ++++++++++++++++++++++++++++++++++++ > drivers/scsi/qla2xxx/qla_target.c | 56 ++++++++++++++++++++++++------------ > 3 files changed, 93 insertions(+), 19 deletions(-) > So looking at this patch beyond the debugfs part, it does change where ->check_initiator_node_acl() gets call during qlt_create_sess(). I assume this is related to new debugfs attribute, and these changes (plus others in qlt_del_sess_work_fn) are not bug-fixes on their own, correct..? Aside from that, I don't have an objection to merge as v4.6 for-next code if QLogic finds it useful for debugging. > diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h > index 9872f34..e6c5bcf 100644 > --- a/drivers/scsi/qla2xxx/qla_def.h > +++ b/drivers/scsi/qla2xxx/qla_def.h > @@ -2929,6 +2929,7 @@ struct qlt_hw_data { > > uint8_t tgt_node_name[WWN_SIZE]; > > + struct dentry *dfs_tgt_sess; > struct list_head q_full_list; > uint32_t num_pend_cmds; > uint32_t num_qfull_cmds_alloc; > diff --git a/drivers/scsi/qla2xxx/qla_dfs.c b/drivers/scsi/qla2xxx/qla_dfs.c > index cd8b96a..34272fd 100644 > --- a/drivers/scsi/qla2xxx/qla_dfs.c > +++ b/drivers/scsi/qla2xxx/qla_dfs.c > @@ -13,6 +13,47 @@ static struct dentry *qla2x00_dfs_root; > static atomic_t qla2x00_dfs_root_count; > > static int > +qla2x00_dfs_tgt_sess_show(struct seq_file *s, void *unused) > +{ > + scsi_qla_host_t *vha = s->private; > + struct qla_hw_data *ha = vha->hw; > + unsigned long flags; > + struct qla_tgt_sess *sess = NULL; > + struct qla_tgt *tgt= vha->vha_tgt.qla_tgt; > + > + seq_printf(s, "%s\n",vha->host_str); > + if (tgt) { > + seq_printf(s, "Port ID Port Name Handle\n"); > + > + spin_lock_irqsave(&ha->tgt.sess_lock, flags); > + list_for_each_entry(sess, &tgt->sess_list, sess_list_entry) { > + seq_printf(s, "%02x:%02x:%02x %8phC %d\n", > + sess->s_id.b.domain,sess->s_id.b.area, > + sess->s_id.b.al_pa, sess->port_name, > + sess->loop_id); > + } > + spin_unlock_irqrestore(&ha->tgt.sess_lock, flags); > + } > + > + return 0; > +} > + > +static int > +qla2x00_dfs_tgt_sess_open(struct inode *inode, struct file *file) > +{ > + scsi_qla_host_t *vha = inode->i_private; > + return single_open(file, qla2x00_dfs_tgt_sess_show, vha); > +} > + > + > +static const struct file_operations dfs_tgt_sess_ops = { > + .open = qla2x00_dfs_tgt_sess_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > +}; > + > +static int > qla_dfs_fw_resource_cnt_show(struct seq_file *s, void *unused) > { > struct scsi_qla_host *vha = s->private; > @@ -248,6 +289,15 @@ create_nodes: > "Unable to create debugfs fce node.\n"); > goto out; > } > + > + ha->tgt.dfs_tgt_sess = debugfs_create_file("tgt_sess", > + S_IRUSR, ha->dfs_dir, vha, &dfs_tgt_sess_ops); > + if (!ha->tgt.dfs_tgt_sess) { > + ql_log(ql_log_warn, vha, 0xffff, > + "Unable to create debugFS tgt_sess node.\n"); > + goto out; > + } > + > out: > return 0; > } > @@ -257,6 +307,11 @@ qla2x00_dfs_remove(scsi_qla_host_t *vha) > { > struct qla_hw_data *ha = vha->hw; > > + if (ha->tgt.dfs_tgt_sess) { > + debugfs_remove(ha->tgt.dfs_tgt_sess); > + ha->tgt.dfs_tgt_sess = NULL; > + } > + > if (ha->dfs_fw_resource_cnt) { > debugfs_remove(ha->dfs_fw_resource_cnt); > ha->dfs_fw_resource_cnt = NULL; > diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c > index 46c6679..a754aa4 100644 > --- a/drivers/scsi/qla2xxx/qla_target.c > +++ b/drivers/scsi/qla2xxx/qla_target.c > @@ -641,7 +641,8 @@ void qlt_unreg_sess(struct qla_tgt_sess *sess) > { > struct scsi_qla_host *vha = sess->vha; > > - vha->hw->tgt.tgt_ops->clear_nacl_from_fcport_map(sess); > + if (sess->se_sess) > + vha->hw->tgt.tgt_ops->clear_nacl_from_fcport_map(sess); > > if (!list_empty(&sess->del_list_entry)) > list_del_init(&sess->del_list_entry); > @@ -856,8 +857,12 @@ static void qlt_del_sess_work_fn(struct delayed_work *work) > ql_dbg(ql_dbg_tgt_mgt, vha, 0xf004, > "Timeout: sess %p about to be deleted\n", > sess); > - ha->tgt.tgt_ops->shutdown_sess(sess); > - ha->tgt.tgt_ops->put_sess(sess); > + if (sess->se_sess) { > + ha->tgt.tgt_ops->shutdown_sess(sess); > + ha->tgt.tgt_ops->put_sess(sess); > + } else { > + qlt_unreg_sess(sess); > + } > } else { > schedule_delayed_work(&tgt->sess_del_work, > sess->expires - elapsed); > @@ -905,6 +910,19 @@ static struct qla_tgt_sess *qlt_create_sess( > if (sess->deleted) > qlt_undelete_sess(sess); > > + if (!sess->se_sess) { > + be_sid[0] = sess->s_id.b.domain; > + be_sid[1] = sess->s_id.b.area; > + be_sid[2] = sess->s_id.b.al_pa; > + > + if (ha->tgt.tgt_ops->check_initiator_node_acl > + (vha, &sess->port_name[0], sess, &be_sid[0], > + sess->loop_id) < 0) { > + spin_unlock_irqrestore(&ha->tgt.sess_lock, flags); > + return NULL; > + } > + } > + > kref_get(&sess->se_sess->sess_kref); > ha->tgt.tgt_ops->update_sess(sess, fcport->d_id, fcport->loop_id, > (fcport->flags & FCF_CONF_COMP_SUPPORTED)); > @@ -951,22 +969,6 @@ static struct qla_tgt_sess *qlt_create_sess( > be_sid[0] = sess->s_id.b.domain; > be_sid[1] = sess->s_id.b.area; > be_sid[2] = sess->s_id.b.al_pa; > - /* > - * Determine if this fc_port->port_name is allowed to access > - * target mode using explict NodeACLs+MappedLUNs, or using > - * TPG demo mode. If this is successful a target mode FC nexus > - * is created. > - */ > - if (ha->tgt.tgt_ops->check_initiator_node_acl(vha, > - &fcport->port_name[0], sess, &be_sid[0], fcport->loop_id) < 0) { > - kfree(sess); > - return NULL; > - } > - /* > - * Take an extra reference to ->sess_kref here to handle qla_tgt_sess > - * access across ->tgt.sess_lock reaquire. > - */ > - kref_get(&sess->se_sess->sess_kref); > > sess->conf_compl_supported = (fcport->flags & FCF_CONF_COMP_SUPPORTED); > BUILD_BUG_ON(sizeof(sess->port_name) != sizeof(fcport->port_name)); > @@ -985,6 +987,22 @@ static struct qla_tgt_sess *qlt_create_sess( > fcport->loop_id, sess->s_id.b.domain, sess->s_id.b.area, > sess->s_id.b.al_pa, sess->conf_compl_supported ? "" : "not "); > > + /* > + * Determine if this fc_port->port_name is allowed to access > + * target mode using explict NodeACLs+MappedLUNs, or using > + * TPG demo mode. If this is successful a target mode FC nexus > + * is created. > + */ > + if (ha->tgt.tgt_ops->check_initiator_node_acl(vha, > + &fcport->port_name[0], sess, &be_sid[0], fcport->loop_id) < 0) > + return NULL; > + else > + /* > + * Take an extra reference to ->sess_kref here to handle qla_tgt_sess > + * access across ->tgt.sess_lock reaquire. > + */ > + kref_get(&sess->se_sess->sess_kref); > + > return sess; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2016-02-04 at 10:16 -0800, Bart Van Assche wrote: > On 02/04/2016 08:45 AM, Himanshu Madhani wrote: > > From: Quinn Tran <quinn.tran@qlogic.com> > > > > #cat /sys/kernel/debug/qla2xxx/qla2xxx_31/tgt_sess > > qla2xxx_31 > > Port ID Port Name Handle > > ff:fc:01 21:fd:00:05:33:c7:ec:16 0 > > 01:0e:00 21:00:00:24:ff:7b:8a:e4 1 > > 01:0f:00 21:00:00:24:ff:7b:8a:e5 2 > > .... > > Hello Quinn and Himanshu, > > The above information is not only useful to people who are debugging the > QLogic target driver but also to end users who want to check which > initiator ports have already logged in to a target port. Hence my > proposal to move this information from debugfs to another location (e.g. > configfs or sysfs). Users of other target drivers are probably also > interested in seeing which sessions are active. We already have that. For explicit NodeACLS, see: /sys/kernel/config/target/$FABRIC/$T_WWPN/$TPGT/acls/$I_WWPN/info and for demo-mode dynamically generated se_node_acl, see: /sys/kernel/config/target/$FABRIC/$T_WWPN/$TPGT/dynamic_sessions -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 2016-02-06 at 20:40 -0800, Nicholas A. Bellinger wrote: > Hi Himanshu & Quinn, > > On Thu, 2016-02-04 at 11:45 -0500, Himanshu Madhani wrote: > > From: Quinn Tran <quinn.tran@qlogic.com> > > > > #cat /sys/kernel/debug/qla2xxx/qla2xxx_31/tgt_sess > > qla2xxx_31 > > Port ID Port Name Handle > > ff:fc:01 21:fd:00:05:33:c7:ec:16 0 > > 01:0e:00 21:00:00:24:ff:7b:8a:e4 1 > > 01:0f:00 21:00:00:24:ff:7b:8a:e5 2 > > .... > > > > Signed-off-by: Quinn Tran <quinn.tran@qlogic.com> > > Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com> > > --- > > drivers/scsi/qla2xxx/qla_def.h | 1 + > > drivers/scsi/qla2xxx/qla_dfs.c | 55 ++++++++++++++++++++++++++++++++++++ > > drivers/scsi/qla2xxx/qla_target.c | 56 ++++++++++++++++++++++++------------ > > 3 files changed, 93 insertions(+), 19 deletions(-) > > > > So looking at this patch beyond the debugfs part, it does change where > ->check_initiator_node_acl() gets call during qlt_create_sess(). > > I assume this is related to new debugfs attribute, and these changes > (plus others in qlt_del_sess_work_fn) are not bug-fixes on their own, > correct..? > > Aside from that, I don't have an objection to merge as v4.6 for-next > code if QLogic finds it useful for debugging. > Btw, this patch has a conflict with target-pending/queue-next wrt to removal of be_sid + loopid from tgt_ops->check_initiator_node_acl(), which is part of the target_alloc_session() conversion for v4.6. Here's the updated version that's been applied to queue-next: https://git.kernel.org/cgit/linux/kernel/git/nab/target-pending.git/commit/?h=queue-next&id=cfd527d9db57ceac8a1e211b77c3357259df48cc Please verify that it looks + works as expected. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Nic, On 2/6/16, 8:40 PM, "Nicholas A. Bellinger" <nab@linux-iscsi.org> wrote: >Hi Himanshu & Quinn, > >On Thu, 2016-02-04 at 11:45 -0500, Himanshu Madhani wrote: >> From: Quinn Tran <quinn.tran@qlogic.com> >> >> #cat /sys/kernel/debug/qla2xxx/qla2xxx_31/tgt_sess >> qla2xxx_31 >> Port ID Port Name Handle >> ff:fc:01 21:fd:00:05:33:c7:ec:16 0 >> 01:0e:00 21:00:00:24:ff:7b:8a:e4 1 >> 01:0f:00 21:00:00:24:ff:7b:8a:e5 2 >> .... >> >> Signed-off-by: Quinn Tran <quinn.tran@qlogic.com> >> Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com> >> --- >> drivers/scsi/qla2xxx/qla_def.h | 1 + >> drivers/scsi/qla2xxx/qla_dfs.c | 55 >>++++++++++++++++++++++++++++++++++++ >> drivers/scsi/qla2xxx/qla_target.c | 56 >>++++++++++++++++++++++++------------ >> 3 files changed, 93 insertions(+), 19 deletions(-) >> > >So looking at this patch beyond the debugfs part, it does change where >->check_initiator_node_acl() gets call during qlt_create_sess(). > >I assume this is related to new debugfs attribute, and these changes >(plus others in qlt_del_sess_work_fn) are not bug-fixes on their own, >correct..? > >Aside from that, I don't have an objection to merge as v4.6 for-next >code if QLogic finds it useful for debugging. Yes. The changes in this patch are strictly for making information available via debugFS and not bug-fixes on their own. This information has been requested by few customer who finds this information useful for debugging and in addition they use this information in their configuration scripts as well. > > > >> diff --git a/drivers/scsi/qla2xxx/qla_def.h >>b/drivers/scsi/qla2xxx/qla_def.h >> index 9872f34..e6c5bcf 100644 >> --- a/drivers/scsi/qla2xxx/qla_def.h >> +++ b/drivers/scsi/qla2xxx/qla_def.h >> @@ -2929,6 +2929,7 @@ struct qlt_hw_data { >> >> uint8_t tgt_node_name[WWN_SIZE]; >> >> + struct dentry *dfs_tgt_sess; >> struct list_head q_full_list; >> uint32_t num_pend_cmds; >> uint32_t num_qfull_cmds_alloc; >> diff --git a/drivers/scsi/qla2xxx/qla_dfs.c >>b/drivers/scsi/qla2xxx/qla_dfs.c >> index cd8b96a..34272fd 100644 >> --- a/drivers/scsi/qla2xxx/qla_dfs.c >> +++ b/drivers/scsi/qla2xxx/qla_dfs.c >> @@ -13,6 +13,47 @@ static struct dentry *qla2x00_dfs_root; >> static atomic_t qla2x00_dfs_root_count; >> >> static int >> +qla2x00_dfs_tgt_sess_show(struct seq_file *s, void *unused) >> +{ >> + scsi_qla_host_t *vha = s->private; >> + struct qla_hw_data *ha = vha->hw; >> + unsigned long flags; >> + struct qla_tgt_sess *sess = NULL; >> + struct qla_tgt *tgt= vha->vha_tgt.qla_tgt; >> + >> + seq_printf(s, "%s\n",vha->host_str); >> + if (tgt) { >> + seq_printf(s, "Port ID Port Name Handle\n"); >> + >> + spin_lock_irqsave(&ha->tgt.sess_lock, flags); >> + list_for_each_entry(sess, &tgt->sess_list, sess_list_entry) { >> + seq_printf(s, "%02x:%02x:%02x %8phC %d\n", >> + sess->s_id.b.domain,sess->s_id.b.area, >> + sess->s_id.b.al_pa, sess->port_name, >> + sess->loop_id); >> + } >> + spin_unlock_irqrestore(&ha->tgt.sess_lock, flags); >> + } >> + >> + return 0; >> +} >> + >> +static int >> +qla2x00_dfs_tgt_sess_open(struct inode *inode, struct file *file) >> +{ >> + scsi_qla_host_t *vha = inode->i_private; >> + return single_open(file, qla2x00_dfs_tgt_sess_show, vha); >> +} >> + >> + >> +static const struct file_operations dfs_tgt_sess_ops = { >> + .open = qla2x00_dfs_tgt_sess_open, >> + .read = seq_read, >> + .llseek = seq_lseek, >> + .release = single_release, >> +}; >> + >> +static int >> qla_dfs_fw_resource_cnt_show(struct seq_file *s, void *unused) >> { >> struct scsi_qla_host *vha = s->private; >> @@ -248,6 +289,15 @@ create_nodes: >> "Unable to create debugfs fce node.\n"); >> goto out; >> } >> + >> + ha->tgt.dfs_tgt_sess = debugfs_create_file("tgt_sess", >> + S_IRUSR, ha->dfs_dir, vha, &dfs_tgt_sess_ops); >> + if (!ha->tgt.dfs_tgt_sess) { >> + ql_log(ql_log_warn, vha, 0xffff, >> + "Unable to create debugFS tgt_sess node.\n"); >> + goto out; >> + } >> + >> out: >> return 0; >> } >> @@ -257,6 +307,11 @@ qla2x00_dfs_remove(scsi_qla_host_t *vha) >> { >> struct qla_hw_data *ha = vha->hw; >> >> + if (ha->tgt.dfs_tgt_sess) { >> + debugfs_remove(ha->tgt.dfs_tgt_sess); >> + ha->tgt.dfs_tgt_sess = NULL; >> + } >> + >> if (ha->dfs_fw_resource_cnt) { >> debugfs_remove(ha->dfs_fw_resource_cnt); >> ha->dfs_fw_resource_cnt = NULL; >> diff --git a/drivers/scsi/qla2xxx/qla_target.c >>b/drivers/scsi/qla2xxx/qla_target.c >> index 46c6679..a754aa4 100644 >> --- a/drivers/scsi/qla2xxx/qla_target.c >> +++ b/drivers/scsi/qla2xxx/qla_target.c >> @@ -641,7 +641,8 @@ void qlt_unreg_sess(struct qla_tgt_sess *sess) >> { >> struct scsi_qla_host *vha = sess->vha; >> >> - vha->hw->tgt.tgt_ops->clear_nacl_from_fcport_map(sess); >> + if (sess->se_sess) >> + vha->hw->tgt.tgt_ops->clear_nacl_from_fcport_map(sess); >> >> if (!list_empty(&sess->del_list_entry)) >> list_del_init(&sess->del_list_entry); >> @@ -856,8 +857,12 @@ static void qlt_del_sess_work_fn(struct >>delayed_work *work) >> ql_dbg(ql_dbg_tgt_mgt, vha, 0xf004, >> "Timeout: sess %p about to be deleted\n", >> sess); >> - ha->tgt.tgt_ops->shutdown_sess(sess); >> - ha->tgt.tgt_ops->put_sess(sess); >> + if (sess->se_sess) { >> + ha->tgt.tgt_ops->shutdown_sess(sess); >> + ha->tgt.tgt_ops->put_sess(sess); >> + } else { >> + qlt_unreg_sess(sess); >> + } >> } else { >> schedule_delayed_work(&tgt->sess_del_work, >> sess->expires - elapsed); >> @@ -905,6 +910,19 @@ static struct qla_tgt_sess *qlt_create_sess( >> if (sess->deleted) >> qlt_undelete_sess(sess); >> >> + if (!sess->se_sess) { >> + be_sid[0] = sess->s_id.b.domain; >> + be_sid[1] = sess->s_id.b.area; >> + be_sid[2] = sess->s_id.b.al_pa; >> + >> + if (ha->tgt.tgt_ops->check_initiator_node_acl >> + (vha, &sess->port_name[0], sess, &be_sid[0], >> + sess->loop_id) < 0) { >> + spin_unlock_irqrestore(&ha->tgt.sess_lock, flags); >> + return NULL; >> + } >> + } >> + >> kref_get(&sess->se_sess->sess_kref); >> ha->tgt.tgt_ops->update_sess(sess, fcport->d_id, fcport->loop_id, >> (fcport->flags & FCF_CONF_COMP_SUPPORTED)); >> @@ -951,22 +969,6 @@ static struct qla_tgt_sess *qlt_create_sess( >> be_sid[0] = sess->s_id.b.domain; >> be_sid[1] = sess->s_id.b.area; >> be_sid[2] = sess->s_id.b.al_pa; >> - /* >> - * Determine if this fc_port->port_name is allowed to access >> - * target mode using explict NodeACLs+MappedLUNs, or using >> - * TPG demo mode. If this is successful a target mode FC nexus >> - * is created. >> - */ >> - if (ha->tgt.tgt_ops->check_initiator_node_acl(vha, >> - &fcport->port_name[0], sess, &be_sid[0], fcport->loop_id) < 0) { >> - kfree(sess); >> - return NULL; >> - } >> - /* >> - * Take an extra reference to ->sess_kref here to handle qla_tgt_sess >> - * access across ->tgt.sess_lock reaquire. >> - */ >> - kref_get(&sess->se_sess->sess_kref); >> >> sess->conf_compl_supported = (fcport->flags & >>FCF_CONF_COMP_SUPPORTED); >> BUILD_BUG_ON(sizeof(sess->port_name) != sizeof(fcport->port_name)); >> @@ -985,6 +987,22 @@ static struct qla_tgt_sess *qlt_create_sess( >> fcport->loop_id, sess->s_id.b.domain, sess->s_id.b.area, >> sess->s_id.b.al_pa, sess->conf_compl_supported ? "" : "not "); >> >> + /* >> + * Determine if this fc_port->port_name is allowed to access >> + * target mode using explict NodeACLs+MappedLUNs, or using >> + * TPG demo mode. If this is successful a target mode FC nexus >> + * is created. >> + */ >> + if (ha->tgt.tgt_ops->check_initiator_node_acl(vha, >> + &fcport->port_name[0], sess, &be_sid[0], fcport->loop_id) < 0) >> + return NULL; >> + else >> + /* >> + * Take an extra reference to ->sess_kref here to handle qla_tgt_sess >> + * access across ->tgt.sess_lock reaquire. >> + */ >> + kref_get(&sess->se_sess->sess_kref); >> + >> return sess; >> } >> > >
On Mon, 2016-02-08 at 17:43 +0000, Himanshu Madhani wrote: > > > >So looking at this patch beyond the debugfs part, it does change where > >->check_initiator_node_acl() gets call during qlt_create_sess(). > > > >I assume this is related to new debugfs attribute, and these changes > >(plus others in qlt_del_sess_work_fn) are not bug-fixes on their own, > >correct..? > > > >Aside from that, I don't have an objection to merge as v4.6 for-next > >code if QLogic finds it useful for debugging. > > Yes. The changes in this patch are strictly for making information > available via debugFS and not bug-fixes on their own. Thanks for clarifying. > This information has been requested by few customer who > finds this information useful for debugging and in addition they use this > information in their configuration scripts as well. So I've got no particular objection to people adding debugfs informational attributes for whatever to aid debugging of their drivers. That said, the same information can too be exposed as a tcm_qla2xxx struct se_node_acl->acl_group configfs attribute, and for demo-mode sessions via a tcm_qla2xxx provided dynamic_sessions configfs attribute handler. I'd be happy to add expose this under tcm_qla2xxx configfs for v4.6 code if you'd like, otherwise I'm OK with the patch in queue-next as-is if you find it useful. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2/6/16, 9:00 PM, "Nicholas A. Bellinger" <nab@linux-iscsi.org> wrote: >On Sat, 2016-02-06 at 20:40 -0800, Nicholas A. Bellinger wrote: >> Hi Himanshu & Quinn, >> >> On Thu, 2016-02-04 at 11:45 -0500, Himanshu Madhani wrote: >> > From: Quinn Tran <quinn.tran@qlogic.com> >> > >> > #cat /sys/kernel/debug/qla2xxx/qla2xxx_31/tgt_sess >> > qla2xxx_31 >> > Port ID Port Name Handle >> > ff:fc:01 21:fd:00:05:33:c7:ec:16 0 >> > 01:0e:00 21:00:00:24:ff:7b:8a:e4 1 >> > 01:0f:00 21:00:00:24:ff:7b:8a:e5 2 >> > .... >> > >> > Signed-off-by: Quinn Tran <quinn.tran@qlogic.com> >> > Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com> >> > --- >> > drivers/scsi/qla2xxx/qla_def.h | 1 + >> > drivers/scsi/qla2xxx/qla_dfs.c | 55 >>++++++++++++++++++++++++++++++++++++ >> > drivers/scsi/qla2xxx/qla_target.c | 56 >>++++++++++++++++++++++++------------ >> > 3 files changed, 93 insertions(+), 19 deletions(-) >> > >> >> So looking at this patch beyond the debugfs part, it does change where >> ->check_initiator_node_acl() gets call during qlt_create_sess(). >> >> I assume this is related to new debugfs attribute, and these changes >> (plus others in qlt_del_sess_work_fn) are not bug-fixes on their own, >> correct..? >> >> Aside from that, I don't have an objection to merge as v4.6 for-next >> code if QLogic finds it useful for debugging. >> > >Btw, this patch has a conflict with target-pending/queue-next wrt to >removal of be_sid + loopid from tgt_ops->check_initiator_node_acl(), >which is part of the target_alloc_session() conversion for v4.6. > >Here's the updated version that's been applied to queue-next: > >https://git.kernel.org/cgit/linux/kernel/git/nab/target-pending.git/commit >/?h=queue-next&id=cfd527d9db57ceac8a1e211b77c3357259df48cc > >Please verify that it looks + works as expected. > This Patch looks good. Tested successfully on my setup. >
On 2/8/16, 9:49 PM, "Nicholas A. Bellinger" <nab@linux-iscsi.org> wrote: >On Mon, 2016-02-08 at 17:43 +0000, Himanshu Madhani wrote: >> > >> >So looking at this patch beyond the debugfs part, it does change where >> >->check_initiator_node_acl() gets call during qlt_create_sess(). >> > >> >I assume this is related to new debugfs attribute, and these changes >> >(plus others in qlt_del_sess_work_fn) are not bug-fixes on their own, >> >correct..? >> > >> >Aside from that, I don't have an objection to merge as v4.6 for-next >> >code if QLogic finds it useful for debugging. >> >> Yes. The changes in this patch are strictly for making information >> available via debugFS and not bug-fixes on their own. > >Thanks for clarifying. > >> This information has been requested by few customer who >> finds this information useful for debugging and in addition they use >>this >> information in their configuration scripts as well. > >So I've got no particular objection to people adding debugfs >informational attributes for whatever to aid debugging of their drivers. > >That said, the same information can too be exposed as a tcm_qla2xxx >struct se_node_acl->acl_group configfs attribute, and for demo-mode >sessions via a tcm_qla2xxx provided dynamic_sessions configfs attribute >handler. > >I'd be happy to add expose this under tcm_qla2xxx configfs for v4.6 code >if you'd like, otherwise I'm OK with the patch in queue-next as-is if >you find it useful. > I really like your idea of exposing this information via configFS. Please go ahead and make updates on exposing this information via se_node_acl->acl_group Configfs attribute. We can ask our customer to explore configfs to retrieve information. However, We would also like to keep this information in debugFS for now. >
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index 9872f34..e6c5bcf 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -2929,6 +2929,7 @@ struct qlt_hw_data { uint8_t tgt_node_name[WWN_SIZE]; + struct dentry *dfs_tgt_sess; struct list_head q_full_list; uint32_t num_pend_cmds; uint32_t num_qfull_cmds_alloc; diff --git a/drivers/scsi/qla2xxx/qla_dfs.c b/drivers/scsi/qla2xxx/qla_dfs.c index cd8b96a..34272fd 100644 --- a/drivers/scsi/qla2xxx/qla_dfs.c +++ b/drivers/scsi/qla2xxx/qla_dfs.c @@ -13,6 +13,47 @@ static struct dentry *qla2x00_dfs_root; static atomic_t qla2x00_dfs_root_count; static int +qla2x00_dfs_tgt_sess_show(struct seq_file *s, void *unused) +{ + scsi_qla_host_t *vha = s->private; + struct qla_hw_data *ha = vha->hw; + unsigned long flags; + struct qla_tgt_sess *sess = NULL; + struct qla_tgt *tgt= vha->vha_tgt.qla_tgt; + + seq_printf(s, "%s\n",vha->host_str); + if (tgt) { + seq_printf(s, "Port ID Port Name Handle\n"); + + spin_lock_irqsave(&ha->tgt.sess_lock, flags); + list_for_each_entry(sess, &tgt->sess_list, sess_list_entry) { + seq_printf(s, "%02x:%02x:%02x %8phC %d\n", + sess->s_id.b.domain,sess->s_id.b.area, + sess->s_id.b.al_pa, sess->port_name, + sess->loop_id); + } + spin_unlock_irqrestore(&ha->tgt.sess_lock, flags); + } + + return 0; +} + +static int +qla2x00_dfs_tgt_sess_open(struct inode *inode, struct file *file) +{ + scsi_qla_host_t *vha = inode->i_private; + return single_open(file, qla2x00_dfs_tgt_sess_show, vha); +} + + +static const struct file_operations dfs_tgt_sess_ops = { + .open = qla2x00_dfs_tgt_sess_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; + +static int qla_dfs_fw_resource_cnt_show(struct seq_file *s, void *unused) { struct scsi_qla_host *vha = s->private; @@ -248,6 +289,15 @@ create_nodes: "Unable to create debugfs fce node.\n"); goto out; } + + ha->tgt.dfs_tgt_sess = debugfs_create_file("tgt_sess", + S_IRUSR, ha->dfs_dir, vha, &dfs_tgt_sess_ops); + if (!ha->tgt.dfs_tgt_sess) { + ql_log(ql_log_warn, vha, 0xffff, + "Unable to create debugFS tgt_sess node.\n"); + goto out; + } + out: return 0; } @@ -257,6 +307,11 @@ qla2x00_dfs_remove(scsi_qla_host_t *vha) { struct qla_hw_data *ha = vha->hw; + if (ha->tgt.dfs_tgt_sess) { + debugfs_remove(ha->tgt.dfs_tgt_sess); + ha->tgt.dfs_tgt_sess = NULL; + } + if (ha->dfs_fw_resource_cnt) { debugfs_remove(ha->dfs_fw_resource_cnt); ha->dfs_fw_resource_cnt = NULL; diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index 46c6679..a754aa4 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -641,7 +641,8 @@ void qlt_unreg_sess(struct qla_tgt_sess *sess) { struct scsi_qla_host *vha = sess->vha; - vha->hw->tgt.tgt_ops->clear_nacl_from_fcport_map(sess); + if (sess->se_sess) + vha->hw->tgt.tgt_ops->clear_nacl_from_fcport_map(sess); if (!list_empty(&sess->del_list_entry)) list_del_init(&sess->del_list_entry); @@ -856,8 +857,12 @@ static void qlt_del_sess_work_fn(struct delayed_work *work) ql_dbg(ql_dbg_tgt_mgt, vha, 0xf004, "Timeout: sess %p about to be deleted\n", sess); - ha->tgt.tgt_ops->shutdown_sess(sess); - ha->tgt.tgt_ops->put_sess(sess); + if (sess->se_sess) { + ha->tgt.tgt_ops->shutdown_sess(sess); + ha->tgt.tgt_ops->put_sess(sess); + } else { + qlt_unreg_sess(sess); + } } else { schedule_delayed_work(&tgt->sess_del_work, sess->expires - elapsed); @@ -905,6 +910,19 @@ static struct qla_tgt_sess *qlt_create_sess( if (sess->deleted) qlt_undelete_sess(sess); + if (!sess->se_sess) { + be_sid[0] = sess->s_id.b.domain; + be_sid[1] = sess->s_id.b.area; + be_sid[2] = sess->s_id.b.al_pa; + + if (ha->tgt.tgt_ops->check_initiator_node_acl + (vha, &sess->port_name[0], sess, &be_sid[0], + sess->loop_id) < 0) { + spin_unlock_irqrestore(&ha->tgt.sess_lock, flags); + return NULL; + } + } + kref_get(&sess->se_sess->sess_kref); ha->tgt.tgt_ops->update_sess(sess, fcport->d_id, fcport->loop_id, (fcport->flags & FCF_CONF_COMP_SUPPORTED)); @@ -951,22 +969,6 @@ static struct qla_tgt_sess *qlt_create_sess( be_sid[0] = sess->s_id.b.domain; be_sid[1] = sess->s_id.b.area; be_sid[2] = sess->s_id.b.al_pa; - /* - * Determine if this fc_port->port_name is allowed to access - * target mode using explict NodeACLs+MappedLUNs, or using - * TPG demo mode. If this is successful a target mode FC nexus - * is created. - */ - if (ha->tgt.tgt_ops->check_initiator_node_acl(vha, - &fcport->port_name[0], sess, &be_sid[0], fcport->loop_id) < 0) { - kfree(sess); - return NULL; - } - /* - * Take an extra reference to ->sess_kref here to handle qla_tgt_sess - * access across ->tgt.sess_lock reaquire. - */ - kref_get(&sess->se_sess->sess_kref); sess->conf_compl_supported = (fcport->flags & FCF_CONF_COMP_SUPPORTED); BUILD_BUG_ON(sizeof(sess->port_name) != sizeof(fcport->port_name)); @@ -985,6 +987,22 @@ static struct qla_tgt_sess *qlt_create_sess( fcport->loop_id, sess->s_id.b.domain, sess->s_id.b.area, sess->s_id.b.al_pa, sess->conf_compl_supported ? "" : "not "); + /* + * Determine if this fc_port->port_name is allowed to access + * target mode using explict NodeACLs+MappedLUNs, or using + * TPG demo mode. If this is successful a target mode FC nexus + * is created. + */ + if (ha->tgt.tgt_ops->check_initiator_node_acl(vha, + &fcport->port_name[0], sess, &be_sid[0], fcport->loop_id) < 0) + return NULL; + else + /* + * Take an extra reference to ->sess_kref here to handle qla_tgt_sess + * access across ->tgt.sess_lock reaquire. + */ + kref_get(&sess->se_sess->sess_kref); + return sess; }