| Message ID | 1591562164-9766-12-git-send-email-michael.christie@oracle.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | target: add sysfs support | expand |
On Sun, Jun 07, 2020 at 03:35:58PM -0500, Mike Christie wrote: > +int target_sysfs_add_session(struct se_portal_group *se_tpg, > + struct se_session *se_sess) > +{ > + struct t10_transport_id *tpt_id = se_sess->tpt_id; > + int ret; > + > + if (!try_module_get(se_sess->tfo->module)) > + return -EINVAL; > + > + ret = target_cp_endpoint_strs(se_tpg, se_sess); > + if (ret) > + goto put_mod; > + > + se_sess->dev.groups = se_sess->tfo->session_attr_groups; > + ret = device_add(&se_sess->dev); > + if (ret) { > + pr_err("Could not add session%d to sysfs. Error %d.\n", > + se_sess->sid, ret); > + goto free_ep_strs; > + } > + > + pr_info("TCM added session-%d from [fabric: %s, target: %s, tpg %s, acl: %s] to [initiator port: %s%s%s]", > + se_sess->sid, se_sess->fabric_name, se_sess->target_name, > + se_sess->tpg_name, se_sess->acl_name ? se_sess->acl_name : "dynamic", > + tpt_id->name, tpt_id->session_id ? "," : "", > + tpt_id->session_id ? tpt_id->session_id : ""); You have a 'struct device', so please use it, no need for pr_info(), always use the dev_*() calls instead. but, when drivers and kernel code is all working properly, no need to be noisy at all, this should just be a dev_dbg() call, right? > + > + se_sess->sysfs_added = true; > + return 0; > + > +free_ep_strs: > + target_free_endpoint_strs(se_sess); > +put_mod: > + module_put(se_sess->tfo->module); > + return ret; > +} > +EXPORT_SYMBOL(target_sysfs_add_session); I have to ask, EXPORT_SYMBOL_GPL()? > + > +void target_sysfs_remove_session(struct se_session *se_sess) > +{ > + struct t10_transport_id *tpt_id = se_sess->tpt_id; > + > + /* discovery sessions are normally not added to sysfs */ > + if (!se_sess->sysfs_added) > + return; > + se_sess->sysfs_added = false; > + > + pr_info("TCM removed session-%d from [fabric: %s, target: %s, tpg %s, acl: %s] to [initiator port: %s%s%s]", > + se_sess->sid, se_sess->fabric_name, se_sess->target_name, > + se_sess->tpg_name, se_sess->acl_name ? se_sess->acl_name : "dynamic", > + tpt_id->name, tpt_id->session_id ? "," : "", > + tpt_id->session_id ? tpt_id->session_id : ""); dev_dbg()? > + > + device_del(&se_sess->dev); > +} > +EXPORT_SYMBOL(target_sysfs_remove_session); EXPORT_SYMBOL_GPL()? > + > +void target_sysfs_init(void) > +{ > + class_register(&session_class); Why not: return class_register(&session_class); you lost the return value of that call :( Other than those minor things, looks good, nice job! greg k-h
On 6/7/20 10:35 PM, Mike Christie wrote: > This patch adds a sysfs interface to export the target module's > sessions using the driver module class structure. With this we > now have a common way to export session info for all fabric modules > and we can export info about all sessions not just the first one > on a ACL like for iscsi. We can also export more info then PAGE_SIZE > bytes for all sessions (looking at iscsi and qla2xxx and the > dynamic_sessions file abuse). > > Here is a tree the new class dir scsi_target_session: > > session-1/ > ├── power > │ ├── autosuspend_delay_ms > │ ├── control > │ ├── runtime_active_time > │ ├── runtime_status > │ └── runtime_suspended_time > ├── subsystem -> ../../../../class/scsi_target_session > ├── tcm_endpoint > │ ├── acl > │ ├── fabric > │ ├── target > │ └── tpg > ├── transport_id > │ ├── name > │ └── proto > └── uevent > > The Documentation/ABI/testing/sysfs-scsi-target-session file in this > patch describes the files and dirs. > > Userspace apps like tcmu-runner and targetcli, can either: > > 1. If they have the session ID, then directly look up the session's > info like with tagetcli's session sid case. > > 2. If they have the target side's endpoint object name like the ACL > or tpg, then they have to scan each session's tcm_endpoint dir. This > should hopefully not be the normal case. For tcmu-runner we will > normally scan sysfs, cache the info, then update the cache as we get > events about sessions being added/removed. > > Note that to make it easier for Greg to review I am just including > the sysfs parts in this patch. The next patch hooks into the target > code and enables it. > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Mike Christie <michael.christie@oracle.com> > --- > .../ABI/testing/sysfs-scsi-target-session | 71 ++++++ > drivers/target/target_core_internal.h | 5 + > drivers/target/target_core_sysfs.c | 265 +++++++++++++++++++++ > include/target/target_core_base.h | 8 + > include/target/target_core_fabric.h | 6 +- > 5 files changed, 354 insertions(+), 1 deletion(-) > create mode 100644 Documentation/ABI/testing/sysfs-scsi-target-session > create mode 100644 drivers/target/target_core_sysfs.c > > diff --git a/Documentation/ABI/testing/sysfs-scsi-target-session b/Documentation/ABI/testing/sysfs-scsi-target-session > new file mode 100644 > index 0000000..c8cefcc > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-scsi-target-session > @@ -0,0 +1,71 @@ > +What: /sys/class/scsi_target_session/session-N > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: The session dir contains a dir for each I_T Nexus. The name of > + the dir is "session-" followed by an integer that is unique > + across all fabrics. The dir contains files that export info like > + the TPG/ACL the session is attached to, SCSI port values, and > + fabric and transport specific values. > + > +What: /sys/class/scsi_target_session/session-N/tcm_endpoint > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: The tcm_endpoint dir indicates what target_core_mod object > + the session is attached to. > + > +What: /sys/class/scsi_target_session/session-N/tcm_endpoint/acl > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: Returns the name of the ACL that was used to validate login or > + an empty string if no user created ACL was used. > + > +What: /sys/class/scsi_target_session/session-N/tcm_endpoint/target > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: Returns the name of the target the session is accessed through. > + > +What: /sys/class/scsi_target_session/session-N/tcm_endpoint/fabric > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: Returns the name of the fabric the session is accessed through. > + > +What: /sys/class/scsi_target_session/session-N/tcm_endpoint/tpg > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: Returns the name of the tpg the session is accessed through. > + > +What: /sys/class/scsi_target_session/session-N/transport_id > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: The transport_id contains the SCSI TransportID values for the > + initiator port as defined in SPC4r37. > + > +What: /sys/class/scsi_target_session/session-N/transport_id/proto > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: Returns the SCSI protocol identifier in hex defined in SPC4r37. > + > +What: /sys/class/scsi_target_session/session-N/transport_id/name > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: Returns the port name/address/id in the protocol specific > + TransportID format defined in SPC4r37's. > + > +What: /sys/class/scsi_target_session/session-N/transport_id/session_id > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: If is proto=0x5 (iSCSI) and TPID FORMAT=1 this file will exist > + and will return the iSCSI Initiator Session ID in ASCII > + characters that are the hexadecimal digits converted from the > + binary iSCSI initiator session identifier value. If iSCSI and > + format=1 is not used by this session this file will not exist. > diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h > index 8533444..93bf5fed 100644 > --- a/drivers/target/target_core_internal.h > +++ b/drivers/target/target_core_internal.h > @@ -105,6 +105,11 @@ int target_get_pr_transport_id(struct se_node_acl *nacl, > const char *target_parse_pr_out_transport_id(struct se_portal_group *tpg, > char *buf, u32 *out_tid_len, char **port_nexus_ptr); > > +/* target_core_sysfs.c */ > +int target_sysfs_init_session(struct se_session *sess); > +void target_sysfs_init(void); > +void target_sysfs_exit(void); > + > /* target_core_hba.c */ > struct se_hba *core_alloc_hba(const char *, u32, u32); > int core_delete_hba(struct se_hba *); > diff --git a/drivers/target/target_core_sysfs.c b/drivers/target/target_core_sysfs.c > new file mode 100644 > index 0000000..b29a6f2 > --- /dev/null > +++ b/drivers/target/target_core_sysfs.c > @@ -0,0 +1,265 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +#include <linux/device.h> > +#include <linux/idr.h> > +#include <linux/module.h> > + > +#include <scsi/scsi_proto.h> > + > +#include <target/target_core_base.h> > +#include <target/target_core_fabric.h> > +#include "target_core_internal.h" > + > +static DEFINE_IDA(session_ida); > + > +#define dev_to_se_sess(_dev) \ > + container_of(dev, struct se_session, dev) > + > +static void target_free_endpoint_strs(struct se_session *se_sess) > +{ > + kfree(se_sess->tpg_name); > + kfree(se_sess->fabric_name); > + kfree(se_sess->target_name); > + kfree(se_sess->acl_name); > +} > + > +static void target_sysfs_session_release(struct device *dev) > +{ > + struct se_session *se_sess = dev_to_se_sess(dev); > + > + target_free_endpoint_strs(se_sess); > + ida_simple_remove(&session_ida, se_sess->sid); > + > + __target_free_session(se_sess); > +} > + > +#define target_attr_show(field) \ > +static ssize_t show_target_##field(struct device *dev, \ > + struct device_attribute *attr, char *buf) \ > +{ \ > + struct se_session *se_sess = dev_to_se_sess(dev); \ > + \ > + if (!se_sess->field##_name) \ > + return 0; \ > + \ > + return snprintf(buf, PAGE_SIZE, "%s", se_sess->field##_name); \ > +} > + > +#define target_attr_str(field) \ > + target_attr_show(field) \ > +static DEVICE_ATTR(field, S_IRUGO, show_target_##field, NULL) > + > +target_attr_str(acl); > +target_attr_str(tpg); > +target_attr_str(target); > +target_attr_str(fabric); > + > +/* > + * attrs needed to reference the session's tcm endpoint (acl or tpg) in > + * configfs > + */ > +static struct attribute *target_endpoint_attrs[] = { > + &dev_attr_acl.attr, > + &dev_attr_tpg.attr, > + &dev_attr_target.attr, > + &dev_attr_fabric.attr, > + NULL, > +}; > + > +static const struct attribute_group target_endpoint_group = { > + .name = "tcm_endpoint", > + .attrs = target_endpoint_attrs, > +}; > + > +/* transportID attrs */ > +#define tpt_id_attr_show(name, fmt_str) \ > +static ssize_t show_tpid_##name(struct device *dev, \ > + struct device_attribute *attr, char *buf) \ > +{ \ > + struct se_session *se_sess = dev_to_se_sess(dev); \ > + return snprintf(buf, PAGE_SIZE, fmt_str, se_sess->tpt_id->name); \ > +} > + > +#define tpt_id_attr(name, fmt_str) \ > + tpt_id_attr_show(name, fmt_str) \ > +static DEVICE_ATTR(name, S_IRUGO, show_tpid_##name, NULL) > + > +tpt_id_attr(proto, "0x%x"); > +tpt_id_attr(name, "%s"); > + > +static ssize_t session_id_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct se_session *se_sess = dev_to_se_sess(dev); > + > + if (!se_sess->tpt_id->session_id) > + return 0; > + > + return snprintf(buf, PAGE_SIZE, "0x%s", se_sess->tpt_id->session_id); > +} > + > +static DEVICE_ATTR(session_id, S_IRUGO, session_id_show, NULL); > + > +static struct attribute *tpt_id_attrs[] = { > + &dev_attr_proto.attr, > + &dev_attr_name.attr, > + &dev_attr_session_id.attr, > + NULL, > +}; > + > +static const struct attribute_group tpt_id_group = { > + .name = "transport_id", > + .attrs = tpt_id_attrs, > +}; > + > +static const struct attribute_group *def_session_groups[] = { > + &tpt_id_group, > + &target_endpoint_group, > + NULL, > +}; > + > +static struct class session_class = { > + .owner = THIS_MODULE, > + .name = "scsi_target_session", > + .dev_release = target_sysfs_session_release, > + .dev_groups = def_session_groups, > +}; > + > +int target_sysfs_init_session(struct se_session *se_sess) > +{ > + int ret; > + > + ret = ida_simple_get(&session_ida, 1, 0, GFP_KERNEL); > + if (ret < 0) { > + pr_err("Unable to allocate session index.\n"); > + return ret; > + } > + se_sess->sid = ret; > + > + device_initialize(&se_sess->dev); > + se_sess->dev.class = &session_class; > + > + ret = dev_set_name(&se_sess->dev, "session-%d", se_sess->sid); > + if (ret) > + goto put_dev; > + > + return 0; > + > +put_dev: > + put_device(&se_sess->dev); > + return ret; > +} > + > +static int target_cp_endpoint_strs(struct se_portal_group *se_tpg, > + struct se_session *se_sess) > +{ > + /* > + * Copy configfs dir/object names so userspace can match the session > + * to its target, and we also don't have to worry about mixing configfs > + * refcounts with sysfs. > + */ > + if (!se_sess->se_node_acl->dynamic_node_acl) { > + se_sess->acl_name = kstrdup(se_sess->se_node_acl->initiatorname, > + GFP_KERNEL); > + if (!se_sess->acl_name) > + return -ENOMEM; > + } > + > + se_sess->target_name = kstrdup(se_tpg->se_tpg_wwn->wwn_group.cg_item.ci_name, > + GFP_KERNEL); > + if (!se_sess->target_name) > + goto free_acl; > + > + if (se_sess->tfo->fabric_alias) > + se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_alias, > + GFP_KERNEL); > + else > + se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_name, > + GFP_KERNEL); > + if (!se_sess->fabric_name) > + goto free_target; > + > + se_sess->tpg_name = kstrdup(se_tpg->tpg_group.cg_item.ci_name, > + GFP_KERNEL); > + if (!se_sess->tpg_name) > + goto free_fabric; > + > + return 0; I wonder if we need to copy these variables. Why can't we display them directly, returning an error if the respective link is not available? If one is worried about the sysfs/configfs reference counting we can get the reference in the _show() functions; wouldn't that be a better solution? Cheers, Hannes
On 06/07/20 22:35, Mike Christie wrote: > This patch adds a sysfs interface to export the target module's > sessions using the driver module class structure. With this we > now have a common way to export session info for all fabric modules > and we can export info about all sessions not just the first one > on a ACL like for iscsi. We can also export more info then PAGE_SIZE > bytes for all sessions (looking at iscsi and qla2xxx and the > dynamic_sessions file abuse). > > Here is a tree the new class dir scsi_target_session: > > session-1/ > ├── power > │ ├── autosuspend_delay_ms > │ ├── control > │ ├── runtime_active_time > │ ├── runtime_status > │ └── runtime_suspended_time > ├── subsystem -> ../../../../class/scsi_target_session > ├── tcm_endpoint > │ ├── acl > │ ├── fabric > │ ├── target > │ └── tpg > ├── transport_id > │ ├── name > │ └── proto > └── uevent > > The Documentation/ABI/testing/sysfs-scsi-target-session file in this > patch describes the files and dirs. > > Userspace apps like tcmu-runner and targetcli, can either: > > 1. If they have the session ID, then directly look up the session's > info like with tagetcli's session sid case. > > 2. If they have the target side's endpoint object name like the ACL > or tpg, then they have to scan each session's tcm_endpoint dir. This > should hopefully not be the normal case. For tcmu-runner we will > normally scan sysfs, cache the info, then update the cache as we get > events about sessions being added/removed. > > Note that to make it easier for Greg to review I am just including > the sysfs parts in this patch. The next patch hooks into the target > code and enables it. > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Mike Christie <michael.christie@oracle.com> > --- > .../ABI/testing/sysfs-scsi-target-session | 71 ++++++ > drivers/target/target_core_internal.h | 5 + > drivers/target/target_core_sysfs.c | 265 +++++++++++++++++++++ > include/target/target_core_base.h | 8 + > include/target/target_core_fabric.h | 6 +- > 5 files changed, 354 insertions(+), 1 deletion(-) > create mode 100644 Documentation/ABI/testing/sysfs-scsi-target-session > create mode 100644 drivers/target/target_core_sysfs.c > > diff --git a/Documentation/ABI/testing/sysfs-scsi-target-session b/Documentation/ABI/testing/sysfs-scsi-target-session > new file mode 100644 > index 0000000..c8cefcc > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-scsi-target-session > @@ -0,0 +1,71 @@ > +What: /sys/class/scsi_target_session/session-N > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: The session dir contains a dir for each I_T Nexus. The name of > + the dir is "session-" followed by an integer that is unique > + across all fabrics. The dir contains files that export info like > + the TPG/ACL the session is attached to, SCSI port values, and > + fabric and transport specific values. > + > +What: /sys/class/scsi_target_session/session-N/tcm_endpoint > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: The tcm_endpoint dir indicates what target_core_mod object > + the session is attached to. > + > +What: /sys/class/scsi_target_session/session-N/tcm_endpoint/acl > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: Returns the name of the ACL that was used to validate login or > + an empty string if no user created ACL was used. > + > +What: /sys/class/scsi_target_session/session-N/tcm_endpoint/target > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: Returns the name of the target the session is accessed through. > + > +What: /sys/class/scsi_target_session/session-N/tcm_endpoint/fabric > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: Returns the name of the fabric the session is accessed through. > + > +What: /sys/class/scsi_target_session/session-N/tcm_endpoint/tpg > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: Returns the name of the tpg the session is accessed through. > + > +What: /sys/class/scsi_target_session/session-N/transport_id > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: The transport_id contains the SCSI TransportID values for the > + initiator port as defined in SPC4r37. > + > +What: /sys/class/scsi_target_session/session-N/transport_id/proto > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: Returns the SCSI protocol identifier in hex defined in SPC4r37. > + > +What: /sys/class/scsi_target_session/session-N/transport_id/name > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: Returns the port name/address/id in the protocol specific > + TransportID format defined in SPC4r37's. > + > +What: /sys/class/scsi_target_session/session-N/transport_id/session_id > +Date: June 5, 2020 > +KernelVersion: 5.9 > +Contact: linux-scsi@vger.kernel.org > +Description: If is proto=0x5 (iSCSI) and TPID FORMAT=1 this file will exist > + and will return the iSCSI Initiator Session ID in ASCII > + characters that are the hexadecimal digits converted from the > + binary iSCSI initiator session identifier value. If iSCSI and > + format=1 is not used by this session this file will not exist. If I got it right, this is not how the code works. AFAICS, the file will always exist, but reading it delivers data only if proto=0x5 (iSCSI) and TPID FORMAT=1 Thank you, Bodo > diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h > index 8533444..93bf5fed 100644 > --- a/drivers/target/target_core_internal.h > +++ b/drivers/target/target_core_internal.h > @@ -105,6 +105,11 @@ int target_get_pr_transport_id(struct se_node_acl *nacl, > const char *target_parse_pr_out_transport_id(struct se_portal_group *tpg, > char *buf, u32 *out_tid_len, char **port_nexus_ptr); > > +/* target_core_sysfs.c */ > +int target_sysfs_init_session(struct se_session *sess); > +void target_sysfs_init(void); > +void target_sysfs_exit(void); > + > /* target_core_hba.c */ > struct se_hba *core_alloc_hba(const char *, u32, u32); > int core_delete_hba(struct se_hba *); > diff --git a/drivers/target/target_core_sysfs.c b/drivers/target/target_core_sysfs.c > new file mode 100644 > index 0000000..b29a6f2 > --- /dev/null > +++ b/drivers/target/target_core_sysfs.c > @@ -0,0 +1,265 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +#include <linux/device.h> > +#include <linux/idr.h> > +#include <linux/module.h> > + > +#include <scsi/scsi_proto.h> > + > +#include <target/target_core_base.h> > +#include <target/target_core_fabric.h> > +#include "target_core_internal.h" > + > +static DEFINE_IDA(session_ida); > + > +#define dev_to_se_sess(_dev) \ > + container_of(dev, struct se_session, dev) > + > +static void target_free_endpoint_strs(struct se_session *se_sess) > +{ > + kfree(se_sess->tpg_name); > + kfree(se_sess->fabric_name); > + kfree(se_sess->target_name); > + kfree(se_sess->acl_name); > +} > + > +static void target_sysfs_session_release(struct device *dev) > +{ > + struct se_session *se_sess = dev_to_se_sess(dev); > + > + target_free_endpoint_strs(se_sess); > + ida_simple_remove(&session_ida, se_sess->sid); > + > + __target_free_session(se_sess); > +} > + > +#define target_attr_show(field) \ > +static ssize_t show_target_##field(struct device *dev, \ > + struct device_attribute *attr, char *buf) \ > +{ \ > + struct se_session *se_sess = dev_to_se_sess(dev); \ > + \ > + if (!se_sess->field##_name) \ > + return 0; \ > + \ > + return snprintf(buf, PAGE_SIZE, "%s", se_sess->field##_name); \ > +} > + > +#define target_attr_str(field) \ > + target_attr_show(field) \ > +static DEVICE_ATTR(field, S_IRUGO, show_target_##field, NULL) > + > +target_attr_str(acl); > +target_attr_str(tpg); > +target_attr_str(target); > +target_attr_str(fabric); > + > +/* > + * attrs needed to reference the session's tcm endpoint (acl or tpg) in > + * configfs > + */ > +static struct attribute *target_endpoint_attrs[] = { > + &dev_attr_acl.attr, > + &dev_attr_tpg.attr, > + &dev_attr_target.attr, > + &dev_attr_fabric.attr, > + NULL, > +}; > + > +static const struct attribute_group target_endpoint_group = { > + .name = "tcm_endpoint", > + .attrs = target_endpoint_attrs, > +}; > + > +/* transportID attrs */ > +#define tpt_id_attr_show(name, fmt_str) \ > +static ssize_t show_tpid_##name(struct device *dev, \ > + struct device_attribute *attr, char *buf) \ > +{ \ > + struct se_session *se_sess = dev_to_se_sess(dev); \ > + return snprintf(buf, PAGE_SIZE, fmt_str, se_sess->tpt_id->name); \ > +} > + > +#define tpt_id_attr(name, fmt_str) \ > + tpt_id_attr_show(name, fmt_str) \ > +static DEVICE_ATTR(name, S_IRUGO, show_tpid_##name, NULL) > + > +tpt_id_attr(proto, "0x%x"); > +tpt_id_attr(name, "%s"); > + > +static ssize_t session_id_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct se_session *se_sess = dev_to_se_sess(dev); > + > + if (!se_sess->tpt_id->session_id) > + return 0; > + > + return snprintf(buf, PAGE_SIZE, "0x%s", se_sess->tpt_id->session_id); > +} > + > +static DEVICE_ATTR(session_id, S_IRUGO, session_id_show, NULL); > + > +static struct attribute *tpt_id_attrs[] = { > + &dev_attr_proto.attr, > + &dev_attr_name.attr, > + &dev_attr_session_id.attr, > + NULL, > +}; > + > +static const struct attribute_group tpt_id_group = { > + .name = "transport_id", > + .attrs = tpt_id_attrs, > +}; > + > +static const struct attribute_group *def_session_groups[] = { > + &tpt_id_group, > + &target_endpoint_group, > + NULL, > +}; > + > +static struct class session_class = { > + .owner = THIS_MODULE, > + .name = "scsi_target_session", > + .dev_release = target_sysfs_session_release, > + .dev_groups = def_session_groups, > +}; > + > +int target_sysfs_init_session(struct se_session *se_sess) > +{ > + int ret; > + > + ret = ida_simple_get(&session_ida, 1, 0, GFP_KERNEL); > + if (ret < 0) { > + pr_err("Unable to allocate session index.\n"); > + return ret; > + } > + se_sess->sid = ret; > + > + device_initialize(&se_sess->dev); > + se_sess->dev.class = &session_class; > + > + ret = dev_set_name(&se_sess->dev, "session-%d", se_sess->sid); > + if (ret) > + goto put_dev; > + > + return 0; > + > +put_dev: > + put_device(&se_sess->dev); > + return ret; > +} > + > +static int target_cp_endpoint_strs(struct se_portal_group *se_tpg, > + struct se_session *se_sess) > +{ > + /* > + * Copy configfs dir/object names so userspace can match the session > + * to its target, and we also don't have to worry about mixing configfs > + * refcounts with sysfs. > + */ > + if (!se_sess->se_node_acl->dynamic_node_acl) { > + se_sess->acl_name = kstrdup(se_sess->se_node_acl->initiatorname, > + GFP_KERNEL); > + if (!se_sess->acl_name) > + return -ENOMEM; > + } > + > + se_sess->target_name = kstrdup(se_tpg->se_tpg_wwn->wwn_group.cg_item.ci_name, > + GFP_KERNEL); > + if (!se_sess->target_name) > + goto free_acl; > + > + if (se_sess->tfo->fabric_alias) > + se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_alias, > + GFP_KERNEL); > + else > + se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_name, > + GFP_KERNEL); > + if (!se_sess->fabric_name) > + goto free_target; > + > + se_sess->tpg_name = kstrdup(se_tpg->tpg_group.cg_item.ci_name, > + GFP_KERNEL); > + if (!se_sess->tpg_name) > + goto free_fabric; > + > + return 0; > + > +free_fabric: > + kfree(se_sess->fabric_name); > + se_sess->fabric_name = NULL; > +free_target: > + kfree(se_sess->target_name); > + se_sess->target_name = NULL; > +free_acl: > + kfree(se_sess->acl_name); > + se_sess->acl_name = NULL; > + return -ENOMEM; > +} > + > +int target_sysfs_add_session(struct se_portal_group *se_tpg, > + struct se_session *se_sess) > +{ > + struct t10_transport_id *tpt_id = se_sess->tpt_id; > + int ret; > + > + if (!try_module_get(se_sess->tfo->module)) > + return -EINVAL; > + > + ret = target_cp_endpoint_strs(se_tpg, se_sess); > + if (ret) > + goto put_mod; > + > + se_sess->dev.groups = se_sess->tfo->session_attr_groups; > + ret = device_add(&se_sess->dev); > + if (ret) { > + pr_err("Could not add session%d to sysfs. Error %d.\n", > + se_sess->sid, ret); > + goto free_ep_strs; > + } > + > + pr_info("TCM added session-%d from [fabric: %s, target: %s, tpg %s, acl: %s] to [initiator port: %s%s%s]", > + se_sess->sid, se_sess->fabric_name, se_sess->target_name, > + se_sess->tpg_name, se_sess->acl_name ? se_sess->acl_name : "dynamic", > + tpt_id->name, tpt_id->session_id ? "," : "", > + tpt_id->session_id ? tpt_id->session_id : ""); > + > + se_sess->sysfs_added = true; > + return 0; > + > +free_ep_strs: > + target_free_endpoint_strs(se_sess); > +put_mod: > + module_put(se_sess->tfo->module); > + return ret; > +} > +EXPORT_SYMBOL(target_sysfs_add_session); > + > +void target_sysfs_remove_session(struct se_session *se_sess) > +{ > + struct t10_transport_id *tpt_id = se_sess->tpt_id; > + > + /* discovery sessions are normally not added to sysfs */ > + if (!se_sess->sysfs_added) > + return; > + se_sess->sysfs_added = false; > + > + pr_info("TCM removed session-%d from [fabric: %s, target: %s, tpg %s, acl: %s] to [initiator port: %s%s%s]", > + se_sess->sid, se_sess->fabric_name, se_sess->target_name, > + se_sess->tpg_name, se_sess->acl_name ? se_sess->acl_name : "dynamic", > + tpt_id->name, tpt_id->session_id ? "," : "", > + tpt_id->session_id ? tpt_id->session_id : ""); > + > + device_del(&se_sess->dev); > +} > +EXPORT_SYMBOL(target_sysfs_remove_session); > + > +void target_sysfs_init(void) > +{ > + class_register(&session_class); > +} > + > +void target_sysfs_exit(void) > +{ > + class_unregister(&session_class); > +} > diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h > index 2e79cce..0d9916b 100644 > --- a/include/target/target_core_base.h > +++ b/include/target/target_core_base.h > @@ -8,6 +8,7 @@ > #include <linux/percpu-refcount.h> > #include <linux/semaphore.h> /* struct semaphore */ > #include <linux/completion.h> > +#include <linux/device.h> > > #define TARGET_CORE_VERSION "v5.0" > > @@ -635,6 +636,13 @@ struct se_session { > wait_queue_head_t cmd_list_wq; > void *sess_cmd_map; > struct sbitmap_queue sess_tag_pool; > + struct device dev; > + int sid; > + bool sysfs_added; > + char *acl_name; > + char *tpg_name; > + char *target_name; > + char *fabric_name; > }; > > struct se_device; > diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h > index ced377f..2a93daa 100644 > --- a/include/target/target_core_fabric.h > +++ b/include/target/target_core_fabric.h > @@ -105,6 +105,8 @@ struct target_core_fabric_ops { > struct configfs_attribute **tfc_tpg_nacl_auth_attrs; > struct configfs_attribute **tfc_tpg_nacl_param_attrs; > > + const struct attribute_group **session_attr_groups; > + > /* > * Set this member variable to true if the SCSI transport protocol > * (e.g. iSCSI) requires that the Data-Out buffer is transferred in > @@ -145,7 +147,9 @@ void transport_register_session(struct se_portal_group *, > void target_put_nacl(struct se_node_acl *); > void transport_deregister_session_configfs(struct se_session *); > void transport_deregister_session(struct se_session *); > - > +void target_sysfs_remove_session(struct se_session *se_sess); > +int target_sysfs_add_session(struct se_portal_group *se_tpg, > + struct se_session *se_sess); > > void transport_init_se_cmd(struct se_cmd *, > const struct target_core_fabric_ops *, >
On Sun, Jun 07, 2020 at 03:35:58PM -0500, Mike Christie wrote: > +#define target_attr_show(field) \ > +static ssize_t show_target_##field(struct device *dev, \ > + struct device_attribute *attr, char *buf) \ > +{ \ > + struct se_session *se_sess = dev_to_se_sess(dev); \ > + \ > + if (!se_sess->field##_name) \ > + return 0; \ > + \ > + return snprintf(buf, PAGE_SIZE, "%s", se_sess->field##_name); \ Nit, you do not need to call snprintf() as you "know" your buffer is big enough, right? Please just use sprintf() for sysfs show functions to enforce that. > +/* transportID attrs */ > +#define tpt_id_attr_show(name, fmt_str) \ > +static ssize_t show_tpid_##name(struct device *dev, \ > + struct device_attribute *attr, char *buf) \ > +{ \ > + struct se_session *se_sess = dev_to_se_sess(dev); \ > + return snprintf(buf, PAGE_SIZE, fmt_str, se_sess->tpt_id->name); \ sprintf() as above. Same for all of the other show calls in this file. thanks, greg k-h > +} > + > +#define tpt_id_attr(name, fmt_str) \ > + tpt_id_attr_show(name, fmt_str) \ > +static DEVICE_ATTR(name, S_IRUGO, show_tpid_##name, NULL) > + > +tpt_id_attr(proto, "0x%x"); > +tpt_id_attr(name, "%s"); > + > +static ssize_t session_id_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct se_session *se_sess = dev_to_se_sess(dev); > + > + if (!se_sess->tpt_id->session_id) > + return 0; > + > + return snprintf(buf, PAGE_SIZE, "0x%s", se_sess->tpt_id->session_id); > +} > + > +static DEVICE_ATTR(session_id, S_IRUGO, session_id_show, NULL); DEVICE_ATTR_RO(). > + > +static struct attribute *tpt_id_attrs[] = { > + &dev_attr_proto.attr, > + &dev_attr_name.attr, > + &dev_attr_session_id.attr, > + NULL, > +}; > + > +static const struct attribute_group tpt_id_group = { > + .name = "transport_id", > + .attrs = tpt_id_attrs, > +}; > + > +static const struct attribute_group *def_session_groups[] = { > + &tpt_id_group, > + &target_endpoint_group, > + NULL, > +}; > + > +static struct class session_class = { > + .owner = THIS_MODULE, > + .name = "scsi_target_session", > + .dev_release = target_sysfs_session_release, > + .dev_groups = def_session_groups, > +}; > + > +int target_sysfs_init_session(struct se_session *se_sess) > +{ > + int ret; > + > + ret = ida_simple_get(&session_ida, 1, 0, GFP_KERNEL); > + if (ret < 0) { > + pr_err("Unable to allocate session index.\n"); > + return ret; > + } > + se_sess->sid = ret; > + > + device_initialize(&se_sess->dev); > + se_sess->dev.class = &session_class; > + > + ret = dev_set_name(&se_sess->dev, "session-%d", se_sess->sid); > + if (ret) > + goto put_dev; > + > + return 0; > + > +put_dev: > + put_device(&se_sess->dev); > + return ret; > +} > + > +static int target_cp_endpoint_strs(struct se_portal_group *se_tpg, > + struct se_session *se_sess) > +{ > + /* > + * Copy configfs dir/object names so userspace can match the session > + * to its target, and we also don't have to worry about mixing configfs > + * refcounts with sysfs. > + */ > + if (!se_sess->se_node_acl->dynamic_node_acl) { > + se_sess->acl_name = kstrdup(se_sess->se_node_acl->initiatorname, > + GFP_KERNEL); > + if (!se_sess->acl_name) > + return -ENOMEM; > + } > + > + se_sess->target_name = kstrdup(se_tpg->se_tpg_wwn->wwn_group.cg_item.ci_name, > + GFP_KERNEL); > + if (!se_sess->target_name) > + goto free_acl; > + > + if (se_sess->tfo->fabric_alias) > + se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_alias, > + GFP_KERNEL); > + else > + se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_name, > + GFP_KERNEL); > + if (!se_sess->fabric_name) > + goto free_target; > + > + se_sess->tpg_name = kstrdup(se_tpg->tpg_group.cg_item.ci_name, > + GFP_KERNEL); > + if (!se_sess->tpg_name) > + goto free_fabric; > + > + return 0; > + > +free_fabric: > + kfree(se_sess->fabric_name); > + se_sess->fabric_name = NULL; > +free_target: > + kfree(se_sess->target_name); > + se_sess->target_name = NULL; > +free_acl: > + kfree(se_sess->acl_name); > + se_sess->acl_name = NULL; > + return -ENOMEM; > +} > + > +int target_sysfs_add_session(struct se_portal_group *se_tpg, > + struct se_session *se_sess) > +{ > + struct t10_transport_id *tpt_id = se_sess->tpt_id; > + int ret; > + > + if (!try_module_get(se_sess->tfo->module)) > + return -EINVAL; > + > + ret = target_cp_endpoint_strs(se_tpg, se_sess); > + if (ret) > + goto put_mod; > + > + se_sess->dev.groups = se_sess->tfo->session_attr_groups; > + ret = device_add(&se_sess->dev); > + if (ret) { > + pr_err("Could not add session%d to sysfs. Error %d.\n", > + se_sess->sid, ret); > + goto free_ep_strs; > + } > + > + pr_info("TCM added session-%d from [fabric: %s, target: %s, tpg %s, acl: %s] to [initiator port: %s%s%s]", > + se_sess->sid, se_sess->fabric_name, se_sess->target_name, > + se_sess->tpg_name, se_sess->acl_name ? se_sess->acl_name : "dynamic", > + tpt_id->name, tpt_id->session_id ? "," : "", > + tpt_id->session_id ? tpt_id->session_id : ""); > + > + se_sess->sysfs_added = true; > + return 0; > + > +free_ep_strs: > + target_free_endpoint_strs(se_sess); > +put_mod: > + module_put(se_sess->tfo->module); > + return ret; > +} > +EXPORT_SYMBOL(target_sysfs_add_session); > + > +void target_sysfs_remove_session(struct se_session *se_sess) > +{ > + struct t10_transport_id *tpt_id = se_sess->tpt_id; > + > + /* discovery sessions are normally not added to sysfs */ > + if (!se_sess->sysfs_added) > + return; > + se_sess->sysfs_added = false; > + > + pr_info("TCM removed session-%d from [fabric: %s, target: %s, tpg %s, acl: %s] to [initiator port: %s%s%s]", > + se_sess->sid, se_sess->fabric_name, se_sess->target_name, > + se_sess->tpg_name, se_sess->acl_name ? se_sess->acl_name : "dynamic", > + tpt_id->name, tpt_id->session_id ? "," : "", > + tpt_id->session_id ? tpt_id->session_id : ""); > + > + device_del(&se_sess->dev); > +} > +EXPORT_SYMBOL(target_sysfs_remove_session); > + > +void target_sysfs_init(void) > +{ > + class_register(&session_class); > +} > + > +void target_sysfs_exit(void) > +{ > + class_unregister(&session_class); I think you forgot to cleanup your ida structure here, right? thanks, greg k-h
On Sun, Jun 07, 2020 at 03:35:58PM -0500, Mike Christie wrote: > +#define target_attr_str(field) \ > + target_attr_show(field) \ > +static DEVICE_ATTR(field, S_IRUGO, show_target_##field, NULL) DEVICE_ATTR_RO() > +#define tpt_id_attr(name, fmt_str) \ > + tpt_id_attr_show(name, fmt_str) \ > +static DEVICE_ATTR(name, S_IRUGO, show_tpid_##name, NULL) DEVICE_ATTR_RO()
On 6/8/20 7:06 AM, Bodo Stroesser wrote: >> +What: >> /sys/class/scsi_target_session/session-N/transport_id/session_id >> +Date: June 5, 2020 >> +KernelVersion: 5.9 >> +Contact: linux-scsi@vger.kernel.org >> +Description: If is proto=0x5 (iSCSI) and TPID FORMAT=1 this file >> will exist >> + and will return the iSCSI Initiator Session ID in ASCII >> + characters that are the hexadecimal digits converted from the >> + binary iSCSI initiator session identifier value. If iSCSI and >> + format=1 is not used by this session this file will not exist. > > If I got it right, this is not how the code works. > AFAICS, the file will always exist, but reading it delivers data only if > proto=0x5 (iSCSI) and TPID FORMAT=1 > I forgot to update the description when I changed the code. Will update. Thanks.
On 6/8/20 1:14 AM, Hannes Reinecke wrote: >> + se_sess->target_name = >> kstrdup(se_tpg->se_tpg_wwn->wwn_group.cg_item.ci_name, >> + GFP_KERNEL); >> + if (!se_sess->target_name) >> + goto free_acl; >> + >> + if (se_sess->tfo->fabric_alias) >> + se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_alias, >> + GFP_KERNEL); >> + else >> + se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_name, >> + GFP_KERNEL); >> + if (!se_sess->fabric_name) >> + goto free_target; >> + >> + se_sess->tpg_name = kstrdup(se_tpg->tpg_group.cg_item.ci_name, >> + GFP_KERNEL); >> + if (!se_sess->tpg_name) >> + goto free_fabric; >> + >> + return 0; > > I wonder if we need to copy these variables. > Why can't we display them directly, returning an error if the respective > link is not available? > If one is worried about the sysfs/configfs reference counting we can get > the reference in the _show() functions; wouldn't that be a better solution? Do you mean in the sysfs show function do a configfs_depend_item() on the tpg, www, acl, etc? If so, I'm not sure that's safe, because for the tpg for example, we could do: 1. Userspace starts tpg removal with a rmdir. 2. Userspace also opens sysfs session file and has a ref to the session, but not the tpg yet. 3. kernel tears down tpg and sessions under it. The tpg is freed. The session is not because of the ref taken in #2. 4. sysfs session show function starts to reference se_sess->se_tpg so it can do a configfs_depend_item on the tpg_group.cg_item, but the se_tpg is freed in #3. We either need to: 1. cp like above. 2. Handle both configfs and device struct refcounts for the same struct. So add a device struct to the www, tpg, acl and then coordinate the refcounting. 3. take a reference to the se_tpg when the session is created then drop it in the session release. 4. add some code and end up mix locking and state checks with refcounts. For example the tpg would have its configfs refcount like it does today (no new device struct in it), and when it deletes the sessions under it make sure the se_sess->se_tpg is NULL'd in a way that the session show function can do lock() if (!se_sess->se_tpg) return se_sess->se_tpg access unlock() And then you have to do that up the stack for the other structs we want to ref.
On 6/8/20 12:32 AM, Greg Kroah-Hartman wrote: > On Sun, Jun 07, 2020 at 03:35:58PM -0500, Mike Christie wrote: >> +int target_sysfs_add_session(struct se_portal_group *se_tpg, >> + struct se_session *se_sess) >> +{ >> + struct t10_transport_id *tpt_id = se_sess->tpt_id; >> + int ret; >> + >> + if (!try_module_get(se_sess->tfo->module)) >> + return -EINVAL; >> + >> + ret = target_cp_endpoint_strs(se_tpg, se_sess); >> + if (ret) >> + goto put_mod; >> + >> + se_sess->dev.groups = se_sess->tfo->session_attr_groups; >> + ret = device_add(&se_sess->dev); >> + if (ret) { >> + pr_err("Could not add session%d to sysfs. Error %d.\n", >> + se_sess->sid, ret); >> + goto free_ep_strs; >> + } >> + >> + pr_info("TCM added session-%d from [fabric: %s, target: %s, tpg %s, acl: %s] to [initiator port: %s%s%s]", >> + se_sess->sid, se_sess->fabric_name, se_sess->target_name, >> + se_sess->tpg_name, se_sess->acl_name ? se_sess->acl_name : "dynamic", >> + tpt_id->name, tpt_id->session_id ? "," : "", >> + tpt_id->session_id ? tpt_id->session_id : ""); > > You have a 'struct device', so please use it, no need for pr_info(), > always use the dev_*() calls instead. > > but, when drivers and kernel code is all working properly, no need to be > noisy at all, this should just be a dev_dbg() call, right? I liked the info one, because the the code can work correctly, but the remote devices we are connecting to are normally going to hit issues. For example every time the storage network goes down temporarily the driver code will call remove function, then call the add again when it comes back up. Having it always logged helps us figure out the root problem later when the customer only has logs available. > >> + >> + se_sess->sysfs_added = true; >> + return 0; >> + >> +free_ep_strs: >> + target_free_endpoint_strs(se_sess); >> +put_mod: >> + module_put(se_sess->tfo->module); >> + return ret; >> +} >> +EXPORT_SYMBOL(target_sysfs_add_session); > > I have to ask, EXPORT_SYMBOL_GPL()? > Just a cp mistake. The original author/maintainer of the target code used the non GPL calls, and I just blindly copied it over. I will use the GPL ones on the resend. I agree with your other review comments in this mail and the others and will fix on the resend. Thanks,
> On Jun 8, 2020, at 10:21 AM, Mike Christie <michael.christie@oracle.com> wrote: > > On 6/8/20 1:14 AM, Hannes Reinecke wrote: >>> + se_sess->target_name = kstrdup(se_tpg->se_tpg_wwn->wwn_group.cg_item.ci_name, >>> + GFP_KERNEL); >>> + if (!se_sess->target_name) >>> + goto free_acl; >>> + >>> + if (se_sess->tfo->fabric_alias) >>> + se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_alias, >>> + GFP_KERNEL); >>> + else >>> + se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_name, >>> + GFP_KERNEL); >>> + if (!se_sess->fabric_name) >>> + goto free_target; >>> + >>> + se_sess->tpg_name = kstrdup(se_tpg->tpg_group.cg_item.ci_name, >>> + GFP_KERNEL); >>> + if (!se_sess->tpg_name) >>> + goto free_fabric; >>> + >>> + return 0; >> I wonder if we need to copy these variables. >> Why can't we display them directly, returning an error if the respective >> link is not available? >> If one is worried about the sysfs/configfs reference counting we can get the reference in the _show() functions; wouldn't that be a better solution? > > Do you mean in the sysfs show function do a configfs_depend_item() on the tpg, www, acl, etc? If so, I'm not sure that's safe, because for the tpg for example, we could do: > > 1. Userspace starts tpg removal with a rmdir. > 2. Userspace also opens sysfs session file and has a ref to the session, but not the tpg yet. > 3. kernel tears down tpg and sessions under it. The tpg is freed. The session is not because of the ref taken in #2. > 4. sysfs session show function starts to reference se_sess->se_tpg so it can do a configfs_depend_item on the tpg_group.cg_item, but the se_tpg is freed in #3. > > We either need to: > 1. cp like above. > 2. Handle both configfs and device struct refcounts for the same struct. So add a device struct to the www, tpg, acl and then coordinate the refcounting. > 3. take a reference to the se_tpg when the session is created then drop it in the session release. Ignore #3. That does not work. Userspace expects to be able to rmdir on a tpg and that will remove the underlying sessions. If the session takes a ref to the tpg, then it breaks userspace because something now has to do the session removal before the tpg rmdir. > 4. add some code and end up mix locking and state checks with refcounts. For example the tpg would have its configfs refcount like it does today (no new device struct in it), and when it deletes the sessions under it make sure the se_sess->se_tpg is NULL'd in a way that the session show function can do > > lock() > if (!se_sess->se_tpg) > return > > se_sess->se_tpg access > unlock() > > And then you have to do that up the stack for the other structs we want to ref.
On Mon, Jun 08, 2020 at 10:35:56AM -0500, Mike Christie wrote: > > > On 6/8/20 12:32 AM, Greg Kroah-Hartman wrote: > > On Sun, Jun 07, 2020 at 03:35:58PM -0500, Mike Christie wrote: > > > +int target_sysfs_add_session(struct se_portal_group *se_tpg, > > > + struct se_session *se_sess) > > > +{ > > > + struct t10_transport_id *tpt_id = se_sess->tpt_id; > > > + int ret; > > > + > > > + if (!try_module_get(se_sess->tfo->module)) > > > + return -EINVAL; > > > + > > > + ret = target_cp_endpoint_strs(se_tpg, se_sess); > > > + if (ret) > > > + goto put_mod; > > > + > > > + se_sess->dev.groups = se_sess->tfo->session_attr_groups; > > > + ret = device_add(&se_sess->dev); > > > + if (ret) { > > > + pr_err("Could not add session%d to sysfs. Error %d.\n", > > > + se_sess->sid, ret); > > > + goto free_ep_strs; > > > + } > > > + > > > + pr_info("TCM added session-%d from [fabric: %s, target: %s, tpg %s, acl: %s] to [initiator port: %s%s%s]", > > > + se_sess->sid, se_sess->fabric_name, se_sess->target_name, > > > + se_sess->tpg_name, se_sess->acl_name ? se_sess->acl_name : "dynamic", > > > + tpt_id->name, tpt_id->session_id ? "," : "", > > > + tpt_id->session_id ? tpt_id->session_id : ""); > > > > You have a 'struct device', so please use it, no need for pr_info(), > > always use the dev_*() calls instead. > > > > but, when drivers and kernel code is all working properly, no need to be > > noisy at all, this should just be a dev_dbg() call, right? > > I liked the info one, because the the code can work correctly, but the > remote devices we are connecting to are normally going to hit issues. > > For example every time the storage network goes down temporarily the driver > code will call remove function, then call the add again when it comes back > up. Having it always logged helps us figure out the root problem later when > the customer only has logs available. Then make this a tracepoint or something, again, do not be noisy for normal system operations. Do you want this to be the case for all of your hardware devices all the time? thanks, greg k-h
On 6/8/20 11:36 AM, Greg Kroah-Hartman wrote: > On Mon, Jun 08, 2020 at 10:35:56AM -0500, Mike Christie wrote: >> >> >> On 6/8/20 12:32 AM, Greg Kroah-Hartman wrote: >>> On Sun, Jun 07, 2020 at 03:35:58PM -0500, Mike Christie wrote: >>>> +int target_sysfs_add_session(struct se_portal_group *se_tpg, >>>> + struct se_session *se_sess) >>>> +{ >>>> + struct t10_transport_id *tpt_id = se_sess->tpt_id; >>>> + int ret; >>>> + >>>> + if (!try_module_get(se_sess->tfo->module)) >>>> + return -EINVAL; >>>> + >>>> + ret = target_cp_endpoint_strs(se_tpg, se_sess); >>>> + if (ret) >>>> + goto put_mod; >>>> + >>>> + se_sess->dev.groups = se_sess->tfo->session_attr_groups; >>>> + ret = device_add(&se_sess->dev); >>>> + if (ret) { >>>> + pr_err("Could not add session%d to sysfs. Error %d.\n", >>>> + se_sess->sid, ret); >>>> + goto free_ep_strs; >>>> + } >>>> + >>>> + pr_info("TCM added session-%d from [fabric: %s, target: %s, tpg %s, acl: %s] to [initiator port: %s%s%s]", >>>> + se_sess->sid, se_sess->fabric_name, se_sess->target_name, >>>> + se_sess->tpg_name, se_sess->acl_name ? se_sess->acl_name : "dynamic", >>>> + tpt_id->name, tpt_id->session_id ? "," : "", >>>> + tpt_id->session_id ? tpt_id->session_id : ""); >>> >>> You have a 'struct device', so please use it, no need for pr_info(), >>> always use the dev_*() calls instead. >>> >>> but, when drivers and kernel code is all working properly, no need to be >>> noisy at all, this should just be a dev_dbg() call, right? >> >> I liked the info one, because the the code can work correctly, but the >> remote devices we are connecting to are normally going to hit issues. >> >> For example every time the storage network goes down temporarily the driver >> code will call remove function, then call the add again when it comes back >> up. Having it always logged helps us figure out the root problem later when >> the customer only has logs available. > > Then make this a tracepoint or something, again, do not be noisy for > normal system operations. Do you want this to be the case for all of What's a special case vs normal? I would agree having a SCSI HBA in your system and having it setup during system boot up is normal. I would agree hardware failing is special. Having a remote client connect to us, lose its connected then recover seems special, because it's a failure case. Even the initial connection seems like a special event, because the user has done some extra steps to have the client connect to us. It's like adding a new disk to the system which we log today or like plugging in or removing a USB device which we also log. > your hardware devices all the time? > I do, but I'm a kernel developer :) For the sys admin and distro debugging type of case, yes, they want this type of thing logged, because they have done some extra setup to have a remote client connect to us. When the connection is lost and then re-added they want all that info logged with the lower level info we are already logging, so they can follow the time of events.
> On Jun 8, 2020, at 10:55 AM, Michael Christie <michael.christie@oracle.com> wrote: > > > >> On Jun 8, 2020, at 10:21 AM, Mike Christie <michael.christie@oracle.com> wrote: >> >> On 6/8/20 1:14 AM, Hannes Reinecke wrote: >>>> + se_sess->target_name = kstrdup(se_tpg->se_tpg_wwn->wwn_group.cg_item.ci_name, >>>> + GFP_KERNEL); >>>> + if (!se_sess->target_name) >>>> + goto free_acl; >>>> + >>>> + if (se_sess->tfo->fabric_alias) >>>> + se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_alias, >>>> + GFP_KERNEL); >>>> + else >>>> + se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_name, >>>> + GFP_KERNEL); >>>> + if (!se_sess->fabric_name) >>>> + goto free_target; >>>> + >>>> + se_sess->tpg_name = kstrdup(se_tpg->tpg_group.cg_item.ci_name, >>>> + GFP_KERNEL); >>>> + if (!se_sess->tpg_name) >>>> + goto free_fabric; >>>> + >>>> + return 0; >>> I wonder if we need to copy these variables. >>> Why can't we display them directly, returning an error if the respective >>> link is not available? >>> If one is worried about the sysfs/configfs reference counting we can get the reference in the _show() functions; wouldn't that be a better solution? >> >> Do you mean in the sysfs show function do a configfs_depend_item() on the tpg, www, acl, etc? If so, I'm not sure that's safe, because for the tpg for example, we could do: >> >> 1. Userspace starts tpg removal with a rmdir. >> 2. Userspace also opens sysfs session file and has a ref to the session, but not the tpg yet. >> 3. kernel tears down tpg and sessions under it. The tpg is freed. The session is not because of the ref taken in #2. >> 4. sysfs session show function starts to reference se_sess->se_tpg so it can do a configfs_depend_item on the tpg_group.cg_item, but the se_tpg is freed in #3. >> >> We either need to: >> 1. cp like above. >> 2. Handle both configfs and device struct refcounts for the same struct. So add a device struct to the www, tpg, acl and then coordinate the refcounting. >> 3. take a reference to the se_tpg when the session is created then drop it in the session release. > > > Ignore #3. That does not work. Userspace expects to be able to rmdir on a tpg and that will remove the underlying sessions. If the session takes a ref to the tpg, then it breaks userspace because something now has to do the session removal before the tpg rmdir. > I was thinking about this some more and we could do the following: 1. When userspace creates a target or tpg, it tells the kernel if it supports the updated session interface. 2. If userspace supports it, when we create a session we get a reference to the tpg with configfs_depend_item(). And create the sysfs interface. We might actually be able to do confifgfs. 3. On tpg deletion, if at #1 it has told us it supports the new interface then userspace just has to tear down the sessions by doing a rmdir on the sessions if we do configfs or write to some new delete file if we use sysfs.
On Mon, Jun 08, 2020 at 02:02:16PM -0500, Mike Christie wrote: > > > On 6/8/20 11:36 AM, Greg Kroah-Hartman wrote: > > On Mon, Jun 08, 2020 at 10:35:56AM -0500, Mike Christie wrote: > > > > > > > > > On 6/8/20 12:32 AM, Greg Kroah-Hartman wrote: > > > > On Sun, Jun 07, 2020 at 03:35:58PM -0500, Mike Christie wrote: > > > > > +int target_sysfs_add_session(struct se_portal_group *se_tpg, > > > > > + struct se_session *se_sess) > > > > > +{ > > > > > + struct t10_transport_id *tpt_id = se_sess->tpt_id; > > > > > + int ret; > > > > > + > > > > > + if (!try_module_get(se_sess->tfo->module)) > > > > > + return -EINVAL; > > > > > + > > > > > + ret = target_cp_endpoint_strs(se_tpg, se_sess); > > > > > + if (ret) > > > > > + goto put_mod; > > > > > + > > > > > + se_sess->dev.groups = se_sess->tfo->session_attr_groups; > > > > > + ret = device_add(&se_sess->dev); > > > > > + if (ret) { > > > > > + pr_err("Could not add session%d to sysfs. Error %d.\n", > > > > > + se_sess->sid, ret); > > > > > + goto free_ep_strs; > > > > > + } > > > > > + > > > > > + pr_info("TCM added session-%d from [fabric: %s, target: %s, tpg %s, acl: %s] to [initiator port: %s%s%s]", > > > > > + se_sess->sid, se_sess->fabric_name, se_sess->target_name, > > > > > + se_sess->tpg_name, se_sess->acl_name ? se_sess->acl_name : "dynamic", > > > > > + tpt_id->name, tpt_id->session_id ? "," : "", > > > > > + tpt_id->session_id ? tpt_id->session_id : ""); > > > > > > > > You have a 'struct device', so please use it, no need for pr_info(), > > > > always use the dev_*() calls instead. > > > > > > > > but, when drivers and kernel code is all working properly, no need to be > > > > noisy at all, this should just be a dev_dbg() call, right? > > > > > > I liked the info one, because the the code can work correctly, but the > > > remote devices we are connecting to are normally going to hit issues. > > > > > > For example every time the storage network goes down temporarily the driver > > > code will call remove function, then call the add again when it comes back > > > up. Having it always logged helps us figure out the root problem later when > > > the customer only has logs available. > > > > Then make this a tracepoint or something, again, do not be noisy for > > normal system operations. Do you want this to be the case for all of > > What's a special case vs normal? > > I would agree having a SCSI HBA in your system and having it setup during > system boot up is normal. > > I would agree hardware failing is special. > > Having a remote client connect to us, lose its connected then recover seems > special, because it's a failure case. > > Even the initial connection seems like a special event, because the user has > done some extra steps to have the client connect to us. It's like adding a > new disk to the system which we log today or like plugging in or removing a > USB device which we also log. > > > > your hardware devices all the time? > > > > I do, but I'm a kernel developer :) > > For the sys admin and distro debugging type of case, yes, they want this > type of thing logged, because they have done some extra setup to have a > remote client connect to us. When the connection is lost and then re-added > they want all that info logged with the lower level info we are already > logging, so they can follow the time of events. Ok, then, if they really need this, use the standard format for logging and use the dev_info() function instead. That allows them to properly determine what device/driver sent that message at that point in time. thanks, greg k-h
diff --git a/Documentation/ABI/testing/sysfs-scsi-target-session b/Documentation/ABI/testing/sysfs-scsi-target-session new file mode 100644 index 0000000..c8cefcc --- /dev/null +++ b/Documentation/ABI/testing/sysfs-scsi-target-session @@ -0,0 +1,71 @@ +What: /sys/class/scsi_target_session/session-N +Date: June 5, 2020 +KernelVersion: 5.9 +Contact: linux-scsi@vger.kernel.org +Description: The session dir contains a dir for each I_T Nexus. The name of + the dir is "session-" followed by an integer that is unique + across all fabrics. The dir contains files that export info like + the TPG/ACL the session is attached to, SCSI port values, and + fabric and transport specific values. + +What: /sys/class/scsi_target_session/session-N/tcm_endpoint +Date: June 5, 2020 +KernelVersion: 5.9 +Contact: linux-scsi@vger.kernel.org +Description: The tcm_endpoint dir indicates what target_core_mod object + the session is attached to. + +What: /sys/class/scsi_target_session/session-N/tcm_endpoint/acl +Date: June 5, 2020 +KernelVersion: 5.9 +Contact: linux-scsi@vger.kernel.org +Description: Returns the name of the ACL that was used to validate login or + an empty string if no user created ACL was used. + +What: /sys/class/scsi_target_session/session-N/tcm_endpoint/target +Date: June 5, 2020 +KernelVersion: 5.9 +Contact: linux-scsi@vger.kernel.org +Description: Returns the name of the target the session is accessed through. + +What: /sys/class/scsi_target_session/session-N/tcm_endpoint/fabric +Date: June 5, 2020 +KernelVersion: 5.9 +Contact: linux-scsi@vger.kernel.org +Description: Returns the name of the fabric the session is accessed through. + +What: /sys/class/scsi_target_session/session-N/tcm_endpoint/tpg +Date: June 5, 2020 +KernelVersion: 5.9 +Contact: linux-scsi@vger.kernel.org +Description: Returns the name of the tpg the session is accessed through. + +What: /sys/class/scsi_target_session/session-N/transport_id +Date: June 5, 2020 +KernelVersion: 5.9 +Contact: linux-scsi@vger.kernel.org +Description: The transport_id contains the SCSI TransportID values for the + initiator port as defined in SPC4r37. + +What: /sys/class/scsi_target_session/session-N/transport_id/proto +Date: June 5, 2020 +KernelVersion: 5.9 +Contact: linux-scsi@vger.kernel.org +Description: Returns the SCSI protocol identifier in hex defined in SPC4r37. + +What: /sys/class/scsi_target_session/session-N/transport_id/name +Date: June 5, 2020 +KernelVersion: 5.9 +Contact: linux-scsi@vger.kernel.org +Description: Returns the port name/address/id in the protocol specific + TransportID format defined in SPC4r37's. + +What: /sys/class/scsi_target_session/session-N/transport_id/session_id +Date: June 5, 2020 +KernelVersion: 5.9 +Contact: linux-scsi@vger.kernel.org +Description: If is proto=0x5 (iSCSI) and TPID FORMAT=1 this file will exist + and will return the iSCSI Initiator Session ID in ASCII + characters that are the hexadecimal digits converted from the + binary iSCSI initiator session identifier value. If iSCSI and + format=1 is not used by this session this file will not exist. diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h index 8533444..93bf5fed 100644 --- a/drivers/target/target_core_internal.h +++ b/drivers/target/target_core_internal.h @@ -105,6 +105,11 @@ int target_get_pr_transport_id(struct se_node_acl *nacl, const char *target_parse_pr_out_transport_id(struct se_portal_group *tpg, char *buf, u32 *out_tid_len, char **port_nexus_ptr); +/* target_core_sysfs.c */ +int target_sysfs_init_session(struct se_session *sess); +void target_sysfs_init(void); +void target_sysfs_exit(void); + /* target_core_hba.c */ struct se_hba *core_alloc_hba(const char *, u32, u32); int core_delete_hba(struct se_hba *); diff --git a/drivers/target/target_core_sysfs.c b/drivers/target/target_core_sysfs.c new file mode 100644 index 0000000..b29a6f2 --- /dev/null +++ b/drivers/target/target_core_sysfs.c @@ -0,0 +1,265 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include <linux/device.h> +#include <linux/idr.h> +#include <linux/module.h> + +#include <scsi/scsi_proto.h> + +#include <target/target_core_base.h> +#include <target/target_core_fabric.h> +#include "target_core_internal.h" + +static DEFINE_IDA(session_ida); + +#define dev_to_se_sess(_dev) \ + container_of(dev, struct se_session, dev) + +static void target_free_endpoint_strs(struct se_session *se_sess) +{ + kfree(se_sess->tpg_name); + kfree(se_sess->fabric_name); + kfree(se_sess->target_name); + kfree(se_sess->acl_name); +} + +static void target_sysfs_session_release(struct device *dev) +{ + struct se_session *se_sess = dev_to_se_sess(dev); + + target_free_endpoint_strs(se_sess); + ida_simple_remove(&session_ida, se_sess->sid); + + __target_free_session(se_sess); +} + +#define target_attr_show(field) \ +static ssize_t show_target_##field(struct device *dev, \ + struct device_attribute *attr, char *buf) \ +{ \ + struct se_session *se_sess = dev_to_se_sess(dev); \ + \ + if (!se_sess->field##_name) \ + return 0; \ + \ + return snprintf(buf, PAGE_SIZE, "%s", se_sess->field##_name); \ +} + +#define target_attr_str(field) \ + target_attr_show(field) \ +static DEVICE_ATTR(field, S_IRUGO, show_target_##field, NULL) + +target_attr_str(acl); +target_attr_str(tpg); +target_attr_str(target); +target_attr_str(fabric); + +/* + * attrs needed to reference the session's tcm endpoint (acl or tpg) in + * configfs + */ +static struct attribute *target_endpoint_attrs[] = { + &dev_attr_acl.attr, + &dev_attr_tpg.attr, + &dev_attr_target.attr, + &dev_attr_fabric.attr, + NULL, +}; + +static const struct attribute_group target_endpoint_group = { + .name = "tcm_endpoint", + .attrs = target_endpoint_attrs, +}; + +/* transportID attrs */ +#define tpt_id_attr_show(name, fmt_str) \ +static ssize_t show_tpid_##name(struct device *dev, \ + struct device_attribute *attr, char *buf) \ +{ \ + struct se_session *se_sess = dev_to_se_sess(dev); \ + return snprintf(buf, PAGE_SIZE, fmt_str, se_sess->tpt_id->name); \ +} + +#define tpt_id_attr(name, fmt_str) \ + tpt_id_attr_show(name, fmt_str) \ +static DEVICE_ATTR(name, S_IRUGO, show_tpid_##name, NULL) + +tpt_id_attr(proto, "0x%x"); +tpt_id_attr(name, "%s"); + +static ssize_t session_id_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct se_session *se_sess = dev_to_se_sess(dev); + + if (!se_sess->tpt_id->session_id) + return 0; + + return snprintf(buf, PAGE_SIZE, "0x%s", se_sess->tpt_id->session_id); +} + +static DEVICE_ATTR(session_id, S_IRUGO, session_id_show, NULL); + +static struct attribute *tpt_id_attrs[] = { + &dev_attr_proto.attr, + &dev_attr_name.attr, + &dev_attr_session_id.attr, + NULL, +}; + +static const struct attribute_group tpt_id_group = { + .name = "transport_id", + .attrs = tpt_id_attrs, +}; + +static const struct attribute_group *def_session_groups[] = { + &tpt_id_group, + &target_endpoint_group, + NULL, +}; + +static struct class session_class = { + .owner = THIS_MODULE, + .name = "scsi_target_session", + .dev_release = target_sysfs_session_release, + .dev_groups = def_session_groups, +}; + +int target_sysfs_init_session(struct se_session *se_sess) +{ + int ret; + + ret = ida_simple_get(&session_ida, 1, 0, GFP_KERNEL); + if (ret < 0) { + pr_err("Unable to allocate session index.\n"); + return ret; + } + se_sess->sid = ret; + + device_initialize(&se_sess->dev); + se_sess->dev.class = &session_class; + + ret = dev_set_name(&se_sess->dev, "session-%d", se_sess->sid); + if (ret) + goto put_dev; + + return 0; + +put_dev: + put_device(&se_sess->dev); + return ret; +} + +static int target_cp_endpoint_strs(struct se_portal_group *se_tpg, + struct se_session *se_sess) +{ + /* + * Copy configfs dir/object names so userspace can match the session + * to its target, and we also don't have to worry about mixing configfs + * refcounts with sysfs. + */ + if (!se_sess->se_node_acl->dynamic_node_acl) { + se_sess->acl_name = kstrdup(se_sess->se_node_acl->initiatorname, + GFP_KERNEL); + if (!se_sess->acl_name) + return -ENOMEM; + } + + se_sess->target_name = kstrdup(se_tpg->se_tpg_wwn->wwn_group.cg_item.ci_name, + GFP_KERNEL); + if (!se_sess->target_name) + goto free_acl; + + if (se_sess->tfo->fabric_alias) + se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_alias, + GFP_KERNEL); + else + se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_name, + GFP_KERNEL); + if (!se_sess->fabric_name) + goto free_target; + + se_sess->tpg_name = kstrdup(se_tpg->tpg_group.cg_item.ci_name, + GFP_KERNEL); + if (!se_sess->tpg_name) + goto free_fabric; + + return 0; + +free_fabric: + kfree(se_sess->fabric_name); + se_sess->fabric_name = NULL; +free_target: + kfree(se_sess->target_name); + se_sess->target_name = NULL; +free_acl: + kfree(se_sess->acl_name); + se_sess->acl_name = NULL; + return -ENOMEM; +} + +int target_sysfs_add_session(struct se_portal_group *se_tpg, + struct se_session *se_sess) +{ + struct t10_transport_id *tpt_id = se_sess->tpt_id; + int ret; + + if (!try_module_get(se_sess->tfo->module)) + return -EINVAL; + + ret = target_cp_endpoint_strs(se_tpg, se_sess); + if (ret) + goto put_mod; + + se_sess->dev.groups = se_sess->tfo->session_attr_groups; + ret = device_add(&se_sess->dev); + if (ret) { + pr_err("Could not add session%d to sysfs. Error %d.\n", + se_sess->sid, ret); + goto free_ep_strs; + } + + pr_info("TCM added session-%d from [fabric: %s, target: %s, tpg %s, acl: %s] to [initiator port: %s%s%s]", + se_sess->sid, se_sess->fabric_name, se_sess->target_name, + se_sess->tpg_name, se_sess->acl_name ? se_sess->acl_name : "dynamic", + tpt_id->name, tpt_id->session_id ? "," : "", + tpt_id->session_id ? tpt_id->session_id : ""); + + se_sess->sysfs_added = true; + return 0; + +free_ep_strs: + target_free_endpoint_strs(se_sess); +put_mod: + module_put(se_sess->tfo->module); + return ret; +} +EXPORT_SYMBOL(target_sysfs_add_session); + +void target_sysfs_remove_session(struct se_session *se_sess) +{ + struct t10_transport_id *tpt_id = se_sess->tpt_id; + + /* discovery sessions are normally not added to sysfs */ + if (!se_sess->sysfs_added) + return; + se_sess->sysfs_added = false; + + pr_info("TCM removed session-%d from [fabric: %s, target: %s, tpg %s, acl: %s] to [initiator port: %s%s%s]", + se_sess->sid, se_sess->fabric_name, se_sess->target_name, + se_sess->tpg_name, se_sess->acl_name ? se_sess->acl_name : "dynamic", + tpt_id->name, tpt_id->session_id ? "," : "", + tpt_id->session_id ? tpt_id->session_id : ""); + + device_del(&se_sess->dev); +} +EXPORT_SYMBOL(target_sysfs_remove_session); + +void target_sysfs_init(void) +{ + class_register(&session_class); +} + +void target_sysfs_exit(void) +{ + class_unregister(&session_class); +} diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 2e79cce..0d9916b 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -8,6 +8,7 @@ #include <linux/percpu-refcount.h> #include <linux/semaphore.h> /* struct semaphore */ #include <linux/completion.h> +#include <linux/device.h> #define TARGET_CORE_VERSION "v5.0" @@ -635,6 +636,13 @@ struct se_session { wait_queue_head_t cmd_list_wq; void *sess_cmd_map; struct sbitmap_queue sess_tag_pool; + struct device dev; + int sid; + bool sysfs_added; + char *acl_name; + char *tpg_name; + char *target_name; + char *fabric_name; }; struct se_device; diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h index ced377f..2a93daa 100644 --- a/include/target/target_core_fabric.h +++ b/include/target/target_core_fabric.h @@ -105,6 +105,8 @@ struct target_core_fabric_ops { struct configfs_attribute **tfc_tpg_nacl_auth_attrs; struct configfs_attribute **tfc_tpg_nacl_param_attrs; + const struct attribute_group **session_attr_groups; + /* * Set this member variable to true if the SCSI transport protocol * (e.g. iSCSI) requires that the Data-Out buffer is transferred in @@ -145,7 +147,9 @@ void transport_register_session(struct se_portal_group *, void target_put_nacl(struct se_node_acl *); void transport_deregister_session_configfs(struct se_session *); void transport_deregister_session(struct se_session *); - +void target_sysfs_remove_session(struct se_session *se_sess); +int target_sysfs_add_session(struct se_portal_group *se_tpg, + struct se_session *se_sess); void transport_init_se_cmd(struct se_cmd *, const struct target_core_fabric_ops *,
This patch adds a sysfs interface to export the target module's sessions using the driver module class structure. With this we now have a common way to export session info for all fabric modules and we can export info about all sessions not just the first one on a ACL like for iscsi. We can also export more info then PAGE_SIZE bytes for all sessions (looking at iscsi and qla2xxx and the dynamic_sessions file abuse). Here is a tree the new class dir scsi_target_session: session-1/ ├── power │ ├── autosuspend_delay_ms │ ├── control │ ├── runtime_active_time │ ├── runtime_status │ └── runtime_suspended_time ├── subsystem -> ../../../../class/scsi_target_session ├── tcm_endpoint │ ├── acl │ ├── fabric │ ├── target │ └── tpg ├── transport_id │ ├── name │ └── proto └── uevent The Documentation/ABI/testing/sysfs-scsi-target-session file in this patch describes the files and dirs. Userspace apps like tcmu-runner and targetcli, can either: 1. If they have the session ID, then directly look up the session's info like with tagetcli's session sid case. 2. If they have the target side's endpoint object name like the ACL or tpg, then they have to scan each session's tcm_endpoint dir. This should hopefully not be the normal case. For tcmu-runner we will normally scan sysfs, cache the info, then update the cache as we get events about sessions being added/removed. Note that to make it easier for Greg to review I am just including the sysfs parts in this patch. The next patch hooks into the target code and enables it. Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Mike Christie <michael.christie@oracle.com> --- .../ABI/testing/sysfs-scsi-target-session | 71 ++++++ drivers/target/target_core_internal.h | 5 + drivers/target/target_core_sysfs.c | 265 +++++++++++++++++++++ include/target/target_core_base.h | 8 + include/target/target_core_fabric.h | 6 +- 5 files changed, 354 insertions(+), 1 deletion(-) create mode 100644 Documentation/ABI/testing/sysfs-scsi-target-session create mode 100644 drivers/target/target_core_sysfs.c