[PATCH v6 00/17] target: add sysfs support

[PATCH v6 00/17] target: add sysfs support

[Mike Christie]

The following patches made over Martin's for-next branch allow lio to
export info about structs like the se_session that the kernel initiates
creation of via events like initiator login where there is no local user
interaction like a mkdir.

Why sysfs when we have configfs?

I started with configfs and hit bugs like:

commit cc57c07343bd071cdf1915a91a24ab7d40c9b590
Author: Mike Christie <mchristi@redhat.com>
Date:   Sun Jul 15 18:16:17 2018 -0500

    configfs: fix registered group removal

and it turns out that bug was not really a bug and was just how configfs
was meant to work. It seems it was not meant to be used where the kernel
initiates creation of dirs/files as a result of some internal action. It's
more geared to the user initiating the creation, and my patch just lead
to other bugs and was reverted:

commit f19e4ed1e1edbfa3c9ccb9fed17759b7d6db24c6
Author: Al Viro <viro@zeniv.linux.org.uk>
Date:   Thu Aug 29 23:13:30 2019 -0400

    configfs_register_group() shouldn't be (and isn't) called in
rmdirable parts




V5:
- In previous versions we went wild and used kobjects directly and
made dirs and symlinks like crazy. This version just uses the
standard struct class and device interfaces and standard naming.
- Added fabric specific driver session attrs example using the iscsi
target driver to make sure the interface will work for the
broadcom driver.

V4:
- Don't drop const char use in fabric drivers.
- add Documentation/ABI
- use initaitor port prefix instead of generic "session"
- only make session_id file if iSCSI format=1 is being used.

V3:
- drop format field
- delay tpg deletion to allow fabric modules time to remove their
  sessions.
- Added root sessions dir for easier lookup if userspace has the
  session id.
- add session symlink
- use simple ida.
- Fix goto use. Actually moved sysfs addition call to after nego
  to avoid sysfs additions when login ends up failing.
- Dropped target_setup_session callback fixups and dropped the
  init/free session callback for now. It's not immediately needed
  for this base session sysfs info support.

V2:
- rename top level dir to scsi_target
- Fix extra newline
- Copy data that's exported to sysfs so we do not have to worry about
configfs and sysfs refcounts.
- Export session info needed for tracking sessions in userspace and
handling commands like PGRs there (still needs a way to notify userspace
when sessions are added/deleted, but that will be a different set since
the focus is different).

[PATCH 01/17] target: check enforce_pr_isids during registration

[Mike Christie]

Move the check for enforce_pr_isids to the registration code where we
can fail at the time an initiator tries to register a path without an
isid. In its current place in __core_scsi3_locate_pr_reg, it is too
late because it can be registered and be reported in PR in commands and
it is stuck in this state because we cannot unregister it.

Note.
I am including in this patchset, because the 9th patch is built on top.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Lee Duncan <lduncan@suse.com>
---
 drivers/target/target_core_pr.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 5e93169..cd2d32f 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -1176,15 +1176,6 @@ static struct t10_pr_registration *__core_scsi3_locate_pr_reg(
 		 * ISID, then we have found a match.
 		 */
 		if (!pr_reg->isid_present_at_reg) {
-			/*
-			 * Determine if this SCSI device server requires that
-			 * SCSI Intiatior TransportID w/ ISIDs is enforced
-			 * for fabric modules (iSCSI) requiring them.
-			 */
-			if (tpg->se_tpg_tfo->sess_get_initiator_sid != NULL) {
-				if (dev->dev_attrib.enforce_pr_isids)
-					continue;
-			}
 			atomic_inc_mb(&pr_reg->pr_res_holders);
 			spin_unlock(&pr_tmpl->registration_lock);
 			return pr_reg;
@@ -1591,10 +1582,25 @@ static void core_scsi3_lunacl_undepend_item(struct se_dev_entry *se_deve)
 				continue;
 			dest_rtpi = tmp_lun->lun_rtpi;
 
+			iport_ptr = NULL;
 			i_str = target_parse_pr_out_transport_id(tmp_tpg,
 					ptr, &tid_len, &iport_ptr);
 			if (!i_str)
 				continue;
+			/*
+			 * Determine if this SCSI device server requires that
+			 * SCSI Intiatior TransportID w/ ISIDs is enforced
+			 * for fabric modules (iSCSI) requiring them.
+			 */
+			if (tpg->se_tpg_tfo->sess_get_initiator_sid &&
+                            dev->dev_attrib.enforce_pr_isids &&
+			    !iport_ptr) {
+				pr_warn("SPC-PR: enforce_pr_isids is set but a isid has not been sent in the SPEC_I_PT data for %s.",
+					i_str);
+				ret = TCM_INVALID_PARAMETER_LIST;
+				spin_unlock(&dev->se_port_lock);
+				goto out_unmap;
+			}
 
 			atomic_inc_mb(&tmp_tpg->tpg_pr_ref_count);
 			spin_unlock(&dev->se_port_lock);
-- 
1.8.3.1

[PATCH 02/17] target: separate acl name from port ids

[Mike Christie]

The PGR code assumes the ACL name is going to be based on the SPC4
transportID type of values. The problem is that for iSCSI we have an extra
session id as part of the SCSI port id and some fabric modules support or
would like to support non transportID values for the ACL name. For example,
iSCSI and SRP would like to use the source address for the ACL name, but
that is not a valud transportID value that you can get in a PGR request.

This patch adds a new transport_id struct which maps to the SPC4
transportID. In the future it will be used for PGR commands instead of the
ACL name. In this patchset it is used to export the initiator info in the
session's sysfs dir, so tools can display the info and daemons that execute
commands like PGRs in userspace can build a session id to I_T nexus mapping.

In this patch only srp is passing in different values for the transport id
and acl name. The next patches will convert loop, scsi vhost and xen
scsiback that are more complex due to their initiator name emulation.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Juergen Gross <jgross@suse.com>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Lee Duncan <lduncan@suse.com>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c    | 10 +++++---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c |  4 +--
 drivers/scsi/qla2xxx/tcm_qla2xxx.c       |  2 +-
 drivers/target/loopback/tcm_loop.c       |  3 ++-
 drivers/target/sbp/sbp_target.c          |  2 +-
 drivers/target/target_core_fabric_lib.c  | 41 +++++++++++++++++++++++++++++++
 drivers/target/target_core_transport.c   | 42 ++++++++++++++++++++++++--------
 drivers/target/tcm_fc/tfc_sess.c         |  3 ++-
 drivers/usb/gadget/function/f_tcm.c      |  3 ++-
 drivers/vhost/scsi.c                     |  1 +
 drivers/xen/xen-scsiback.c               |  3 ++-
 include/target/target_core_base.h        | 12 +++++++++
 include/target/target_core_fabric.h      |  5 +++-
 13 files changed, 108 insertions(+), 23 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 9855274..d107f63 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -2320,7 +2320,8 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev,
 			break;
 		ch->sess = target_setup_session(&stpg->tpg, tag_num,
 						tag_size, TARGET_PROT_NORMAL,
-						ch->sess_name, ch, NULL);
+						i_port_id + 2, ch->sess_name,
+						ch, NULL);
 	}
 	mutex_unlock(&sport->port_guid_id.mutex);
 
@@ -2329,14 +2330,15 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev,
 		if (!IS_ERR_OR_NULL(ch->sess))
 			break;
 		ch->sess = target_setup_session(&stpg->tpg, tag_num,
-					tag_size, TARGET_PROT_NORMAL, i_port_id,
-					ch, NULL);
+					tag_size, TARGET_PROT_NORMAL,
+					i_port_id + 2, i_port_id, ch, NULL);
 		if (!IS_ERR_OR_NULL(ch->sess))
 			break;
 		/* Retry without leading "0x" */
 		ch->sess = target_setup_session(&stpg->tpg, tag_num,
 						tag_size, TARGET_PROT_NORMAL,
-						i_port_id + 2, ch, NULL);
+						i_port_id + 2, i_port_id + 2,
+						ch, NULL);
 	}
 	mutex_unlock(&sport->port_gid_id.mutex);
 
diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index d9e94e8..31459f3 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -2223,8 +2223,8 @@ static int ibmvscsis_make_nexus(struct ibmvscsis_tport *tport)
 	}
 
 	nexus->se_sess = target_setup_session(&tport->se_tpg, 0, 0,
-					      TARGET_PROT_NORMAL, name, nexus,
-					      NULL);
+					      TARGET_PROT_NORMAL, name, name,
+					      nexus, NULL);
 	if (IS_ERR(nexus->se_sess)) {
 		rc = PTR_ERR(nexus->se_sess);
 		goto transport_init_fail;
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 1f0a185..7b21809 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -1483,7 +1483,7 @@ static int tcm_qla2xxx_check_initiator_node_acl(
 	 */
 	se_sess = target_setup_session(&tpg->se_tpg, num_tags,
 				       sizeof(struct qla_tgt_cmd),
-				       TARGET_PROT_ALL, port_name,
+				       TARGET_PROT_ALL, port_name, port_name,
 				       qlat_sess, tcm_qla2xxx_session_cb);
 	if (IS_ERR(se_sess))
 		return PTR_ERR(se_sess);
diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c
index 3305b47..74aded7 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -742,7 +742,8 @@ static int tcm_loop_make_nexus(
 
 	tl_nexus->se_sess = target_setup_session(&tl_tpg->tl_se_tpg, 0, 0,
 					TARGET_PROT_DIN_PASS | TARGET_PROT_DOUT_PASS,
-					name, tl_nexus, tcm_loop_alloc_sess_cb);
+					name, name, tl_nexus,
+					tcm_loop_alloc_sess_cb);
 	if (IS_ERR(tl_nexus->se_sess)) {
 		ret = PTR_ERR(tl_nexus->se_sess);
 		kfree(tl_nexus);
diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c
index e4a9b9f..2cac4d9 100644
--- a/drivers/target/sbp/sbp_target.c
+++ b/drivers/target/sbp/sbp_target.c
@@ -199,7 +199,7 @@ static struct sbp_session *sbp_session_create(
 	sess->se_sess = target_setup_session(&tpg->se_tpg, 128,
 					     sizeof(struct sbp_target_request),
 					     TARGET_PROT_NORMAL, guid_str,
-					     sess, NULL);
+					     guid_str, sess, NULL);
 	if (IS_ERR(sess->se_sess)) {
 		pr_err("failed to init se_session\n");
 		ret = PTR_ERR(sess->se_sess);
diff --git a/drivers/target/target_core_fabric_lib.c b/drivers/target/target_core_fabric_lib.c
index 1e031d8..e89b3d8 100644
--- a/drivers/target/target_core_fabric_lib.c
+++ b/drivers/target/target_core_fabric_lib.c
@@ -422,3 +422,44 @@ const char *target_parse_pr_out_transport_id(struct se_portal_group *tpg,
 	*out_tid_len = 24;
 	return buf + offset;
 }
+
+struct t10_transport_id *target_create_transport_id(u8 proto, const char *name,
+						    const char *session_id)
+{
+	struct t10_transport_id *tpt_id;
+
+	tpt_id = kzalloc(sizeof(*tpt_id), GFP_KERNEL);
+	if (!tpt_id)
+		return NULL;
+	tpt_id->proto = proto;
+
+	tpt_id->name = kstrdup(name, GFP_KERNEL);
+	if (!tpt_id->name)
+		goto free_tpt_id;
+
+	if (session_id) {
+		tpt_id->session_id = kstrdup(session_id, GFP_KERNEL);
+		if (!tpt_id->session_id)
+			goto free_name;
+	}
+
+	return tpt_id;
+
+free_name:
+	kfree(tpt_id->name);
+free_tpt_id:
+	kfree(tpt_id);
+	return NULL;
+}
+EXPORT_SYMBOL(target_create_transport_id);
+
+void target_free_transport_id(struct t10_transport_id *tpt_id)
+{
+	if (!tpt_id)
+		return;
+
+	kfree(tpt_id->name);
+	kfree(tpt_id->session_id);
+	kfree(tpt_id);
+}
+EXPORT_SYMBOL(target_free_transport_id);
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 264a822..c1b0d15 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -413,15 +413,27 @@ void transport_register_session(
 }
 EXPORT_SYMBOL(transport_register_session);
 
+/**
+ * target_setup_session - alloc and add a session to lio core
+ * @tpg: parent tpg
+ * @tag_num: if non-zero max num in-flight commands.
+ * @tag_size: if tag_num is non-zero, fabric driver's per cmd data in bytes.
+ * @prot_op: bitmask that defines which T10-PI modes are supported.
+ * @tpt_id_name: SCSI TransportID name/address/identifier
+ * @acl_name: name used for se_node_acl
+ * @private: storage for fabric driver accessible via fabric_sess_ptr
+ * @callback: opt function called before session has been added to lio core.
+ */
 struct se_session *
 target_setup_session(struct se_portal_group *tpg,
 		     unsigned int tag_num, unsigned int tag_size,
-		     enum target_prot_op prot_op,
-		     const char *initiatorname, void *private,
+		     enum target_prot_op prot_op, const char *tpt_id_name,
+		     const char *acl_name, void *private,
 		     int (*callback)(struct se_portal_group *,
 				     struct se_session *, void *))
 {
 	struct se_session *sess;
+	int rc;
 
 	/*
 	 * If the fabric driver is using percpu-ida based pre allocation
@@ -435,26 +447,35 @@ struct se_session *
 	if (IS_ERR(sess))
 		return sess;
 
+	sess->tpt_id = target_create_transport_id(tpg->proto_id, tpt_id_name,
+						  NULL);
+	if (!sess->tpt_id) {
+		rc = -ENOMEM;
+		goto free_sess;
+	}
+
 	sess->se_node_acl = core_tpg_check_initiator_node_acl(tpg,
-					(unsigned char *)initiatorname);
+					(unsigned char *)acl_name);
 	if (!sess->se_node_acl) {
-		transport_free_session(sess);
-		return ERR_PTR(-EACCES);
+		rc = -EACCES;
+		goto free_sess;
 	}
 	/*
 	 * Go ahead and perform any remaining fabric setup that is
 	 * required before transport_register_session().
 	 */
 	if (callback != NULL) {
-		int rc = callback(tpg, sess, private);
-		if (rc) {
-			transport_free_session(sess);
-			return ERR_PTR(rc);
-		}
+		rc = callback(tpg, sess, private);
+		if (rc)
+			goto free_sess;
 	}
 
 	transport_register_session(tpg, sess->se_node_acl, sess, private);
 	return sess;
+
+free_sess:
+	transport_free_session(sess);
+	return ERR_PTR(rc);
 }
 EXPORT_SYMBOL(target_setup_session);
 
@@ -579,6 +600,7 @@ void transport_free_session(struct se_session *se_sess)
 		sbitmap_queue_free(&se_sess->sess_tag_pool);
 		kvfree(se_sess->sess_cmd_map);
 	}
+	target_free_transport_id(se_sess->tpt_id);
 	percpu_ref_exit(&se_sess->cmd_count);
 	kmem_cache_free(se_sess_cache, se_sess);
 }
diff --git a/drivers/target/tcm_fc/tfc_sess.c b/drivers/target/tcm_fc/tfc_sess.c
index 4fd6a1d..1d1c460 100644
--- a/drivers/target/tcm_fc/tfc_sess.c
+++ b/drivers/target/tcm_fc/tfc_sess.c
@@ -230,7 +230,8 @@ static struct ft_sess *ft_sess_create(struct ft_tport *tport, u32 port_id,
 	sess->se_sess = target_setup_session(se_tpg, TCM_FC_DEFAULT_TAGS,
 					     sizeof(struct ft_cmd),
 					     TARGET_PROT_NORMAL, &initiatorname[0],
-					     sess, ft_sess_alloc_cb);
+					     &initiatorname[0], sess,
+					     ft_sess_alloc_cb);
 	if (IS_ERR(sess->se_sess)) {
 		int rc = PTR_ERR(sess->se_sess);
 		kfree(sess);
diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
index 3650493..cab3036 100644
--- a/drivers/usb/gadget/function/f_tcm.c
+++ b/drivers/usb/gadget/function/f_tcm.c
@@ -1583,7 +1583,8 @@ static int tcm_usbg_make_nexus(struct usbg_tpg *tpg, char *name)
 						     USB_G_DEFAULT_SESSION_TAGS,
 						     sizeof(struct usbg_cmd),
 						     TARGET_PROT_NORMAL, name,
-						     tv_nexus, usbg_alloc_sess_cb);
+						     name, tv_nexus,
+						     usbg_alloc_sess_cb);
 	if (IS_ERR(tv_nexus->tvn_se_sess)) {
 #define MAKE_NEXUS_MSG "core_tpg_check_initiator_node_acl() failed for %s\n"
 		pr_debug(MAKE_NEXUS_MSG, name);
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index c399522..15aabc2 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1964,6 +1964,7 @@ static int vhost_scsi_make_nexus(struct vhost_scsi_tpg *tpg,
 					VHOST_SCSI_DEFAULT_TAGS,
 					sizeof(struct vhost_scsi_cmd),
 					TARGET_PROT_DIN_PASS | TARGET_PROT_DOUT_PASS,
+					(unsigned char *)name,
 					(unsigned char *)name, tv_nexus,
 					vhost_scsi_nexus_cb);
 	if (IS_ERR(tv_nexus->tvn_se_sess)) {
diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 75c0a2e..93cb386 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -1531,7 +1531,8 @@ static int scsiback_make_nexus(struct scsiback_tpg *tpg,
 						     VSCSI_DEFAULT_SESSION_TAGS,
 						     sizeof(struct vscsibk_pend),
 						     TARGET_PROT_NORMAL, name,
-						     tv_nexus, scsiback_alloc_sess_cb);
+						     name, tv_nexus,
+						     scsiback_alloc_sess_cb);
 	if (IS_ERR(tv_nexus->tvn_se_sess)) {
 		kfree(tv_nexus);
 		ret = -ENOMEM;
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 6d4a694..2e79cce 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -331,6 +331,17 @@ struct t10_wwn {
 	struct list_head t10_vpd_list;
 };
 
+struct t10_transport_id {
+	/* The format=0 transport specific port id/name value. */
+	char *name;
+	/*
+	 * If proto is iSCSI and it's using format=1, then this is set to the
+	 * initiator session id string defined in spc4r37 table 508.
+	 */
+	char *session_id;
+	u8 proto;
+};
+
 struct t10_pr_registration {
 	/* Used for fabrics that contain WWN+ISID */
 #define PR_REG_ISID_LEN				16
@@ -610,6 +621,7 @@ static inline struct se_node_acl *fabric_stat_to_nacl(struct config_item *item)
 struct se_session {
 	unsigned		sess_tearing_down:1;
 	u64			sess_bin_isid;
+	struct t10_transport_id	*tpt_id;
 	enum target_prot_op	sup_prot_ops;
 	enum target_prot_type	sess_prot_type;
 	struct se_node_acl	*se_node_acl;
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index 063f133..af1dd81 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -125,9 +125,12 @@ struct target_core_fabric_ops {
 int target_depend_item(struct config_item *item);
 void target_undepend_item(struct config_item *item);
 
+struct t10_transport_id *target_create_transport_id(u8, const char *,
+						    const char *);
+void target_free_transport_id(struct t10_transport_id *);
 struct se_session *target_setup_session(struct se_portal_group *,
 		unsigned int, unsigned int, enum target_prot_op prot_op,
-		const char *, void *,
+		const char *, const char *, void *,
 		int (*callback)(struct se_portal_group *,
 				struct se_session *, void *));
 void target_remove_session(struct se_session *);
-- 
1.8.3.1

Re: [PATCH 02/17] target: separate acl name from port ids

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2414 bytes --]

On Sun, Jun 07, 2020 at 03:35:49PM -0500, Mike Christie wrote:
> The PGR code assumes the ACL name is going to be based on the SPC4
> transportID type of values. The problem is that for iSCSI we have an extra
> session id as part of the SCSI port id and some fabric modules support or
> would like to support non transportID values for the ACL name. For example,
> iSCSI and SRP would like to use the source address for the ACL name, but
> that is not a valud transportID value that you can get in a PGR request.
> 
> This patch adds a new transport_id struct which maps to the SPC4
> transportID. In the future it will be used for PGR commands instead of the
> ACL name. In this patchset it is used to export the initiator info in the
> session's sysfs dir, so tools can display the info and daemons that execute
> commands like PGRs in userspace can build a session id to I_T nexus mapping.
> 
> In this patch only srp is passing in different values for the transport id
> and acl name. The next patches will convert loop, scsi vhost and xen
> scsiback that are more complex due to their initiator name emulation.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Juergen Gross <jgross@suse.com>
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Lee Duncan <lduncan@suse.com>
> ---
>  drivers/infiniband/ulp/srpt/ib_srpt.c    | 10 +++++---
>  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c |  4 +--
>  drivers/scsi/qla2xxx/tcm_qla2xxx.c       |  2 +-
>  drivers/target/loopback/tcm_loop.c       |  3 ++-
>  drivers/target/sbp/sbp_target.c          |  2 +-
>  drivers/target/target_core_fabric_lib.c  | 41 +++++++++++++++++++++++++++++++
>  drivers/target/target_core_transport.c   | 42 ++++++++++++++++++++++++--------
>  drivers/target/tcm_fc/tfc_sess.c         |  3 ++-
>  drivers/usb/gadget/function/f_tcm.c      |  3 ++-
>  drivers/vhost/scsi.c                     |  1 +
>  drivers/xen/xen-scsiback.c               |  3 ++-
>  include/target/target_core_base.h        | 12 +++++++++
>  include/target/target_core_fabric.h      |  5 +++-
>  13 files changed, 108 insertions(+), 23 deletions(-)

For drivers/vhost/scsi.c:
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 02/17] target: separate acl name from port ids

[Himanshu Madhani]

On 6/7/20 3:35 PM, Mike Christie wrote:
> The PGR code assumes the ACL name is going to be based on the SPC4
> transportID type of values. The problem is that for iSCSI we have an extra
> session id as part of the SCSI port id and some fabric modules support or
> would like to support non transportID values for the ACL name. For example,
> iSCSI and SRP would like to use the source address for the ACL name, but
> that is not a valud transportID value that you can get in a PGR request.
> 
> This patch adds a new transport_id struct which maps to the SPC4
> transportID. In the future it will be used for PGR commands instead of the
> ACL name. In this patchset it is used to export the initiator info in the
> session's sysfs dir, so tools can display the info and daemons that execute
> commands like PGRs in userspace can build a session id to I_T nexus mapping.
> 
> In this patch only srp is passing in different values for the transport id
> and acl name. The next patches will convert loop, scsi vhost and xen
> scsiback that are more complex due to their initiator name emulation.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Juergen Gross <jgross@suse.com>
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Lee Duncan <lduncan@suse.com>
> ---
>   drivers/infiniband/ulp/srpt/ib_srpt.c    | 10 +++++---
>   drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c |  4 +--
>   drivers/scsi/qla2xxx/tcm_qla2xxx.c       |  2 +-
>   drivers/target/loopback/tcm_loop.c       |  3 ++-
>   drivers/target/sbp/sbp_target.c          |  2 +-
>   drivers/target/target_core_fabric_lib.c  | 41 +++++++++++++++++++++++++++++++
>   drivers/target/target_core_transport.c   | 42 ++++++++++++++++++++++++--------
>   drivers/target/tcm_fc/tfc_sess.c         |  3 ++-
>   drivers/usb/gadget/function/f_tcm.c      |  3 ++-
>   drivers/vhost/scsi.c                     |  1 +
>   drivers/xen/xen-scsiback.c               |  3 ++-
>   include/target/target_core_base.h        | 12 +++++++++
>   include/target/target_core_fabric.h      |  5 +++-
>   13 files changed, 108 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> index 9855274..d107f63 100644
> --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> @@ -2320,7 +2320,8 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev,
>   			break;
>   		ch->sess = target_setup_session(&stpg->tpg, tag_num,
>   						tag_size, TARGET_PROT_NORMAL,
> -						ch->sess_name, ch, NULL);
> +						i_port_id + 2, ch->sess_name,
> +						ch, NULL);
>   	}
>   	mutex_unlock(&sport->port_guid_id.mutex);
>   
> @@ -2329,14 +2330,15 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev,
>   		if (!IS_ERR_OR_NULL(ch->sess))
>   			break;
>   		ch->sess = target_setup_session(&stpg->tpg, tag_num,
> -					tag_size, TARGET_PROT_NORMAL, i_port_id,
> -					ch, NULL);
> +					tag_size, TARGET_PROT_NORMAL,
> +					i_port_id + 2, i_port_id, ch, NULL);
>   		if (!IS_ERR_OR_NULL(ch->sess))
>   			break;
>   		/* Retry without leading "0x" */
>   		ch->sess = target_setup_session(&stpg->tpg, tag_num,
>   						tag_size, TARGET_PROT_NORMAL,
> -						i_port_id + 2, ch, NULL);
> +						i_port_id + 2, i_port_id + 2,
> +						ch, NULL);
>   	}
>   	mutex_unlock(&sport->port_gid_id.mutex);
>   
> diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> index d9e94e8..31459f3 100644
> --- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> +++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> @@ -2223,8 +2223,8 @@ static int ibmvscsis_make_nexus(struct ibmvscsis_tport *tport)
>   	}
>   
>   	nexus->se_sess = target_setup_session(&tport->se_tpg, 0, 0,
> -					      TARGET_PROT_NORMAL, name, nexus,
> -					      NULL);
> +					      TARGET_PROT_NORMAL, name, name,
> +					      nexus, NULL);
>   	if (IS_ERR(nexus->se_sess)) {
>   		rc = PTR_ERR(nexus->se_sess);
>   		goto transport_init_fail;
> diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> index 1f0a185..7b21809 100644
> --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
> @@ -1483,7 +1483,7 @@ static int tcm_qla2xxx_check_initiator_node_acl(
>   	 */
>   	se_sess = target_setup_session(&tpg->se_tpg, num_tags,
>   				       sizeof(struct qla_tgt_cmd),
> -				       TARGET_PROT_ALL, port_name,
> +				       TARGET_PROT_ALL, port_name, port_name,
>   				       qlat_sess, tcm_qla2xxx_session_cb);
>   	if (IS_ERR(se_sess))
>   		return PTR_ERR(se_sess);
> diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c
> index 3305b47..74aded7 100644
> --- a/drivers/target/loopback/tcm_loop.c
> +++ b/drivers/target/loopback/tcm_loop.c
> @@ -742,7 +742,8 @@ static int tcm_loop_make_nexus(
>   
>   	tl_nexus->se_sess = target_setup_session(&tl_tpg->tl_se_tpg, 0, 0,
>   					TARGET_PROT_DIN_PASS | TARGET_PROT_DOUT_PASS,
> -					name, tl_nexus, tcm_loop_alloc_sess_cb);
> +					name, name, tl_nexus,
> +					tcm_loop_alloc_sess_cb);
>   	if (IS_ERR(tl_nexus->se_sess)) {
>   		ret = PTR_ERR(tl_nexus->se_sess);
>   		kfree(tl_nexus);
> diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c
> index e4a9b9f..2cac4d9 100644
> --- a/drivers/target/sbp/sbp_target.c
> +++ b/drivers/target/sbp/sbp_target.c
> @@ -199,7 +199,7 @@ static struct sbp_session *sbp_session_create(
>   	sess->se_sess = target_setup_session(&tpg->se_tpg, 128,
>   					     sizeof(struct sbp_target_request),
>   					     TARGET_PROT_NORMAL, guid_str,
> -					     sess, NULL);
> +					     guid_str, sess, NULL);
>   	if (IS_ERR(sess->se_sess)) {
>   		pr_err("failed to init se_session\n");
>   		ret = PTR_ERR(sess->se_sess);
> diff --git a/drivers/target/target_core_fabric_lib.c b/drivers/target/target_core_fabric_lib.c
> index 1e031d8..e89b3d8 100644
> --- a/drivers/target/target_core_fabric_lib.c
> +++ b/drivers/target/target_core_fabric_lib.c
> @@ -422,3 +422,44 @@ const char *target_parse_pr_out_transport_id(struct se_portal_group *tpg,
>   	*out_tid_len = 24;
>   	return buf + offset;
>   }
> +
> +struct t10_transport_id *target_create_transport_id(u8 proto, const char *name,
> +						    const char *session_id)
> +{
> +	struct t10_transport_id *tpt_id;
> +
> +	tpt_id = kzalloc(sizeof(*tpt_id), GFP_KERNEL);
> +	if (!tpt_id)
> +		return NULL;
> +	tpt_id->proto = proto;
> +
> +	tpt_id->name = kstrdup(name, GFP_KERNEL);
> +	if (!tpt_id->name)
> +		goto free_tpt_id;
> +
> +	if (session_id) {
> +		tpt_id->session_id = kstrdup(session_id, GFP_KERNEL);
> +		if (!tpt_id->session_id)
> +			goto free_name;
> +	}
> +
> +	return tpt_id;
> +
> +free_name:
> +	kfree(tpt_id->name);
> +free_tpt_id:
> +	kfree(tpt_id);
> +	return NULL;
> +}
> +EXPORT_SYMBOL(target_create_transport_id);
> +
> +void target_free_transport_id(struct t10_transport_id *tpt_id)
> +{
> +	if (!tpt_id)
> +		return;
> +
> +	kfree(tpt_id->name);
> +	kfree(tpt_id->session_id);
> +	kfree(tpt_id);
> +}
> +EXPORT_SYMBOL(target_free_transport_id);
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index 264a822..c1b0d15 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -413,15 +413,27 @@ void transport_register_session(
>   }
>   EXPORT_SYMBOL(transport_register_session);
>   
> +/**
> + * target_setup_session - alloc and add a session to lio core
> + * @tpg: parent tpg
> + * @tag_num: if non-zero max num in-flight commands.
> + * @tag_size: if tag_num is non-zero, fabric driver's per cmd data in bytes.
> + * @prot_op: bitmask that defines which T10-PI modes are supported.
> + * @tpt_id_name: SCSI TransportID name/address/identifier
> + * @acl_name: name used for se_node_acl
> + * @private: storage for fabric driver accessible via fabric_sess_ptr
> + * @callback: opt function called before session has been added to lio core.
> + */
>   struct se_session *
>   target_setup_session(struct se_portal_group *tpg,
>   		     unsigned int tag_num, unsigned int tag_size,
> -		     enum target_prot_op prot_op,
> -		     const char *initiatorname, void *private,
> +		     enum target_prot_op prot_op, const char *tpt_id_name,
> +		     const char *acl_name, void *private,
>   		     int (*callback)(struct se_portal_group *,
>   				     struct se_session *, void *))
>   {
>   	struct se_session *sess;
> +	int rc;
>   
>   	/*
>   	 * If the fabric driver is using percpu-ida based pre allocation
> @@ -435,26 +447,35 @@ struct se_session *
>   	if (IS_ERR(sess))
>   		return sess;
>   
> +	sess->tpt_id = target_create_transport_id(tpg->proto_id, tpt_id_name,
> +						  NULL);
> +	if (!sess->tpt_id) {
> +		rc = -ENOMEM;
> +		goto free_sess;
> +	}
> +
>   	sess->se_node_acl = core_tpg_check_initiator_node_acl(tpg,
> -					(unsigned char *)initiatorname);
> +					(unsigned char *)acl_name);
>   	if (!sess->se_node_acl) {
> -		transport_free_session(sess);
> -		return ERR_PTR(-EACCES);
> +		rc = -EACCES;
> +		goto free_sess;
>   	}
>   	/*
>   	 * Go ahead and perform any remaining fabric setup that is
>   	 * required before transport_register_session().
>   	 */
>   	if (callback != NULL) {
> -		int rc = callback(tpg, sess, private);
> -		if (rc) {
> -			transport_free_session(sess);
> -			return ERR_PTR(rc);
> -		}
> +		rc = callback(tpg, sess, private);
> +		if (rc)
> +			goto free_sess;
>   	}
>   
>   	transport_register_session(tpg, sess->se_node_acl, sess, private);
>   	return sess;
> +
> +free_sess:
> +	transport_free_session(sess);
> +	return ERR_PTR(rc);
>   }
>   EXPORT_SYMBOL(target_setup_session);
>   
> @@ -579,6 +600,7 @@ void transport_free_session(struct se_session *se_sess)
>   		sbitmap_queue_free(&se_sess->sess_tag_pool);
>   		kvfree(se_sess->sess_cmd_map);
>   	}
> +	target_free_transport_id(se_sess->tpt_id);
>   	percpu_ref_exit(&se_sess->cmd_count);
>   	kmem_cache_free(se_sess_cache, se_sess);
>   }
> diff --git a/drivers/target/tcm_fc/tfc_sess.c b/drivers/target/tcm_fc/tfc_sess.c
> index 4fd6a1d..1d1c460 100644
> --- a/drivers/target/tcm_fc/tfc_sess.c
> +++ b/drivers/target/tcm_fc/tfc_sess.c
> @@ -230,7 +230,8 @@ static struct ft_sess *ft_sess_create(struct ft_tport *tport, u32 port_id,
>   	sess->se_sess = target_setup_session(se_tpg, TCM_FC_DEFAULT_TAGS,
>   					     sizeof(struct ft_cmd),
>   					     TARGET_PROT_NORMAL, &initiatorname[0],
> -					     sess, ft_sess_alloc_cb);
> +					     &initiatorname[0], sess,
> +					     ft_sess_alloc_cb);
>   	if (IS_ERR(sess->se_sess)) {
>   		int rc = PTR_ERR(sess->se_sess);
>   		kfree(sess);
> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index 3650493..cab3036 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -1583,7 +1583,8 @@ static int tcm_usbg_make_nexus(struct usbg_tpg *tpg, char *name)
>   						     USB_G_DEFAULT_SESSION_TAGS,
>   						     sizeof(struct usbg_cmd),
>   						     TARGET_PROT_NORMAL, name,
> -						     tv_nexus, usbg_alloc_sess_cb);
> +						     name, tv_nexus,
> +						     usbg_alloc_sess_cb);
>   	if (IS_ERR(tv_nexus->tvn_se_sess)) {
>   #define MAKE_NEXUS_MSG "core_tpg_check_initiator_node_acl() failed for %s\n"
>   		pr_debug(MAKE_NEXUS_MSG, name);
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index c399522..15aabc2 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1964,6 +1964,7 @@ static int vhost_scsi_make_nexus(struct vhost_scsi_tpg *tpg,
>   					VHOST_SCSI_DEFAULT_TAGS,
>   					sizeof(struct vhost_scsi_cmd),
>   					TARGET_PROT_DIN_PASS | TARGET_PROT_DOUT_PASS,
> +					(unsigned char *)name,
>   					(unsigned char *)name, tv_nexus,
>   					vhost_scsi_nexus_cb);
>   	if (IS_ERR(tv_nexus->tvn_se_sess)) {
> diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
> index 75c0a2e..93cb386 100644
> --- a/drivers/xen/xen-scsiback.c
> +++ b/drivers/xen/xen-scsiback.c
> @@ -1531,7 +1531,8 @@ static int scsiback_make_nexus(struct scsiback_tpg *tpg,
>   						     VSCSI_DEFAULT_SESSION_TAGS,
>   						     sizeof(struct vscsibk_pend),
>   						     TARGET_PROT_NORMAL, name,
> -						     tv_nexus, scsiback_alloc_sess_cb);
> +						     name, tv_nexus,
> +						     scsiback_alloc_sess_cb);
>   	if (IS_ERR(tv_nexus->tvn_se_sess)) {
>   		kfree(tv_nexus);
>   		ret = -ENOMEM;
> diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
> index 6d4a694..2e79cce 100644
> --- a/include/target/target_core_base.h
> +++ b/include/target/target_core_base.h
> @@ -331,6 +331,17 @@ struct t10_wwn {
>   	struct list_head t10_vpd_list;
>   };
>   
> +struct t10_transport_id {
> +	/* The format=0 transport specific port id/name value. */
> +	char *name;
> +	/*
> +	 * If proto is iSCSI and it's using format=1, then this is set to the
> +	 * initiator session id string defined in spc4r37 table 508.
> +	 */
> +	char *session_id;
> +	u8 proto;
> +};
> +
>   struct t10_pr_registration {
>   	/* Used for fabrics that contain WWN+ISID */
>   #define PR_REG_ISID_LEN				16
> @@ -610,6 +621,7 @@ static inline struct se_node_acl *fabric_stat_to_nacl(struct config_item *item)
>   struct se_session {
>   	unsigned		sess_tearing_down:1;
>   	u64			sess_bin_isid;
> +	struct t10_transport_id	*tpt_id;
>   	enum target_prot_op	sup_prot_ops;
>   	enum target_prot_type	sess_prot_type;
>   	struct se_node_acl	*se_node_acl;
> diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
> index 063f133..af1dd81 100644
> --- a/include/target/target_core_fabric.h
> +++ b/include/target/target_core_fabric.h
> @@ -125,9 +125,12 @@ struct target_core_fabric_ops {
>   int target_depend_item(struct config_item *item);
>   void target_undepend_item(struct config_item *item);
>   
> +struct t10_transport_id *target_create_transport_id(u8, const char *,
> +						    const char *);
> +void target_free_transport_id(struct t10_transport_id *);
>   struct se_session *target_setup_session(struct se_portal_group *,
>   		unsigned int, unsigned int, enum target_prot_op prot_op,
> -		const char *, void *,
> +		const char *, const char *, void *,
>   		int (*callback)(struct se_portal_group *,
>   				struct se_session *, void *));
>   void target_remove_session(struct se_session *);
> 

for drivers/scsi/qla2xxx/tcm_qla2xxx.c

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

-- 
Himanshu Madhani                     Oracle Linux Engineering

[PATCH 03/17] target: add helper to parse acl and transport name

[Mike Christie]

The drivers that emulate the initiator port id (loop, scsi vhost, xen scsiback)
do almost the extact same parsing when making their I_T_nexus. This adds a
helper that parses out the acl name and port name from the user buffer, so
these types of drivers drop prefixes like "naa." when they need to for the
SCSI SPC4 TransportID SAS address, but then keep it for the LIO ACL name.

The next patches will then convert those drivers.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Juergen Gross <jgross@suse.com>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Lee Duncan <lduncan@suse.com>
---
 drivers/target/target_core_fabric_lib.c | 73 +++++++++++++++++++++++++++++++++
 include/target/target_core_fabric.h     |  2 +
 2 files changed, 75 insertions(+)

diff --git a/drivers/target/target_core_fabric_lib.c b/drivers/target/target_core_fabric_lib.c
index e89b3d8..81ed7d5 100644
--- a/drivers/target/target_core_fabric_lib.c
+++ b/drivers/target/target_core_fabric_lib.c
@@ -423,6 +423,79 @@ const char *target_parse_pr_out_transport_id(struct se_portal_group *tpg,
 	return buf + offset;
 }
 
+/**
+ * target_parse_emulated_name - parse TransportID and acl name from user buffer
+ * @proto_id: SCSI protocol identifier
+ * @user_buf: buffer with emualted name to extract acl and TransportID from
+ * @acl_name: buffer to store se_node_acl name in
+ * @max_name_len: len of acl_name buffer
+ * @tpt_id_name: Pointer to the TransportID name will be stored here.
+ */
+int target_parse_emulated_name(u8 proto_id, const char *user_buf,
+			       unsigned char *acl_name, int max_name_len,
+			       unsigned char **tpt_id_name)
+{
+	int user_len = strlen(user_buf);
+	char *proto_prefix, *name_start;
+
+	if (user_len >= max_name_len) {
+		pr_err("Emulated name: %s, exceeds max: %d\n", user_buf,
+		       max_name_len);
+		return -EINVAL;
+	}
+
+	switch (proto_id) {
+	case SCSI_PROTOCOL_SAS:
+		proto_prefix = "naa.";
+		break;
+	case SCSI_PROTOCOL_FCP:
+		proto_prefix = "fc.";
+		break;
+	case SCSI_PROTOCOL_ISCSI:
+		proto_prefix = "iqn.";
+		break;
+	default:
+		pr_err("Unsupported proto_id: 0x%02x\n", proto_id);
+		return -EINVAL;
+	}
+
+	name_start = strstr(user_buf, proto_prefix);
+	if (!name_start) {
+		pr_err("Invalid emulated name %s. Must start with %s\n",
+		       user_buf, proto_prefix);
+		return -EINVAL;
+	}
+
+	switch (proto_id) {
+	case SCSI_PROTOCOL_SAS:
+		sprintf(acl_name, name_start);
+		break;
+	case SCSI_PROTOCOL_FCP:
+		sprintf(acl_name, &name_start[3]); /* Skip over "fc." */
+		break;
+	case SCSI_PROTOCOL_ISCSI:
+		sprintf(acl_name, name_start);
+		break;
+	}
+
+	if (acl_name[user_len - 1] == '\n')
+		acl_name[user_len - 1] = '\0';
+
+	if (proto_id == SCSI_PROTOCOL_SAS) {
+		/*
+		 * target_setup_session will want the naa. prefix to match
+		 * the ACL name, but the t10 transport id only wants the
+		 * address.
+		 */
+		*tpt_id_name = acl_name + 4;
+	} else {
+		*tpt_id_name = acl_name;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(target_parse_emulated_name);
+
 struct t10_transport_id *target_create_transport_id(u8 proto, const char *name,
 						    const char *session_id)
 {
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index af1dd81..0113e1c 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -125,6 +125,8 @@ struct target_core_fabric_ops {
 int target_depend_item(struct config_item *item);
 void target_undepend_item(struct config_item *item);
 
+int target_parse_emulated_name(u8, const char *, unsigned char *, int,
+			       unsigned char **);
 struct t10_transport_id *target_create_transport_id(u8, const char *,
 						    const char *);
 void target_free_transport_id(struct t10_transport_id *);
-- 
1.8.3.1

[PATCH 04/17] tcm loop: use target_parse_emulated_name

[Mike Christie]

Use target_parse_emulated_name so the acl and SCSI names are properly
formatted.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Lee Duncan <lduncan@suse.com>
---
 drivers/target/loopback/tcm_loop.c | 65 ++++++--------------------------------
 1 file changed, 10 insertions(+), 55 deletions(-)

diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c
index 74aded7..64e5f1f 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -725,7 +725,8 @@ static int tcm_loop_alloc_sess_cb(struct se_portal_group *se_tpg,
 
 static int tcm_loop_make_nexus(
 	struct tcm_loop_tpg *tl_tpg,
-	const char *name)
+	const char *tpt_id_name,
+	const char *acl_name)
 {
 	struct tcm_loop_hba *tl_hba = tl_tpg->tl_hba;
 	struct tcm_loop_nexus *tl_nexus;
@@ -742,7 +743,7 @@ static int tcm_loop_make_nexus(
 
 	tl_nexus->se_sess = target_setup_session(&tl_tpg->tl_se_tpg, 0, 0,
 					TARGET_PROT_DIN_PASS | TARGET_PROT_DOUT_PASS,
-					name, name, tl_nexus,
+					tpt_id_name, acl_name, tl_nexus,
 					tcm_loop_alloc_sess_cb);
 	if (IS_ERR(tl_nexus->se_sess)) {
 		ret = PTR_ERR(tl_nexus->se_sess);
@@ -751,7 +752,7 @@ static int tcm_loop_make_nexus(
 	}
 
 	pr_debug("TCM_Loop_ConfigFS: Established I_T Nexus to emulated %s Initiator Port: %s\n",
-		 tcm_loop_dump_proto_id(tl_hba), name);
+		 tcm_loop_dump_proto_id(tl_hba), acl_name);
 	return 0;
 }
 
@@ -814,7 +815,7 @@ static ssize_t tcm_loop_tpg_nexus_store(struct config_item *item,
 	struct tcm_loop_tpg *tl_tpg = container_of(se_tpg,
 			struct tcm_loop_tpg, tl_se_tpg);
 	struct tcm_loop_hba *tl_hba = tl_tpg->tl_hba;
-	unsigned char i_port[TL_WWN_ADDR_LEN], *ptr, *port_ptr;
+	unsigned char i_port[TL_WWN_ADDR_LEN], *tpt_id_name;
 	int ret;
 	/*
 	 * Shutdown the active I_T nexus if 'NULL' is passed..
@@ -823,59 +824,13 @@ static ssize_t tcm_loop_tpg_nexus_store(struct config_item *item,
 		ret = tcm_loop_drop_nexus(tl_tpg);
 		return (!ret) ? count : ret;
 	}
-	/*
-	 * Otherwise make sure the passed virtual Initiator port WWN matches
-	 * the fabric protocol_id set in tcm_loop_make_scsi_hba(), and call
-	 * tcm_loop_make_nexus()
-	 */
-	if (strlen(page) >= TL_WWN_ADDR_LEN) {
-		pr_err("Emulated NAA Sas Address: %s, exceeds max: %d\n",
-		       page, TL_WWN_ADDR_LEN);
-		return -EINVAL;
-	}
-	snprintf(&i_port[0], TL_WWN_ADDR_LEN, "%s", page);
 
-	ptr = strstr(i_port, "naa.");
-	if (ptr) {
-		if (tl_hba->tl_proto_id != SCSI_PROTOCOL_SAS) {
-			pr_err("Passed SAS Initiator Port %s does not match target port protoid: %s\n",
-			       i_port, tcm_loop_dump_proto_id(tl_hba));
-			return -EINVAL;
-		}
-		port_ptr = &i_port[0];
-		goto check_newline;
-	}
-	ptr = strstr(i_port, "fc.");
-	if (ptr) {
-		if (tl_hba->tl_proto_id != SCSI_PROTOCOL_FCP) {
-			pr_err("Passed FCP Initiator Port %s does not match target port protoid: %s\n",
-			       i_port, tcm_loop_dump_proto_id(tl_hba));
-			return -EINVAL;
-		}
-		port_ptr = &i_port[3]; /* Skip over "fc." */
-		goto check_newline;
-	}
-	ptr = strstr(i_port, "iqn.");
-	if (ptr) {
-		if (tl_hba->tl_proto_id != SCSI_PROTOCOL_ISCSI) {
-			pr_err("Passed iSCSI Initiator Port %s does not match target port protoid: %s\n",
-			       i_port, tcm_loop_dump_proto_id(tl_hba));
-			return -EINVAL;
-		}
-		port_ptr = &i_port[0];
-		goto check_newline;
-	}
-	pr_err("Unable to locate prefix for emulated Initiator Port: %s\n",
-	       i_port);
-	return -EINVAL;
-	/*
-	 * Clear any trailing newline for the NAA WWN
-	 */
-check_newline:
-	if (i_port[strlen(i_port)-1] == '\n')
-		i_port[strlen(i_port)-1] = '\0';
+	ret = target_parse_emulated_name(tl_hba->tl_proto_id, page, i_port,
+					 TL_WWN_ADDR_LEN, &tpt_id_name);
+	if (ret)
+		return ret;
 
-	ret = tcm_loop_make_nexus(tl_tpg, port_ptr);
+	ret = tcm_loop_make_nexus(tl_tpg, tpt_id_name, i_port);
 	if (ret < 0)
 		return ret;
 
-- 
1.8.3.1

[PATCH 05/17] vhost scsi: use target_parse_emulated_name

[Mike Christie]

Use target_parse_emulated_name so the acl and SCSI names are
properly formatted.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/vhost/scsi.c | 69 +++++++++-------------------------------------------
 1 file changed, 11 insertions(+), 58 deletions(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 15aabc2..37f66f8 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1938,7 +1938,8 @@ static int vhost_scsi_nexus_cb(struct se_portal_group *se_tpg,
 }
 
 static int vhost_scsi_make_nexus(struct vhost_scsi_tpg *tpg,
-				const char *name)
+				const char *tpt_id_name,
+				const char *acl_name)
 {
 	struct vhost_scsi_nexus *tv_nexus;
 
@@ -1964,8 +1965,8 @@ static int vhost_scsi_make_nexus(struct vhost_scsi_tpg *tpg,
 					VHOST_SCSI_DEFAULT_TAGS,
 					sizeof(struct vhost_scsi_cmd),
 					TARGET_PROT_DIN_PASS | TARGET_PROT_DOUT_PASS,
-					(unsigned char *)name,
-					(unsigned char *)name, tv_nexus,
+					(unsigned char *)tpt_id_name,
+					(unsigned char *)acl_name, tv_nexus,
 					vhost_scsi_nexus_cb);
 	if (IS_ERR(tv_nexus->tvn_se_sess)) {
 		mutex_unlock(&tpg->tv_tpg_mutex);
@@ -2056,7 +2057,7 @@ static ssize_t vhost_scsi_tpg_nexus_store(struct config_item *item,
 	struct vhost_scsi_tpg *tpg = container_of(se_tpg,
 				struct vhost_scsi_tpg, se_tpg);
 	struct vhost_scsi_tport *tport_wwn = tpg->tport;
-	unsigned char i_port[VHOST_SCSI_NAMELEN], *ptr, *port_ptr;
+	unsigned char i_port[VHOST_SCSI_NAMELEN], *tpt_id_name;
 	int ret;
 	/*
 	 * Shutdown the active I_T nexus if 'NULL' is passed..
@@ -2065,62 +2066,14 @@ static ssize_t vhost_scsi_tpg_nexus_store(struct config_item *item,
 		ret = vhost_scsi_drop_nexus(tpg);
 		return (!ret) ? count : ret;
 	}
-	/*
-	 * Otherwise make sure the passed virtual Initiator port WWN matches
-	 * the fabric protocol_id set in vhost_scsi_make_tport(), and call
-	 * vhost_scsi_make_nexus().
-	 */
-	if (strlen(page) >= VHOST_SCSI_NAMELEN) {
-		pr_err("Emulated NAA Sas Address: %s, exceeds"
-				" max: %d\n", page, VHOST_SCSI_NAMELEN);
-		return -EINVAL;
-	}
-	snprintf(&i_port[0], VHOST_SCSI_NAMELEN, "%s", page);
 
-	ptr = strstr(i_port, "naa.");
-	if (ptr) {
-		if (tport_wwn->tport_proto_id != SCSI_PROTOCOL_SAS) {
-			pr_err("Passed SAS Initiator Port %s does not"
-				" match target port protoid: %s\n", i_port,
-				vhost_scsi_dump_proto_id(tport_wwn));
-			return -EINVAL;
-		}
-		port_ptr = &i_port[0];
-		goto check_newline;
-	}
-	ptr = strstr(i_port, "fc.");
-	if (ptr) {
-		if (tport_wwn->tport_proto_id != SCSI_PROTOCOL_FCP) {
-			pr_err("Passed FCP Initiator Port %s does not"
-				" match target port protoid: %s\n", i_port,
-				vhost_scsi_dump_proto_id(tport_wwn));
-			return -EINVAL;
-		}
-		port_ptr = &i_port[3]; /* Skip over "fc." */
-		goto check_newline;
-	}
-	ptr = strstr(i_port, "iqn.");
-	if (ptr) {
-		if (tport_wwn->tport_proto_id != SCSI_PROTOCOL_ISCSI) {
-			pr_err("Passed iSCSI Initiator Port %s does not"
-				" match target port protoid: %s\n", i_port,
-				vhost_scsi_dump_proto_id(tport_wwn));
-			return -EINVAL;
-		}
-		port_ptr = &i_port[0];
-		goto check_newline;
-	}
-	pr_err("Unable to locate prefix for emulated Initiator Port:"
-			" %s\n", i_port);
-	return -EINVAL;
-	/*
-	 * Clear any trailing newline for the NAA WWN
-	 */
-check_newline:
-	if (i_port[strlen(i_port)-1] == '\n')
-		i_port[strlen(i_port)-1] = '\0';
+	ret = target_parse_emulated_name(tport_wwn->tport_proto_id, page,
+					 i_port, VHOST_SCSI_NAMELEN,
+					 &tpt_id_name);
+	if (ret)
+		return ret;
 
-	ret = vhost_scsi_make_nexus(tpg, port_ptr);
+	ret = vhost_scsi_make_nexus(tpg, tpt_id_name, i_port);
 	if (ret < 0)
 		return ret;
 
-- 
1.8.3.1

Re: [PATCH 05/17] vhost scsi: use target_parse_emulated_name

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 633 bytes --]

On Sun, Jun 07, 2020 at 03:35:52PM -0500, Mike Christie wrote:
> Use target_parse_emulated_name so the acl and SCSI names are
> properly formatted.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/vhost/scsi.c | 69 +++++++++-------------------------------------------
>  1 file changed, 11 insertions(+), 58 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 06/17] xen scsiback: use target_parse_emulated_name

[Mike Christie]

Use target_parse_emulated_name so the acl and SCSI names are
properly formatted.

Cc: Juergen Gross <jgross@suse.com>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Acked-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/xen/xen-scsiback.c | 66 ++++++++--------------------------------------
 1 file changed, 11 insertions(+), 55 deletions(-)

diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 93cb386..f70b6da 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -1509,7 +1509,8 @@ static int scsiback_alloc_sess_cb(struct se_portal_group *se_tpg,
 }
 
 static int scsiback_make_nexus(struct scsiback_tpg *tpg,
-				const char *name)
+				const char *tpt_id_name,
+				const char *acl_name)
 {
 	struct scsiback_nexus *tv_nexus;
 	int ret = 0;
@@ -1530,8 +1531,9 @@ static int scsiback_make_nexus(struct scsiback_tpg *tpg,
 	tv_nexus->tvn_se_sess = target_setup_session(&tpg->se_tpg,
 						     VSCSI_DEFAULT_SESSION_TAGS,
 						     sizeof(struct vscsibk_pend),
-						     TARGET_PROT_NORMAL, name,
-						     name, tv_nexus,
+						     TARGET_PROT_NORMAL,
+						     tpt_id_name, acl_name,
+						     tv_nexus,
 						     scsiback_alloc_sess_cb);
 	if (IS_ERR(tv_nexus->tvn_se_sess)) {
 		kfree(tv_nexus);
@@ -1619,7 +1621,7 @@ static ssize_t scsiback_tpg_nexus_store(struct config_item *item,
 	struct scsiback_tpg *tpg = container_of(se_tpg,
 				struct scsiback_tpg, se_tpg);
 	struct scsiback_tport *tport_wwn = tpg->tport;
-	unsigned char i_port[VSCSI_NAMELEN], *ptr, *port_ptr;
+	unsigned char i_port[VSCSI_NAMELEN], *tpt_id_name;
 	int ret;
 	/*
 	 * Shutdown the active I_T nexus if 'NULL' is passed.
@@ -1628,59 +1630,13 @@ static ssize_t scsiback_tpg_nexus_store(struct config_item *item,
 		ret = scsiback_drop_nexus(tpg);
 		return (!ret) ? count : ret;
 	}
-	/*
-	 * Otherwise make sure the passed virtual Initiator port WWN matches
-	 * the fabric protocol_id set in scsiback_make_tport(), and call
-	 * scsiback_make_nexus().
-	 */
-	if (strlen(page) >= VSCSI_NAMELEN) {
-		pr_err("Emulated NAA Sas Address: %s, exceeds max: %d\n",
-			page, VSCSI_NAMELEN);
-		return -EINVAL;
-	}
-	snprintf(&i_port[0], VSCSI_NAMELEN, "%s", page);
 
-	ptr = strstr(i_port, "naa.");
-	if (ptr) {
-		if (tport_wwn->tport_proto_id != SCSI_PROTOCOL_SAS) {
-			pr_err("Passed SAS Initiator Port %s does not match target port protoid: %s\n",
-				i_port, scsiback_dump_proto_id(tport_wwn));
-			return -EINVAL;
-		}
-		port_ptr = &i_port[0];
-		goto check_newline;
-	}
-	ptr = strstr(i_port, "fc.");
-	if (ptr) {
-		if (tport_wwn->tport_proto_id != SCSI_PROTOCOL_FCP) {
-			pr_err("Passed FCP Initiator Port %s does not match target port protoid: %s\n",
-				i_port, scsiback_dump_proto_id(tport_wwn));
-			return -EINVAL;
-		}
-		port_ptr = &i_port[3]; /* Skip over "fc." */
-		goto check_newline;
-	}
-	ptr = strstr(i_port, "iqn.");
-	if (ptr) {
-		if (tport_wwn->tport_proto_id != SCSI_PROTOCOL_ISCSI) {
-			pr_err("Passed iSCSI Initiator Port %s does not match target port protoid: %s\n",
-				i_port, scsiback_dump_proto_id(tport_wwn));
-			return -EINVAL;
-		}
-		port_ptr = &i_port[0];
-		goto check_newline;
-	}
-	pr_err("Unable to locate prefix for emulated Initiator Port: %s\n",
-		i_port);
-	return -EINVAL;
-	/*
-	 * Clear any trailing newline for the NAA WWN
-	 */
-check_newline:
-	if (i_port[strlen(i_port) - 1] == '\n')
-		i_port[strlen(i_port) - 1] = '\0';
+	ret = target_parse_emulated_name(tport_wwn->tport_proto_id, page,
+					 i_port, VSCSI_NAMELEN, &tpt_id_name);
+	if (ret)
+		return ret;
 
-	ret = scsiback_make_nexus(tpg, port_ptr);
+	ret = scsiback_make_nexus(tpg, tpt_id_name, i_port);
 	if (ret < 0)
 		return ret;
 
-- 
1.8.3.1

[PATCH 07/17] iscsi target: setup transport_id

[Mike Christie]

The iscsi target does its own session setup. This patch updates the
driver so it sets up the transport id.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/target/iscsi/iscsi_target_nego.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_nego.c b/drivers/target/iscsi/iscsi_target_nego.c
index 685d771..c44613a2 100644
--- a/drivers/target/iscsi/iscsi_target_nego.c
+++ b/drivers/target/iscsi/iscsi_target_nego.c
@@ -1032,6 +1032,20 @@ static void iscsi_initiatorname_tolower(
 	}
 }
 
+static int iscsi_setup_i_tpt_id(struct iscsi_session *sess, char *iname)
+{
+	char isid_buf[13];
+
+	sprintf(isid_buf, "%6phN", sess->isid);
+
+	sess->se_sess->tpt_id = target_create_transport_id(SCSI_PROTOCOL_ISCSI,
+							   iname, isid_buf);
+	if (!sess->se_sess->tpt_id)
+		return -ENOMEM;
+
+	return 0;
+}
+
 /*
  * Processes the first Login Request..
  */
@@ -1260,11 +1274,21 @@ int iscsi_target_locate_portal(
 	tag_size = sizeof(struct iscsi_cmd) + conn->conn_transport->priv_size;
 
 	ret = transport_alloc_session_tags(sess->se_sess, tag_num, tag_size);
-	if (ret < 0) {
-		iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
-				    ISCSI_LOGIN_STATUS_NO_RESOURCES);
-		ret = -1;
+	if (ret < 0)
+		goto return_oom;
+
+	if (conn->tpg != iscsit_global->discovery_tpg) {
+		if (iscsi_setup_i_tpt_id(sess, i_buf))
+			/* tags and nacl released when session is freed */
+			goto return_oom;
 	}
+
+	goto out;
+
+return_oom:
+	iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
+			    ISCSI_LOGIN_STATUS_NO_RESOURCES);
+	ret = -1;
 out:
 	kfree(tmpbuf);
 	return ret;
-- 
1.8.3.1

[PATCH 08/17] target: use tpt_id in target_stat_iport_port_ident_show

[Mike Christie]

Use the tpt_id session id instead of sess_get_initiator_sid.

Note that for userspace compat this patch continues the behavior:

1. Still add the "+i+" even if there is no session_id.
2. Use the acl initiatorname instead of the transportID port/addr/name.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/target/target_core_stat.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/target/target_core_stat.c b/drivers/target/target_core_stat.c
index 237309d..3eb1b9b 100644
--- a/drivers/target/target_core_stat.c
+++ b/drivers/target/target_core_stat.c
@@ -1308,9 +1308,7 @@ static ssize_t target_stat_iport_port_ident_show(struct config_item *item,
 	struct se_lun_acl *lacl = iport_to_lacl(item);
 	struct se_node_acl *nacl = lacl->se_lun_nacl;
 	struct se_session *se_sess;
-	struct se_portal_group *tpg;
 	ssize_t ret;
-	unsigned char buf[64];
 
 	spin_lock_irq(&nacl->nacl_sess_lock);
 	se_sess = nacl->nacl_sess;
@@ -1319,13 +1317,9 @@ static ssize_t target_stat_iport_port_ident_show(struct config_item *item,
 		return -ENODEV;
 	}
 
-	tpg = nacl->se_tpg;
-	/* scsiAttIntrPortName+scsiAttIntrPortIdentifier */
-	memset(buf, 0, 64);
-	if (tpg->se_tpg_tfo->sess_get_initiator_sid != NULL)
-		tpg->se_tpg_tfo->sess_get_initiator_sid(se_sess, buf, 64);
-
-	ret = snprintf(page, PAGE_SIZE, "%s+i+%s\n", nacl->initiatorname, buf);
+	ret = snprintf(page, PAGE_SIZE, "%s+i+%s\n", nacl->initiatorname,
+		       se_sess->tpt_id->session_id ? se_sess->tpt_id->session_id :
+		       "");
 	spin_unlock_irq(&nacl->nacl_sess_lock);
 	return ret;
 }
-- 
1.8.3.1

[PATCH 09/17] target: drop sess_get_initiator_sid from PR code

[Mike Christie]

Use the transport id session id in the PR related code.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/target/target_core_pr.c        | 23 ++++-------------------
 drivers/target/target_core_transport.c | 11 ++++-------
 2 files changed, 8 insertions(+), 26 deletions(-)

diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index cd2d32f..fbe3638 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -1204,17 +1204,7 @@ static struct t10_pr_registration *core_scsi3_locate_pr_reg(
 	struct se_node_acl *nacl,
 	struct se_session *sess)
 {
-	struct se_portal_group *tpg = nacl->se_tpg;
-	unsigned char buf[PR_REG_ISID_LEN], *isid_ptr = NULL;
-
-	if (tpg->se_tpg_tfo->sess_get_initiator_sid != NULL) {
-		memset(&buf[0], 0, PR_REG_ISID_LEN);
-		tpg->se_tpg_tfo->sess_get_initiator_sid(sess, &buf[0],
-					PR_REG_ISID_LEN);
-		isid_ptr = &buf[0];
-	}
-
-	return __core_scsi3_locate_pr_reg(dev, nacl, isid_ptr);
+	return __core_scsi3_locate_pr_reg(dev, nacl, sess->tpt_id->session_id);
 }
 
 static void core_scsi3_put_pr_reg(struct t10_pr_registration *pr_reg)
@@ -1592,7 +1582,7 @@ static void core_scsi3_lunacl_undepend_item(struct se_dev_entry *se_deve)
 			 * SCSI Intiatior TransportID w/ ISIDs is enforced
 			 * for fabric modules (iSCSI) requiring them.
 			 */
-			if (tpg->se_tpg_tfo->sess_get_initiator_sid &&
+			if (se_sess->tpt_id->session_id &&
                             dev->dev_attrib.enforce_pr_isids &&
 			    !iport_ptr) {
 				pr_warn("SPC-PR: enforce_pr_isids is set but a isid has not been sent in the SPEC_I_PT data for %s.",
@@ -2057,7 +2047,7 @@ static sense_reason_t core_scsi3_update_and_write_aptpl(struct se_device *dev, b
 	struct se_portal_group *se_tpg;
 	struct t10_pr_registration *pr_reg, *pr_reg_p, *pr_reg_tmp;
 	struct t10_reservation *pr_tmpl = &dev->t10_pr;
-	unsigned char isid_buf[PR_REG_ISID_LEN], *isid_ptr = NULL;
+	unsigned char *isid_ptr = NULL;
 	sense_reason_t ret = TCM_NO_SENSE;
 	int pr_holder = 0, type;
 
@@ -2067,12 +2057,7 @@ static sense_reason_t core_scsi3_update_and_write_aptpl(struct se_device *dev, b
 	}
 	se_tpg = se_sess->se_tpg;
 
-	if (se_tpg->se_tpg_tfo->sess_get_initiator_sid) {
-		memset(&isid_buf[0], 0, PR_REG_ISID_LEN);
-		se_tpg->se_tpg_tfo->sess_get_initiator_sid(se_sess, &isid_buf[0],
-				PR_REG_ISID_LEN);
-		isid_ptr = &isid_buf[0];
-	}
+	isid_ptr = se_sess->tpt_id->session_id;
 	/*
 	 * Follow logic from spc4r17 Section 5.7.7, Register Behaviors Table 47
 	 */
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index c1b0d15..5d6d736 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -343,7 +343,7 @@ void __transport_register_session(
 	void *fabric_sess_ptr)
 {
 	const struct target_core_fabric_ops *tfo = se_tpg->se_tpg_tfo;
-	unsigned char buf[PR_REG_ISID_LEN];
+	unsigned char *sid;
 	unsigned long flags;
 
 	se_sess->se_tpg = se_tpg;
@@ -374,12 +374,9 @@ void __transport_register_session(
 		 * If the fabric module supports an ISID based TransportID,
 		 * save this value in binary from the fabric I_T Nexus now.
 		 */
-		if (se_tpg->se_tpg_tfo->sess_get_initiator_sid != NULL) {
-			memset(&buf[0], 0, PR_REG_ISID_LEN);
-			se_tpg->se_tpg_tfo->sess_get_initiator_sid(se_sess,
-					&buf[0], PR_REG_ISID_LEN);
-			se_sess->sess_bin_isid = get_unaligned_be64(&buf[0]);
-		}
+		sid = se_sess->tpt_id->session_id;
+		if (sid)
+			se_sess->sess_bin_isid = get_unaligned_be64(sid);
 
 		spin_lock_irqsave(&se_nacl->nacl_sess_lock, flags);
 		/*
-- 
1.8.3.1

[PATCH 10/17] target: drop sess_get_initiator_sid

[Mike Christie]

sess_get_initiator_sid is no longer used. Drop it.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c        |  1 -
 drivers/scsi/qla2xxx/tcm_qla2xxx.c           |  2 --
 drivers/target/iscsi/iscsi_target_configfs.c | 13 -------------
 drivers/target/tcm_fc/tfc_conf.c             |  1 -
 drivers/usb/gadget/function/f_tcm.c          |  1 -
 drivers/vhost/scsi.c                         |  1 -
 drivers/xen/xen-scsiback.c                   |  1 -
 include/target/target_core_fabric.h          |  6 ------
 8 files changed, 26 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index d107f63..3cda73c 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -3844,7 +3844,6 @@ static ssize_t srpt_wwn_version_show(struct config_item *item, char *buf)
 	.check_stop_free		= srpt_check_stop_free,
 	.close_session			= srpt_close_session,
 	.sess_get_index			= srpt_sess_get_index,
-	.sess_get_initiator_sid		= NULL,
 	.write_pending			= srpt_write_pending,
 	.set_default_node_attributes	= srpt_set_default_node_attrs,
 	.get_cmd_state			= srpt_get_tcm_cmd_state,
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 7b21809..fff695a 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -1852,7 +1852,6 @@ static ssize_t tcm_qla2xxx_wwn_version_show(struct config_item *item,
 	.release_cmd			= tcm_qla2xxx_release_cmd,
 	.close_session			= tcm_qla2xxx_close_session,
 	.sess_get_index			= tcm_qla2xxx_sess_get_index,
-	.sess_get_initiator_sid		= NULL,
 	.write_pending			= tcm_qla2xxx_write_pending,
 	.set_default_node_attributes	= tcm_qla2xxx_set_default_node_attrs,
 	.get_cmd_state			= tcm_qla2xxx_get_cmd_state,
@@ -1892,7 +1891,6 @@ static ssize_t tcm_qla2xxx_wwn_version_show(struct config_item *item,
 	.release_cmd			= tcm_qla2xxx_release_cmd,
 	.close_session			= tcm_qla2xxx_close_session,
 	.sess_get_index			= tcm_qla2xxx_sess_get_index,
-	.sess_get_initiator_sid		= NULL,
 	.write_pending			= tcm_qla2xxx_write_pending,
 	.set_default_node_attributes	= tcm_qla2xxx_set_default_node_attrs,
 	.get_cmd_state			= tcm_qla2xxx_get_cmd_state,
diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c
index 0fa1d57..4513740 100644
--- a/drivers/target/iscsi/iscsi_target_configfs.c
+++ b/drivers/target/iscsi/iscsi_target_configfs.c
@@ -1349,18 +1349,6 @@ static u32 lio_sess_get_index(struct se_session *se_sess)
 	return sess->session_index;
 }
 
-static u32 lio_sess_get_initiator_sid(
-	struct se_session *se_sess,
-	unsigned char *buf,
-	u32 size)
-{
-	struct iscsi_session *sess = se_sess->fabric_sess_ptr;
-	/*
-	 * iSCSI Initiator Session Identifier from RFC-3720.
-	 */
-	return snprintf(buf, size, "%6phN", sess->isid);
-}
-
 static int lio_queue_data_in(struct se_cmd *se_cmd)
 {
 	struct iscsi_cmd *cmd = container_of(se_cmd, struct iscsi_cmd, se_cmd);
@@ -1545,7 +1533,6 @@ static void lio_release_cmd(struct se_cmd *se_cmd)
 	.release_cmd			= lio_release_cmd,
 	.close_session			= lio_tpg_close_session,
 	.sess_get_index			= lio_sess_get_index,
-	.sess_get_initiator_sid		= lio_sess_get_initiator_sid,
 	.write_pending			= lio_write_pending,
 	.set_default_node_attributes	= lio_set_default_node_attributes,
 	.get_cmd_state			= iscsi_get_cmd_state,
diff --git a/drivers/target/tcm_fc/tfc_conf.c b/drivers/target/tcm_fc/tfc_conf.c
index 1a38c98..870b7bb 100644
--- a/drivers/target/tcm_fc/tfc_conf.c
+++ b/drivers/target/tcm_fc/tfc_conf.c
@@ -427,7 +427,6 @@ static u32 ft_tpg_get_inst_index(struct se_portal_group *se_tpg)
 	.release_cmd =			ft_release_cmd,
 	.close_session =		ft_sess_close,
 	.sess_get_index =		ft_sess_get_index,
-	.sess_get_initiator_sid =	NULL,
 	.write_pending =		ft_write_pending,
 	.set_default_node_attributes =	ft_set_default_node_attr,
 	.get_cmd_state =		ft_get_cmd_state,
diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
index cab3036..e81bdbd 100644
--- a/drivers/usb/gadget/function/f_tcm.c
+++ b/drivers/usb/gadget/function/f_tcm.c
@@ -1716,7 +1716,6 @@ static int usbg_check_stop_free(struct se_cmd *se_cmd)
 	.tpg_get_inst_index		= usbg_tpg_get_inst_index,
 	.release_cmd			= usbg_release_cmd,
 	.sess_get_index			= usbg_sess_get_index,
-	.sess_get_initiator_sid		= NULL,
 	.write_pending			= usbg_send_write_request,
 	.set_default_node_attributes	= usbg_set_default_node_attrs,
 	.get_cmd_state			= usbg_get_cmd_state,
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 37f66f8..d7c9bfc 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -2245,7 +2245,6 @@ static void vhost_scsi_drop_tport(struct se_wwn *wwn)
 	.release_cmd			= vhost_scsi_release_cmd,
 	.check_stop_free		= vhost_scsi_check_stop_free,
 	.sess_get_index			= vhost_scsi_sess_get_index,
-	.sess_get_initiator_sid		= NULL,
 	.write_pending			= vhost_scsi_write_pending,
 	.set_default_node_attributes	= vhost_scsi_set_default_node_attrs,
 	.get_cmd_state			= vhost_scsi_get_cmd_state,
diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index f70b6da..883b15c 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -1769,7 +1769,6 @@ static int scsiback_check_false(struct se_portal_group *se_tpg)
 	.check_stop_free		= scsiback_check_stop_free,
 	.release_cmd			= scsiback_release_cmd,
 	.sess_get_index			= scsiback_sess_get_index,
-	.sess_get_initiator_sid		= NULL,
 	.write_pending			= scsiback_write_pending,
 	.set_default_node_attributes	= scsiback_set_default_node_attrs,
 	.get_cmd_state			= scsiback_get_cmd_state,
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index 0113e1c..ced377f 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -67,12 +67,6 @@ struct target_core_fabric_ops {
 	void (*release_cmd)(struct se_cmd *);
 	void (*close_session)(struct se_session *);
 	u32 (*sess_get_index)(struct se_session *);
-	/*
-	 * Used only for SCSI fabrics that contain multi-value TransportIDs
-	 * (like iSCSI).  All other SCSI fabrics should set this to NULL.
-	 */
-	u32 (*sess_get_initiator_sid)(struct se_session *,
-				      unsigned char *, u32);
 	int (*write_pending)(struct se_cmd *);
 	void (*set_default_node_attributes)(struct se_node_acl *);
 	int (*get_cmd_state)(struct se_cmd *);
-- 
1.8.3.1

[PATCH 11/17] target: add session sysfs class support

[Mike Christie]

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;
+
+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 *,
-- 
1.8.3.1

Re: [PATCH 11/17] target: add session sysfs class support

[Greg Kroah-Hartman]

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

Re: [PATCH 11/17] target: add session sysfs class support

[Mike Christie]

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,

Re: [PATCH 11/17] target: add session sysfs class support

[Greg Kroah-Hartman]

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

Re: [PATCH 11/17] target: add session sysfs class support

[Mike Christie]

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.

Re: [PATCH 11/17] target: add session sysfs class support

[Greg Kroah-Hartman]

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

Re: [PATCH 11/17] target: add session sysfs class support

[Hannes Reinecke]

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
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH 11/17] target: add session sysfs class support

[Mike Christie]

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.

Re: [PATCH 11/17] target: add session sysfs class support

[Michael Christie]

> 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.

Re: [PATCH 11/17] target: add session sysfs class support

[Michael Christie]

> 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.

Re: [PATCH 11/17] target: add session sysfs class support

[Bodo Stroesser]

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 *,
> 

Re: [PATCH 11/17] target: add session sysfs class support

[Mike Christie]

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.

Re: [PATCH 11/17] target: add session sysfs class support

[Greg Kroah-Hartman]

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

Re: [PATCH 11/17] target: add session sysfs class support

[Greg Kroah-Hartman]

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()

[PATCH 12/17] target: hook most target users into sysfs API

[Mike Christie]

This hooks lio core and all the targets but iscsi into the sysfs API.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/target/Makefile                |  1 +
 drivers/target/target_core_configfs.c  |  3 +++
 drivers/target/target_core_internal.h  |  1 +
 drivers/target/target_core_transport.c | 38 ++++++++++++++++++++++++++++------
 4 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/drivers/target/Makefile b/drivers/target/Makefile
index 4563474..4a7246e 100644
--- a/drivers/target/Makefile
+++ b/drivers/target/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 target_core_mod-y		:= target_core_configfs.o \
+				   target_core_sysfs.o \
 				   target_core_device.o \
 				   target_core_fabric_configfs.o \
 				   target_core_fabric_lib.o \
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index ff82b21f..c23552b 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -3537,6 +3537,7 @@ static int __init target_core_init_configfs(void)
 		goto out;
 
 	target_init_dbroot();
+	target_sysfs_init();
 
 	return 0;
 
@@ -3555,6 +3556,8 @@ static int __init target_core_init_configfs(void)
 
 static void __exit target_core_exit_configfs(void)
 {
+	target_sysfs_exit();
+
 	configfs_remove_default_groups(&alua_lu_gps_group);
 	configfs_remove_default_groups(&alua_group);
 	configfs_remove_default_groups(&target_core_hbagroup);
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index 93bf5fed..23d9a0e 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -157,6 +157,7 @@ void	transport_dump_dev_info(struct se_device *, struct se_lun *,
 bool	target_check_wce(struct se_device *dev);
 bool	target_check_fua(struct se_device *dev);
 void	__target_execute_cmd(struct se_cmd *, bool);
+void	__target_free_session(struct se_session *se_sess);
 
 /* target_core_stat.c */
 void	target_stat_setup_dev_default_groups(struct se_device *);
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 5d6d736..34da12a 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -22,6 +22,7 @@
 #include <linux/module.h>
 #include <linux/ratelimit.h>
 #include <linux/vmalloc.h>
+#include <linux/device.h>
 #include <asm/unaligned.h>
 #include <net/sock.h>
 #include <net/tcp.h>
@@ -251,14 +252,27 @@ struct se_session *transport_alloc_session(enum target_prot_op sup_prot_ops)
 				" se_sess_cache\n");
 		return ERR_PTR(-ENOMEM);
 	}
-	ret = transport_init_session(se_sess);
-	if (ret < 0) {
+
+	ret = target_sysfs_init_session(se_sess);
+	if (ret) {
 		kmem_cache_free(se_sess_cache, se_sess);
 		return ERR_PTR(ret);
 	}
-	se_sess->sup_prot_ops = sup_prot_ops;
+	/*
+	 * After this point we switch from handlng the freeing of the sess
+	 * to using the se_sess->dev refcounting.
+	 */
 
+	ret = transport_init_session(se_sess);
+	if (ret < 0)
+		goto put_dev;
+
+	se_sess->sup_prot_ops = sup_prot_ops;
 	return se_sess;
+
+put_dev:
+	put_device(&se_sess->dev);
+	return ERR_PTR(ret);
 }
 EXPORT_SYMBOL(transport_alloc_session);
 
@@ -457,6 +471,10 @@ struct se_session *
 		rc = -EACCES;
 		goto free_sess;
 	}
+
+	rc = target_sysfs_add_session(tpg, sess);
+	if (rc)
+		goto free_sess;
 	/*
 	 * Go ahead and perform any remaining fabric setup that is
 	 * required before transport_register_session().
@@ -464,12 +482,14 @@ struct se_session *
 	if (callback != NULL) {
 		rc = callback(tpg, sess, private);
 		if (rc)
-			goto free_sess;
+			goto rm_sysfs;
 	}
 
 	transport_register_session(tpg, sess->se_node_acl, sess, private);
 	return sess;
 
+rm_sysfs:
+	target_sysfs_remove_session(sess);
 free_sess:
 	transport_free_session(sess);
 	return ERR_PTR(rc);
@@ -597,12 +617,17 @@ void transport_free_session(struct se_session *se_sess)
 		sbitmap_queue_free(&se_sess->sess_tag_pool);
 		kvfree(se_sess->sess_cmd_map);
 	}
-	target_free_transport_id(se_sess->tpt_id);
 	percpu_ref_exit(&se_sess->cmd_count);
-	kmem_cache_free(se_sess_cache, se_sess);
+	put_device(&se_sess->dev);
 }
 EXPORT_SYMBOL(transport_free_session);
 
+void __target_free_session(struct se_session *se_sess)
+{
+	target_free_transport_id(se_sess->tpt_id);
+	kmem_cache_free(se_sess_cache, se_sess);
+}
+
 static int target_release_res(struct se_device *dev, void *data)
 {
 	struct se_session *sess = data;
@@ -651,6 +676,7 @@ void transport_deregister_session(struct se_session *se_sess)
 
 void target_remove_session(struct se_session *se_sess)
 {
+	target_sysfs_remove_session(se_sess);
 	transport_deregister_session_configfs(se_sess);
 	transport_deregister_session(se_sess);
 }
-- 
1.8.3.1

Re: [PATCH 12/17] target: hook most target users into sysfs API

[Hannes Reinecke]

On 6/7/20 10:35 PM, Mike Christie wrote:
> This hooks lio core and all the targets but iscsi into the sysfs API.
> 
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> ---
>   drivers/target/Makefile                |  1 +
>   drivers/target/target_core_configfs.c  |  3 +++
>   drivers/target/target_core_internal.h  |  1 +
>   drivers/target/target_core_transport.c | 38 ++++++++++++++++++++++++++++------
>   4 files changed, 37 insertions(+), 6 deletions(-)
> Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH 12/17] target: hook most target users into sysfs API

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 6535 bytes --]

Hi Mike,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mkp-scsi/for-next]
[also build test ERROR on scsi/for-next vhost/linux-next xen-tip/linux-next v5.7 next-20200605]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Mike-Christie/target-add-sysfs-support/20200608-044027
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: sparc-allyesconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sparc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

Note: the linux-review/Mike-Christie/target-add-sysfs-support/20200608-044027 HEAD fc08cd1eb111346ece0dceae6f01be21509d2c49 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>, old ones prefixed by <<):

drivers/target/target_core_sysfs.c: In function 'target_cp_endpoint_strs':
>> drivers/target/target_core_sysfs.c:172:13: error: 'struct se_session' has no member named 'tfo'
172 |  if (se_sess->tfo->fabric_alias)
|             ^~
drivers/target/target_core_sysfs.c:173:41: error: 'struct se_session' has no member named 'tfo'
173 |   se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_alias,
|                                         ^~
drivers/target/target_core_sysfs.c:176:41: error: 'struct se_session' has no member named 'tfo'
176 |   se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_name,
|                                         ^~
drivers/target/target_core_sysfs.c: In function 'target_sysfs_add_session':
drivers/target/target_core_sysfs.c:206:29: error: 'struct se_session' has no member named 'tfo'
206 |  if (!try_module_get(se_sess->tfo->module))
|                             ^~
drivers/target/target_core_sysfs.c:213:31: error: 'struct se_session' has no member named 'tfo'
213 |  se_sess->dev.groups = se_sess->tfo->session_attr_groups;
|                               ^~
drivers/target/target_core_sysfs.c:233:20: error: 'struct se_session' has no member named 'tfo'
233 |  module_put(se_sess->tfo->module);
|                    ^~

vim +172 drivers/target/target_core_sysfs.c

03975f2939ad69 Mike Christie 2020-06-07  151  
03975f2939ad69 Mike Christie 2020-06-07  152  static int target_cp_endpoint_strs(struct se_portal_group *se_tpg,
03975f2939ad69 Mike Christie 2020-06-07  153  				   struct se_session *se_sess)
03975f2939ad69 Mike Christie 2020-06-07  154  {
03975f2939ad69 Mike Christie 2020-06-07  155  	/*
03975f2939ad69 Mike Christie 2020-06-07  156  	 * Copy configfs dir/object names so userspace can match the session
03975f2939ad69 Mike Christie 2020-06-07  157  	 * to its target, and we also don't have to worry about mixing configfs
03975f2939ad69 Mike Christie 2020-06-07  158  	 * refcounts with sysfs.
03975f2939ad69 Mike Christie 2020-06-07  159  	 */
03975f2939ad69 Mike Christie 2020-06-07  160  	if (!se_sess->se_node_acl->dynamic_node_acl) {
03975f2939ad69 Mike Christie 2020-06-07  161  		se_sess->acl_name = kstrdup(se_sess->se_node_acl->initiatorname,
03975f2939ad69 Mike Christie 2020-06-07  162  					    GFP_KERNEL);
03975f2939ad69 Mike Christie 2020-06-07  163  		if (!se_sess->acl_name)
03975f2939ad69 Mike Christie 2020-06-07  164  			return -ENOMEM;
03975f2939ad69 Mike Christie 2020-06-07  165  	}
03975f2939ad69 Mike Christie 2020-06-07  166  
03975f2939ad69 Mike Christie 2020-06-07  167  	se_sess->target_name = kstrdup(se_tpg->se_tpg_wwn->wwn_group.cg_item.ci_name,
03975f2939ad69 Mike Christie 2020-06-07  168  				       GFP_KERNEL);
03975f2939ad69 Mike Christie 2020-06-07  169  	if (!se_sess->target_name)
03975f2939ad69 Mike Christie 2020-06-07  170  		goto free_acl;
03975f2939ad69 Mike Christie 2020-06-07  171  
03975f2939ad69 Mike Christie 2020-06-07 @172  	if (se_sess->tfo->fabric_alias)
03975f2939ad69 Mike Christie 2020-06-07  173  		se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_alias,
03975f2939ad69 Mike Christie 2020-06-07  174  					       GFP_KERNEL);
03975f2939ad69 Mike Christie 2020-06-07  175  	else
03975f2939ad69 Mike Christie 2020-06-07  176  		se_sess->fabric_name = kstrdup(se_sess->tfo->fabric_name,
03975f2939ad69 Mike Christie 2020-06-07  177  					       GFP_KERNEL);
03975f2939ad69 Mike Christie 2020-06-07  178  	if (!se_sess->fabric_name)
03975f2939ad69 Mike Christie 2020-06-07  179  		goto free_target;
03975f2939ad69 Mike Christie 2020-06-07  180  
03975f2939ad69 Mike Christie 2020-06-07  181  	se_sess->tpg_name = kstrdup(se_tpg->tpg_group.cg_item.ci_name,
03975f2939ad69 Mike Christie 2020-06-07  182  				    GFP_KERNEL);
03975f2939ad69 Mike Christie 2020-06-07  183  	if (!se_sess->tpg_name)
03975f2939ad69 Mike Christie 2020-06-07  184  		goto free_fabric;
03975f2939ad69 Mike Christie 2020-06-07  185  
03975f2939ad69 Mike Christie 2020-06-07  186  	return 0;
03975f2939ad69 Mike Christie 2020-06-07  187  
03975f2939ad69 Mike Christie 2020-06-07  188  free_fabric:
03975f2939ad69 Mike Christie 2020-06-07  189  	kfree(se_sess->fabric_name);
03975f2939ad69 Mike Christie 2020-06-07  190  	se_sess->fabric_name = NULL;
03975f2939ad69 Mike Christie 2020-06-07  191  free_target:
03975f2939ad69 Mike Christie 2020-06-07  192  	kfree(se_sess->target_name);
03975f2939ad69 Mike Christie 2020-06-07  193  	se_sess->target_name = NULL;
03975f2939ad69 Mike Christie 2020-06-07  194  free_acl:
03975f2939ad69 Mike Christie 2020-06-07  195  	kfree(se_sess->acl_name);
03975f2939ad69 Mike Christie 2020-06-07  196  	se_sess->acl_name = NULL;
03975f2939ad69 Mike Christie 2020-06-07  197  	return -ENOMEM;
03975f2939ad69 Mike Christie 2020-06-07  198  }
03975f2939ad69 Mike Christie 2020-06-07  199  

:::::: The code at line 172 was first introduced by commit
:::::: 03975f2939ad69a19b1a7cdd49cb83e66258688e target: add session sysfs class support

:::::: TO: Mike Christie <michael.christie@oracle.com>
:::::: CC: 0day robot <lkp@intel.com>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 62562 bytes --]

[PATCH 13/17] iscsi target: replace module sids with lio's sid

[Mike Christie]

This is the first phase in hooking iscsi into the sysfs API. This patch
has it use lio core's sid instead of its internal ones.

We have 2 sids in the iscsi target layer:
- module sid: int id that is unique across all iscsi targets. Used for
  sess_get_index().
- iscsi target port group sid: int id that is unique in the tpg. Uses
  for logging.

The lio one works exactly like the iscsi target module one, and the iscsi
tpg one is not very useful because when you have multiple tpgs you can't
tell which tpg the session is under. In the latter case the lio core one
is more useful, because it matches what we see in userspace and logs and
we can distinguish what fabric/target/tpg the session is under.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/target/iscsi/iscsi_target.c          |  6 ++----
 drivers/target/iscsi/iscsi_target_configfs.c |  6 ++----
 drivers/target/iscsi/iscsi_target_erl0.c     | 11 ++++++-----
 drivers/target/iscsi/iscsi_target_erl2.c     |  8 ++++----
 drivers/target/iscsi/iscsi_target_login.c    | 20 ++------------------
 drivers/target/iscsi/iscsi_target_stat.c     |  3 +--
 drivers/target/iscsi/iscsi_target_tmr.c      |  2 +-
 drivers/target/iscsi/iscsi_target_tpg.c      | 16 +++++++---------
 drivers/target/iscsi/iscsi_target_util.c     |  2 +-
 include/target/iscsi/iscsi_target_core.h     |  6 ------
 10 files changed, 26 insertions(+), 54 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 59379d6..d9853d5 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -49,7 +49,6 @@
 static DEFINE_MUTEX(np_lock);
 
 static struct idr tiqn_idr;
-DEFINE_IDA(sess_ida);
 struct mutex auth_id_lock;
 
 struct iscsit_global *iscsit_global;
@@ -2328,7 +2327,7 @@ int iscsit_logout_closesession(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
 	struct iscsi_session *sess = conn->sess;
 
 	pr_debug("Received logout request CLOSESESSION on CID: %hu"
-		" for SID: %u.\n", conn->cid, conn->sess->sid);
+		" for SID: %u.\n", conn->cid, conn->sess->se_sess->sid);
 
 	atomic_set(&sess->session_logout, 1);
 	atomic_set(&conn->conn_logout_remove, 1);
@@ -4107,7 +4106,7 @@ int iscsit_close_connection(
 	struct iscsi_session	*sess = conn->sess;
 
 	pr_debug("Closing iSCSI connection CID %hu on SID:"
-		" %u\n", conn->cid, sess->sid);
+		" %u\n", conn->cid, sess->se_sess->sid);
 	/*
 	 * Always up conn_logout_comp for the traditional TCP and HW_OFFLOAD
 	 * case just in case the RX Thread in iscsi_target_rx_opcode() is
@@ -4403,7 +4402,6 @@ int iscsit_close_session(struct iscsi_session *sess)
 	pr_debug("Decremented number of active iSCSI Sessions on"
 		" iSCSI TPG: %hu to %u\n", tpg->tpgt, tpg->nsessions);
 
-	ida_free(&sess_ida, sess->session_index);
 	kfree(sess->sess_ops);
 	sess->sess_ops = NULL;
 	spin_unlock_bh(&se_tpg->session_lock);
diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c
index 4513740..66a3b8d 100644
--- a/drivers/target/iscsi/iscsi_target_configfs.c
+++ b/drivers/target/iscsi/iscsi_target_configfs.c
@@ -520,7 +520,7 @@ static ssize_t lio_target_nacl_info_show(struct config_item *item, char *page)
 
 		rb += sprintf(page+rb,
 			      "LIO Session ID: %u   ISID: 0x%6ph  TSIH: %hu  ",
-			      sess->sid, sess->isid, sess->tsih);
+			      se_sess->sid, sess->isid, sess->tsih);
 		rb += sprintf(page+rb, "SessionType: %s\n",
 				(sess->sess_ops->SessionType) ?
 				"Discovery" : "Normal");
@@ -1344,9 +1344,7 @@ static int iscsi_get_cmd_state(struct se_cmd *se_cmd)
 
 static u32 lio_sess_get_index(struct se_session *se_sess)
 {
-	struct iscsi_session *sess = se_sess->fabric_sess_ptr;
-
-	return sess->session_index;
+	return se_sess->sid;
 }
 
 static int lio_queue_data_in(struct se_cmd *se_cmd)
diff --git a/drivers/target/iscsi/iscsi_target_erl0.c b/drivers/target/iscsi/iscsi_target_erl0.c
index b4abd7b..e6acd54 100644
--- a/drivers/target/iscsi/iscsi_target_erl0.c
+++ b/drivers/target/iscsi/iscsi_target_erl0.c
@@ -761,7 +761,7 @@ void iscsit_handle_time2retain_timeout(struct timer_list *t)
 	sess->time2retain_timer_flags |= ISCSI_TF_EXPIRED;
 
 	pr_err("Time2Retain timer expired for SID: %u, cleaning up"
-			" iSCSI session.\n", sess->sid);
+			" iSCSI session.\n", sess->se_sess->sid);
 
 	iscsit_fill_cxn_timeout_err_stats(sess);
 	spin_unlock_bh(&se_tpg->session_lock);
@@ -786,7 +786,8 @@ void iscsit_start_time2retain_handler(struct iscsi_session *sess)
 		return;
 
 	pr_debug("Starting Time2Retain timer for %u seconds on"
-		" SID: %u\n", sess->sess_ops->DefaultTime2Retain, sess->sid);
+		" SID: %u\n", sess->sess_ops->DefaultTime2Retain,
+		sess->se_sess->sid);
 
 	sess->time2retain_timer_flags &= ~ISCSI_TF_STOP;
 	sess->time2retain_timer_flags |= ISCSI_TF_RUNNING;
@@ -815,7 +816,7 @@ int iscsit_stop_time2retain_timer(struct iscsi_session *sess)
 	spin_lock(&se_tpg->session_lock);
 	sess->time2retain_timer_flags &= ~ISCSI_TF_RUNNING;
 	pr_debug("Stopped Time2Retain Timer for SID: %u\n",
-			sess->sid);
+		 sess->se_sess->sid);
 	return 0;
 }
 
@@ -882,8 +883,8 @@ void iscsit_cause_connection_reinstatement(struct iscsi_conn *conn, int sleep)
 
 void iscsit_fall_back_to_erl0(struct iscsi_session *sess)
 {
-	pr_debug("Falling back to ErrorRecoveryLevel=0 for SID:"
-			" %u\n", sess->sid);
+	pr_debug("Falling back to ErrorRecoveryLevel=0 for SID: %u\n",
+		 sess->se_sess->sid);
 
 	atomic_set(&sess->session_fall_back_to_erl0, 1);
 }
diff --git a/drivers/target/iscsi/iscsi_target_erl2.c b/drivers/target/iscsi/iscsi_target_erl2.c
index b1b7db9d..bdc9558 100644
--- a/drivers/target/iscsi/iscsi_target_erl2.c
+++ b/drivers/target/iscsi/iscsi_target_erl2.c
@@ -93,7 +93,7 @@ static int iscsit_attach_inactive_connection_recovery_entry(
 
 	sess->conn_recovery_count++;
 	pr_debug("Incremented connection recovery count to %u for"
-		" SID: %u\n", sess->conn_recovery_count, sess->sid);
+		" SID: %u\n", sess->conn_recovery_count, sess->se_sess->sid);
 	spin_unlock(&sess->cr_i_lock);
 
 	return 0;
@@ -176,7 +176,7 @@ int iscsit_remove_active_connection_recovery_entry(
 
 	sess->conn_recovery_count--;
 	pr_debug("Decremented connection recovery count to %u for"
-		" SID: %u\n", sess->conn_recovery_count, sess->sid);
+		" SID: %u\n", sess->conn_recovery_count, sess->se_sess->sid);
 	spin_unlock(&sess->cr_a_lock);
 
 	kfree(cr);
@@ -251,11 +251,11 @@ void iscsit_discard_cr_cmds_by_expstatsn(
 	if (!cr->cmd_count) {
 		pr_debug("No commands to be reassigned for failed"
 			" connection CID: %hu on SID: %u\n",
-			cr->cid, sess->sid);
+			cr->cid, sess->se_sess->sid);
 		iscsit_remove_inactive_connection_recovery_entry(cr, sess);
 		iscsit_attach_active_connection_recovery_entry(sess, cr);
 		pr_debug("iSCSI connection recovery successful for CID:"
-			" %hu on SID: %u\n", cr->cid, sess->sid);
+			" %hu on SID: %u\n", cr->cid, sess->se_sess->sid);
 		iscsit_remove_active_connection_recovery_entry(cr, sess);
 	} else {
 		iscsit_remove_inactive_connection_recovery_entry(cr, sess);
diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c
index 85748e3..417b797 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -186,7 +186,7 @@ int iscsi_check_for_session_reinstatement(struct iscsi_conn *conn)
 
 	pr_debug("%s iSCSI Session SID %u is still active for %s,"
 		" performing session reinstatement.\n", (sessiontype) ?
-		"Discovery" : "Normal", sess->sid,
+		"Discovery" : "Normal", sess->se_sess->sid,
 		sess->sess_ops->InitiatorName);
 
 	spin_lock_bh(&sess->conn_lock);
@@ -258,7 +258,6 @@ static int iscsi_login_zero_tsih_s1(
 {
 	struct iscsi_session *sess = NULL;
 	struct iscsi_login_req *pdu = (struct iscsi_login_req *)buf;
-	int ret;
 
 	sess = kzalloc(sizeof(struct iscsi_session), GFP_KERNEL);
 	if (!sess) {
@@ -292,15 +291,6 @@ static int iscsi_login_zero_tsih_s1(
 	timer_setup(&sess->time2retain_timer,
 		    iscsit_handle_time2retain_timeout, 0);
 
-	ret = ida_alloc(&sess_ida, GFP_KERNEL);
-	if (ret < 0) {
-		pr_err("Session ID allocation failed %d\n", ret);
-		iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
-				ISCSI_LOGIN_STATUS_NO_RESOURCES);
-		goto free_sess;
-	}
-
-	sess->session_index = ret;
 	sess->creation_time = get_jiffies_64();
 	/*
 	 * The FFP CmdSN window values will be allocated from the TPG's
@@ -314,7 +304,7 @@ static int iscsi_login_zero_tsih_s1(
 				ISCSI_LOGIN_STATUS_NO_RESOURCES);
 		pr_err("Unable to allocate memory for"
 				" struct iscsi_sess_ops.\n");
-		goto free_id;
+		goto free_sess;
 	}
 
 	sess->se_sess = transport_alloc_session(TARGET_PROT_NORMAL);
@@ -328,8 +318,6 @@ static int iscsi_login_zero_tsih_s1(
 
 free_ops:
 	kfree(sess->sess_ops);
-free_id:
-	ida_free(&sess_ida, sess->session_index);
 free_sess:
 	kfree(sess);
 	conn->sess = NULL;
@@ -768,9 +756,6 @@ void iscsi_post_login_handler(
 		sess->sess_ops->InitiatorName);
 	spin_unlock_bh(&sess->conn_lock);
 
-	sess->sid = tpg->sid++;
-	if (!sess->sid)
-		sess->sid = tpg->sid++;
 	pr_debug("Established iSCSI session from node: %s\n",
 			sess->sess_ops->InitiatorName);
 
@@ -1161,7 +1146,6 @@ void iscsi_target_login_sess_out(struct iscsi_conn *conn,
 		goto old_sess_out;
 
 	transport_free_session(conn->sess->se_sess);
-	ida_free(&sess_ida, conn->sess->session_index);
 	kfree(conn->sess->sess_ops);
 	kfree(conn->sess);
 	conn->sess = NULL;
diff --git a/drivers/target/iscsi/iscsi_target_stat.c b/drivers/target/iscsi/iscsi_target_stat.c
index 35e75a3..8167fdc 100644
--- a/drivers/target/iscsi/iscsi_target_stat.c
+++ b/drivers/target/iscsi/iscsi_target_stat.c
@@ -630,8 +630,7 @@ static ssize_t iscsi_stat_sess_indx_show(struct config_item *item, char *page)
 	if (se_sess) {
 		sess = se_sess->fabric_sess_ptr;
 		if (sess)
-			ret = snprintf(page, PAGE_SIZE, "%u\n",
-					sess->session_index);
+			ret = snprintf(page, PAGE_SIZE, "%u\n", se_sess->sid);
 	}
 	spin_unlock_bh(&se_nacl->nacl_sess_lock);
 
diff --git a/drivers/target/iscsi/iscsi_target_tmr.c b/drivers/target/iscsi/iscsi_target_tmr.c
index 7d618db..dbc95eb 100644
--- a/drivers/target/iscsi/iscsi_target_tmr.c
+++ b/drivers/target/iscsi/iscsi_target_tmr.c
@@ -186,7 +186,7 @@ static void iscsit_task_reassign_remove_cmd(
 	spin_unlock(&cr->conn_recovery_cmd_lock);
 	if (!ret) {
 		pr_debug("iSCSI connection recovery successful for CID:"
-			" %hu on SID: %u\n", cr->cid, sess->sid);
+			" %hu on SID: %u\n", cr->cid, sess->se_sess->sid);
 		iscsit_remove_active_connection_recovery_entry(cr, sess);
 	}
 }
diff --git a/drivers/target/iscsi/iscsi_target_tpg.c b/drivers/target/iscsi/iscsi_target_tpg.c
index 8075f60..e252a7f 100644
--- a/drivers/target/iscsi/iscsi_target_tpg.c
+++ b/drivers/target/iscsi/iscsi_target_tpg.c
@@ -64,16 +64,13 @@ int iscsit_load_discovery_tpg(void)
 	 */
 	tpg->tpg_se_tpg.se_tpg_tfo = &iscsi_ops;
 	ret = core_tpg_register(NULL, &tpg->tpg_se_tpg, -1);
-	if (ret < 0) {
-		kfree(tpg);
-		return -1;
-	}
+	if (ret < 0)
+		goto free_tpg;
 
-	tpg->sid = 1; /* First Assigned LIO Session ID */
 	iscsit_set_default_tpg_attribs(tpg);
 
 	if (iscsi_create_default_params(&tpg->param_list) < 0)
-		goto out;
+		goto dereg_se_tpg;
 	/*
 	 * By default we disable authentication for discovery sessions,
 	 * this can be changed with:
@@ -97,11 +94,12 @@ int iscsit_load_discovery_tpg(void)
 	pr_debug("CORE[0] - Allocated Discovery TPG\n");
 
 	return 0;
+
 free_pl_out:
 	iscsi_release_param_list(tpg->param_list);
-out:
-	if (tpg->sid == 1)
-		core_tpg_deregister(&tpg->tpg_se_tpg);
+dereg_se_tpg:
+	core_tpg_deregister(&tpg->tpg_se_tpg);
+free_tpg:
 	kfree(tpg);
 	return -1;
 }
diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
index 89183b3..f9c0ed0 100644
--- a/drivers/target/iscsi/iscsi_target_util.c
+++ b/drivers/target/iscsi/iscsi_target_util.c
@@ -1219,7 +1219,7 @@ void iscsit_print_session_params(struct iscsi_session *sess)
 	struct iscsi_conn *conn;
 
 	pr_debug("-----------------------------[Session Params for"
-		" SID: %u]-----------------------------\n", sess->sid);
+		" SID: %u]-----------------------------\n", sess->se_sess->sid);
 	spin_lock_bh(&sess->conn_lock);
 	list_for_each_entry(conn, &sess->sess_conn_list, conn_list)
 		iscsi_dump_conn_ops(conn->conn_ops);
diff --git a/include/target/iscsi/iscsi_target_core.h b/include/target/iscsi/iscsi_target_core.h
index 591cd9e..64937be 100644
--- a/include/target/iscsi/iscsi_target_core.h
+++ b/include/target/iscsi/iscsi_target_core.h
@@ -654,11 +654,7 @@ struct iscsi_session {
 	atomic_t		max_cmd_sn;
 	struct list_head	sess_ooo_cmdsn_list;
 
-	/* LIO specific session ID */
-	u32			sid;
 	char			auth_type[8];
-	/* unique within the target */
-	int			session_index;
 	/* Used for session reference counting */
 	int			session_usage_count;
 	int			session_waiting_on_uc;
@@ -830,8 +826,6 @@ struct iscsi_portal_group {
 	u32			nsessions;
 	/* Number of Network Portals available for this TPG */
 	u32			num_tpg_nps;
-	/* Per TPG LIO specific session ID. */
-	u32			sid;
 	/* Spinlock for adding/removing Network Portals */
 	spinlock_t		tpg_np_lock;
 	spinlock_t		tpg_state_lock;
-- 
1.8.3.1

Re: [PATCH 13/17] iscsi target: replace module sids with lio's sid

[Hannes Reinecke]

On 6/7/20 10:36 PM, Mike Christie wrote:
> This is the first phase in hooking iscsi into the sysfs API. This patch
> has it use lio core's sid instead of its internal ones.
> 
> We have 2 sids in the iscsi target layer:
> - module sid: int id that is unique across all iscsi targets. Used for
>    sess_get_index().
> - iscsi target port group sid: int id that is unique in the tpg. Uses
>    for logging.
> 
> The lio one works exactly like the iscsi target module one, and the iscsi
> tpg one is not very useful because when you have multiple tpgs you can't
> tell which tpg the session is under. In the latter case the lio core one
> is more useful, because it matches what we see in userspace and logs and
> we can distinguish what fabric/target/tpg the session is under.
> 
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> ---
>   drivers/target/iscsi/iscsi_target.c          |  6 ++----
>   drivers/target/iscsi/iscsi_target_configfs.c |  6 ++----
>   drivers/target/iscsi/iscsi_target_erl0.c     | 11 ++++++-----
>   drivers/target/iscsi/iscsi_target_erl2.c     |  8 ++++----
>   drivers/target/iscsi/iscsi_target_login.c    | 20 ++------------------
>   drivers/target/iscsi/iscsi_target_stat.c     |  3 +--
>   drivers/target/iscsi/iscsi_target_tmr.c      |  2 +-
>   drivers/target/iscsi/iscsi_target_tpg.c      | 16 +++++++---------
>   drivers/target/iscsi/iscsi_target_util.c     |  2 +-
>   include/target/iscsi/iscsi_target_core.h     |  6 ------
>   10 files changed, 26 insertions(+), 54 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

[PATCH 14/17] target: add free_session callout

[Mike Christie]

The next patches hook the iscsi target into the syfs API and has it
export some basic info about the session.

To allow fabric drivers to export info in sysfs, they need to be able
to free resources when all userspace refs have dropped. This patch
adds a new session callback free_session which the fabric drivers can
use to free their internal resources that are accessed via sysfs.

This also moves the setting of the fabric_sess_ptr to
transport_alloc_session so drivers like iscsi that use that function
can know that if that function returns success then they can always
free the resources pointed at fabric_sess_ptr in the free_session
callback.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/target/iscsi/iscsi_target.c          |  7 +----
 drivers/target/iscsi/iscsi_target_configfs.c |  9 ++++++
 drivers/target/iscsi/iscsi_target_login.c    |  8 ++---
 drivers/target/target_core_transport.c       | 47 +++++++++++++++++-----------
 include/target/target_core_base.h            |  1 +
 include/target/target_core_fabric.h          | 12 +++++--
 6 files changed, 51 insertions(+), 33 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index d9853d5..4c73374 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -4383,8 +4383,6 @@ int iscsit_close_session(struct iscsi_session *sess)
 		}
 	}
 
-	transport_deregister_session(sess->se_sess);
-
 	if (sess->sess_ops->ErrorRecoveryLevel == 2)
 		iscsit_free_connection_recovery_entries(sess);
 
@@ -4401,12 +4399,9 @@ int iscsit_close_session(struct iscsi_session *sess)
 
 	pr_debug("Decremented number of active iSCSI Sessions on"
 		" iSCSI TPG: %hu to %u\n", tpg->tpgt, tpg->nsessions);
-
-	kfree(sess->sess_ops);
-	sess->sess_ops = NULL;
 	spin_unlock_bh(&se_tpg->session_lock);
 
-	kfree(sess);
+	transport_deregister_session(sess->se_sess);
 	return 0;
 }
 
diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c
index 66a3b8d..ba608fa 100644
--- a/drivers/target/iscsi/iscsi_target_configfs.c
+++ b/drivers/target/iscsi/iscsi_target_configfs.c
@@ -1481,6 +1481,14 @@ static void lio_tpg_close_session(struct se_session *se_sess)
 	iscsit_dec_session_usage_count(sess);
 }
 
+static void lio_free_session(struct se_session *se_sess)
+{
+	struct iscsi_session *sess = se_sess->fabric_sess_ptr;
+
+	kfree(sess->sess_ops);
+	kfree(sess);
+}
+
 static u32 lio_tpg_get_inst_index(struct se_portal_group *se_tpg)
 {
 	return iscsi_tpg(se_tpg)->tpg_tiqn->tiqn_index;
@@ -1530,6 +1538,7 @@ static void lio_release_cmd(struct se_cmd *se_cmd)
 	.check_stop_free		= lio_check_stop_free,
 	.release_cmd			= lio_release_cmd,
 	.close_session			= lio_tpg_close_session,
+	.free_session			= lio_free_session,
 	.sess_get_index			= lio_sess_get_index,
 	.write_pending			= lio_write_pending,
 	.set_default_node_attributes	= lio_set_default_node_attributes,
diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c
index 417b797..3001dd7 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -307,7 +307,8 @@ static int iscsi_login_zero_tsih_s1(
 		goto free_sess;
 	}
 
-	sess->se_sess = transport_alloc_session(TARGET_PROT_NORMAL);
+	sess->se_sess = transport_alloc_session(&iscsi_ops, TARGET_PROT_NORMAL,
+						sess);
 	if (IS_ERR(sess->se_sess)) {
 		iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
 				ISCSI_LOGIN_STATUS_NO_RESOURCES);
@@ -740,7 +741,7 @@ void iscsi_post_login_handler(
 
 	spin_lock_bh(&se_tpg->session_lock);
 	__transport_register_session(&sess->tpg->tpg_se_tpg,
-			se_sess->se_node_acl, se_sess, sess);
+			se_sess->se_node_acl, se_sess);
 	pr_debug("Moving to TARG_SESS_STATE_LOGGED_IN.\n");
 	sess->session_state = TARG_SESS_STATE_LOGGED_IN;
 
@@ -1146,9 +1147,6 @@ void iscsi_target_login_sess_out(struct iscsi_conn *conn,
 		goto old_sess_out;
 
 	transport_free_session(conn->sess->se_sess);
-	kfree(conn->sess->sess_ops);
-	kfree(conn->sess);
-	conn->sess = NULL;
 
 old_sess_out:
 	iscsi_stop_login_thread_timer(np);
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 34da12a..ed63e36 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -239,9 +239,13 @@ int transport_init_session(struct se_session *se_sess)
 
 /**
  * transport_alloc_session - allocate a session object and initialize it
+ * @tfo: target core fabric ops
  * @sup_prot_ops: bitmask that defines which T10-PI modes are supported.
+ * @private: storage for fabric driver accessible via fabric_sess_ptr
  */
-struct se_session *transport_alloc_session(enum target_prot_op sup_prot_ops)
+struct se_session *transport_alloc_session(const struct target_core_fabric_ops *tfo,
+					   enum target_prot_op sup_prot_ops,
+					   void *private)
 {
 	struct se_session *se_sess;
 	int ret;
@@ -252,6 +256,8 @@ struct se_session *transport_alloc_session(enum target_prot_op sup_prot_ops)
 				" se_sess_cache\n");
 		return ERR_PTR(-ENOMEM);
 	}
+	se_sess->fabric_sess_ptr = private;
+	se_sess->tfo = tfo;
 
 	ret = target_sysfs_init_session(se_sess);
 	if (ret) {
@@ -311,14 +317,17 @@ int transport_alloc_session_tags(struct se_session *se_sess,
 
 /**
  * transport_init_session_tags - allocate a session and target driver private data
+ * @tfo: target core fabric ops
  * @tag_num:  Maximum number of in-flight commands between initiator and target.
  * @tag_size: Size in bytes of the private data a target driver associates with
  *	      each command.
  * @sup_prot_ops: bitmask that defines which T10-PI modes are supported.
+ * @private: storage for fabric driver accessible via fabric_sess_ptr
  */
 static struct se_session *
-transport_init_session_tags(unsigned int tag_num, unsigned int tag_size,
-			    enum target_prot_op sup_prot_ops)
+transport_init_session_tags(const struct target_core_fabric_ops *tfo,
+			    unsigned int tag_num, unsigned int tag_size,
+			    enum target_prot_op sup_prot_ops, void *private)
 {
 	struct se_session *se_sess;
 	int rc;
@@ -334,7 +343,7 @@ int transport_alloc_session_tags(struct se_session *se_sess,
 		return ERR_PTR(-EINVAL);
 	}
 
-	se_sess = transport_alloc_session(sup_prot_ops);
+	se_sess = transport_alloc_session(tfo, sup_prot_ops, private);
 	if (IS_ERR(se_sess))
 		return se_sess;
 
@@ -353,15 +362,13 @@ int transport_alloc_session_tags(struct se_session *se_sess,
 void __transport_register_session(
 	struct se_portal_group *se_tpg,
 	struct se_node_acl *se_nacl,
-	struct se_session *se_sess,
-	void *fabric_sess_ptr)
+	struct se_session *se_sess)
 {
-	const struct target_core_fabric_ops *tfo = se_tpg->se_tpg_tfo;
+	const struct target_core_fabric_ops *tfo = se_sess->tfo;
 	unsigned char *sid;
 	unsigned long flags;
 
 	se_sess->se_tpg = se_tpg;
-	se_sess->fabric_sess_ptr = fabric_sess_ptr;
 	/*
 	 * Used by struct se_node_acl's under ConfigFS to locate active se_session-t
 	 *
@@ -406,20 +413,19 @@ void __transport_register_session(
 	list_add_tail(&se_sess->sess_list, &se_tpg->tpg_sess_list);
 
 	pr_debug("TARGET_CORE[%s]: Registered fabric_sess_ptr: %p\n",
-		se_tpg->se_tpg_tfo->fabric_name, se_sess->fabric_sess_ptr);
+		se_sess->tfo->fabric_name, se_sess->fabric_sess_ptr);
 }
 EXPORT_SYMBOL(__transport_register_session);
 
 void transport_register_session(
 	struct se_portal_group *se_tpg,
 	struct se_node_acl *se_nacl,
-	struct se_session *se_sess,
-	void *fabric_sess_ptr)
+	struct se_session *se_sess)
 {
 	unsigned long flags;
 
 	spin_lock_irqsave(&se_tpg->session_lock, flags);
-	__transport_register_session(se_tpg, se_nacl, se_sess, fabric_sess_ptr);
+	__transport_register_session(se_tpg, se_nacl, se_sess);
 	spin_unlock_irqrestore(&se_tpg->session_lock, flags);
 }
 EXPORT_SYMBOL(transport_register_session);
@@ -451,9 +457,11 @@ struct se_session *
 	 * of I/O descriptor tags, go ahead and perform that setup now..
 	 */
 	if (tag_num != 0)
-		sess = transport_init_session_tags(tag_num, tag_size, prot_op);
+		sess = transport_init_session_tags(tpg->se_tpg_tfo, tag_num,
+						   tag_size, prot_op, private);
 	else
-		sess = transport_alloc_session(prot_op);
+		sess = transport_alloc_session(tpg->se_tpg_tfo, prot_op,
+					       private);
 
 	if (IS_ERR(sess))
 		return sess;
@@ -485,7 +493,7 @@ struct se_session *
 			goto rm_sysfs;
 	}
 
-	transport_register_session(tpg, sess->se_node_acl, sess, private);
+	transport_register_session(tpg, sess->se_node_acl, sess);
 	return sess;
 
 rm_sysfs:
@@ -585,7 +593,6 @@ void transport_free_session(struct se_session *se_sess)
 	 */
 	if (se_nacl) {
 		struct se_portal_group *se_tpg = se_nacl->se_tpg;
-		const struct target_core_fabric_ops *se_tfo = se_tpg->se_tpg_tfo;
 		unsigned long flags;
 
 		se_sess->se_node_acl = NULL;
@@ -597,7 +604,7 @@ void transport_free_session(struct se_session *se_sess)
 		 */
 		mutex_lock(&se_tpg->acl_node_mutex);
 		if (se_nacl->dynamic_node_acl &&
-		    !se_tfo->tpg_check_demo_mode_cache(se_tpg)) {
+		    !se_sess->tfo->tpg_check_demo_mode_cache(se_tpg)) {
 			spin_lock_irqsave(&se_nacl->nacl_sess_lock, flags);
 			if (list_empty(&se_nacl->acl_sess_list))
 				se_nacl->dynamic_stop = true;
@@ -624,6 +631,9 @@ void transport_free_session(struct se_session *se_sess)
 
 void __target_free_session(struct se_session *se_sess)
 {
+	if (se_sess->tfo->free_session)
+		se_sess->tfo->free_session(se_sess);
+
 	target_free_transport_id(se_sess->tpt_id);
 	kmem_cache_free(se_sess_cache, se_sess);
 }
@@ -650,7 +660,6 @@ void transport_deregister_session(struct se_session *se_sess)
 	spin_lock_irqsave(&se_tpg->session_lock, flags);
 	list_del(&se_sess->sess_list);
 	se_sess->se_tpg = NULL;
-	se_sess->fabric_sess_ptr = NULL;
 	spin_unlock_irqrestore(&se_tpg->session_lock, flags);
 
 	/*
@@ -660,7 +669,7 @@ void transport_deregister_session(struct se_session *se_sess)
 	target_for_each_device(target_release_res, se_sess);
 
 	pr_debug("TARGET_CORE[%s]: Deregistered fabric_sess\n",
-		se_tpg->se_tpg_tfo->fabric_name);
+		 se_sess->tfo->fabric_name);
 	/*
 	 * If last kref is dropping now for an explicit NodeACL, awake sleeping
 	 * ->acl_free_comp caller to wakeup configfs se_node_acl->acl_group
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 0d9916b..85c0eea2 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -643,6 +643,7 @@ struct se_session {
 	char			*tpg_name;
 	char			*target_name;
 	char			*fabric_name;
+	const struct target_core_fabric_ops *tfo;
 };
 
 struct se_device;
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index 2a93daa..af7d585 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -65,6 +65,11 @@ struct target_core_fabric_ops {
 	 */
 	int (*check_stop_free)(struct se_cmd *);
 	void (*release_cmd)(struct se_cmd *);
+	/*
+	 * Callout to free session resources that can be accessed from
+	 * session_attrs below.
+	 */
+	void (*free_session)(struct se_session *);
 	void (*close_session)(struct se_session *);
 	u32 (*sess_get_index)(struct se_session *);
 	int (*write_pending)(struct se_cmd *);
@@ -134,13 +139,14 @@ struct se_session *target_setup_session(struct se_portal_group *,
 void target_remove_session(struct se_session *);
 
 int transport_init_session(struct se_session *se_sess);
-struct se_session *transport_alloc_session(enum target_prot_op);
+struct se_session *transport_alloc_session(const struct target_core_fabric_ops *,
+					   enum target_prot_op, void *);
 int transport_alloc_session_tags(struct se_session *, unsigned int,
 		unsigned int);
 void	__transport_register_session(struct se_portal_group *,
-		struct se_node_acl *, struct se_session *, void *);
+		struct se_node_acl *, struct se_session *);
 void	transport_register_session(struct se_portal_group *,
-		struct se_node_acl *, struct se_session *, void *);
+		struct se_node_acl *, struct se_session *);
 ssize_t	target_show_dynamic_sessions(struct se_portal_group *, char *);
 void	transport_free_session(struct se_session *);
 void	target_spc2_release(struct se_node_acl *nacl);
-- 
1.8.3.1

Re: [PATCH 14/17] target: add free_session callout

[Hannes Reinecke]

On 6/7/20 10:36 PM, Mike Christie wrote:
> The next patches hook the iscsi target into the syfs API and has it
> export some basic info about the session.
> 
> To allow fabric drivers to export info in sysfs, they need to be able
> to free resources when all userspace refs have dropped. This patch
> adds a new session callback free_session which the fabric drivers can
> use to free their internal resources that are accessed via sysfs.
> 
> This also moves the setting of the fabric_sess_ptr to
> transport_alloc_session so drivers like iscsi that use that function
> can know that if that function returns success then they can always
> free the resources pointed at fabric_sess_ptr in the free_session
> callback.
> 
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> ---
>   drivers/target/iscsi/iscsi_target.c          |  7 +----
>   drivers/target/iscsi/iscsi_target_configfs.c |  9 ++++++
>   drivers/target/iscsi/iscsi_target_login.c    |  8 ++---
>   drivers/target/target_core_transport.c       | 47 +++++++++++++++++-----------
>   include/target/target_core_base.h            |  1 +
>   include/target/target_core_fabric.h          | 12 +++++--
>   6 files changed, 51 insertions(+), 33 deletions(-)
> One wonders why this isn't refcounted, but otherwise:

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

[PATCH 15/17] iscsi target: hook iscsi target into sysfs API

[Mike Christie]

Have the iscsi target export it's sessions in sysfs.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/target/iscsi/iscsi_target.c      | 2 ++
 drivers/target/iscsi/iscsi_target_nego.c | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 4c73374..f26d364 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -4383,6 +4383,8 @@ int iscsit_close_session(struct iscsi_session *sess)
 		}
 	}
 
+	target_sysfs_remove_session(sess->se_sess);
+
 	if (sess->sess_ops->ErrorRecoveryLevel == 2)
 		iscsit_free_connection_recovery_entries(sess);
 
diff --git a/drivers/target/iscsi/iscsi_target_nego.c b/drivers/target/iscsi/iscsi_target_nego.c
index c44613a2..6dcb8ff 100644
--- a/drivers/target/iscsi/iscsi_target_nego.c
+++ b/drivers/target/iscsi/iscsi_target_nego.c
@@ -359,6 +359,13 @@ static int iscsi_target_do_tx_login_io(struct iscsi_conn *conn, struct iscsi_log
 					    ISCSI_LOGIN_STATUS_NO_RESOURCES);
 			return -1;
 		}
+
+		if (target_sysfs_add_session(&conn->tpg->tpg_se_tpg,
+					     conn->sess->se_sess)) {
+			iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
+					    ISCSI_LOGIN_STATUS_NO_RESOURCES);
+			return -1;
+		}
 	}
 
 	if (conn->conn_transport->iscsit_put_login_tx(conn, login,
@@ -371,6 +378,8 @@ static int iscsi_target_do_tx_login_io(struct iscsi_conn *conn, struct iscsi_log
 
 err:
 	if (login->login_complete) {
+		target_sysfs_remove_session(conn->sess->se_sess);
+
 		if (conn->rx_thread && conn->rx_thread_active) {
 			send_sig(SIGINT, conn->rx_thread, 1);
 			complete(&conn->rx_login_comp);
-- 
1.8.3.1

Re: [PATCH 15/17] iscsi target: hook iscsi target into sysfs API

[Hannes Reinecke]

On 6/7/20 10:36 PM, Mike Christie wrote:
> Have the iscsi target export it's sessions in sysfs.
> 
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> ---
>   drivers/target/iscsi/iscsi_target.c      | 2 ++
>   drivers/target/iscsi/iscsi_target_nego.c | 9 +++++++++
>   2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> index 4c73374..f26d364 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c
> @@ -4383,6 +4383,8 @@ int iscsit_close_session(struct iscsi_session *sess)
>   		}
>   	}
>   
> +	target_sysfs_remove_session(sess->se_sess);
> +
>   	if (sess->sess_ops->ErrorRecoveryLevel == 2)
>   		iscsit_free_connection_recovery_entries(sess);
>   
> diff --git a/drivers/target/iscsi/iscsi_target_nego.c b/drivers/target/iscsi/iscsi_target_nego.c
> index c44613a2..6dcb8ff 100644
> --- a/drivers/target/iscsi/iscsi_target_nego.c
> +++ b/drivers/target/iscsi/iscsi_target_nego.c
> @@ -359,6 +359,13 @@ static int iscsi_target_do_tx_login_io(struct iscsi_conn *conn, struct iscsi_log
>   					    ISCSI_LOGIN_STATUS_NO_RESOURCES);
>   			return -1;
>   		}
> +
> +		if (target_sysfs_add_session(&conn->tpg->tpg_se_tpg,
> +					     conn->sess->se_sess)) {
> +			iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
> +					    ISCSI_LOGIN_STATUS_NO_RESOURCES);
> +			return -1;
> +		}
>   	}
>   
>   	if (conn->conn_transport->iscsit_put_login_tx(conn, login,
> @@ -371,6 +378,8 @@ static int iscsi_target_do_tx_login_io(struct iscsi_conn *conn, struct iscsi_log
>   
>   err:
>   	if (login->login_complete) {
> +		target_sysfs_remove_session(conn->sess->se_sess);
> +
>   		if (conn->rx_thread && conn->rx_thread_active) {
>   			send_sig(SIGINT, conn->rx_thread, 1);
>   			complete(&conn->rx_login_comp);
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

[PATCH 16/17] iscsi target: export session state and alias in sysfs

[Mike Christie]

Export the session state and alias in sysfs.

Note:
It does not export the per connection state. In the future we can
have the iscsi target add/rm an attribute group directly for each
connection when we login and logout/drop a connection. Since those
are dynamic and can change while the session exists, we don't need
them at device addition so it can be done directly from the module.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/target/iscsi/iscsi_target_configfs.c | 97 ++++++++++++++++++++++------
 1 file changed, 76 insertions(+), 21 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c
index ba608fa..64de2ec 100644
--- a/drivers/target/iscsi/iscsi_target_configfs.c
+++ b/drivers/target/iscsi/iscsi_target_configfs.c
@@ -28,6 +28,28 @@
 #include "iscsi_target.h"
 #include <target/iscsi/iscsi_target_stat.h>
 
+static const struct {
+	enum target_sess_state_table	value;
+	char				*name;
+} session_states[] = {
+	{ TARG_SESS_STATE_FREE,		"TARG_SESS_FREE" },
+	{ TARG_SESS_STATE_ACTIVE,	"TARG_SESS_STATE_ACTIVE" },
+	{ TARG_SESS_STATE_LOGGED_IN,	"TARG_SESS_STATE_LOGGED_IN" },
+	{ TARG_SESS_STATE_FAILED,	"TARG_SESS_STATE_FAILED" },
+	{ TARG_SESS_STATE_IN_CONTINUE,	"TARG_SESS_STATE_IN_CONTINUE" },
+};
+
+static const char *session_state_name(enum target_sess_state_table state)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(session_states); i++) {
+		if (session_states[i].value == state)
+			return session_states[i].name;
+	}
+
+	return NULL;
+}
 
 /* Start items for lio_target_portal_cit */
 
@@ -502,6 +524,7 @@ static ssize_t lio_target_nacl_info_show(struct config_item *item, char *page)
 	struct iscsi_session *sess;
 	struct iscsi_conn *conn;
 	struct se_session *se_sess;
+	const char *state_name;
 	ssize_t rb = 0;
 	u32 max_cmd_sn;
 
@@ -525,27 +548,12 @@ static ssize_t lio_target_nacl_info_show(struct config_item *item, char *page)
 				(sess->sess_ops->SessionType) ?
 				"Discovery" : "Normal");
 		rb += sprintf(page+rb, "Session State: ");
-		switch (sess->session_state) {
-		case TARG_SESS_STATE_FREE:
-			rb += sprintf(page+rb, "TARG_SESS_FREE\n");
-			break;
-		case TARG_SESS_STATE_ACTIVE:
-			rb += sprintf(page+rb, "TARG_SESS_STATE_ACTIVE\n");
-			break;
-		case TARG_SESS_STATE_LOGGED_IN:
-			rb += sprintf(page+rb, "TARG_SESS_STATE_LOGGED_IN\n");
-			break;
-		case TARG_SESS_STATE_FAILED:
-			rb += sprintf(page+rb, "TARG_SESS_STATE_FAILED\n");
-			break;
-		case TARG_SESS_STATE_IN_CONTINUE:
-			rb += sprintf(page+rb, "TARG_SESS_STATE_IN_CONTINUE\n");
-			break;
-		default:
-			rb += sprintf(page+rb, "ERROR: Unknown Session"
-					" State!\n");
-			break;
-		}
+
+		state_name = session_state_name(sess->session_state);
+		if (state_name)
+			rb += sprintf(page+rb, "%s\n", state_name);
+		else
+			rb += sprintf(page+rb, "ERROR: Unknown Session State!\n");
 
 		rb += sprintf(page+rb, "---------------------[iSCSI Session"
 				" Values]-----------------------\n");
@@ -1333,6 +1341,51 @@ static ssize_t iscsi_disc_enforce_discovery_auth_store(struct config_item *item,
 
 /* End lio_target_discovery_auth_cit */
 
+/* session sysfs */
+static ssize_t
+lio_show_sess_state(struct device *dev, struct device_attribute *attr,
+		    char *buf)
+{
+	struct se_session *se_sess = container_of(dev, struct se_session, dev);
+	struct iscsi_session *sess = se_sess->fabric_sess_ptr;
+	const char *state_name;
+
+	state_name = session_state_name(sess->session_state);
+	if (!state_name)
+		return -EINVAL;
+
+	return snprintf(buf, PAGE_SIZE, "%s", state_name);
+}
+
+static ssize_t
+lio_show_initiator_alias(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct se_session *se_sess = container_of(dev, struct se_session, dev);
+	struct iscsi_session *sess = se_sess->fabric_sess_ptr;
+
+	return snprintf(buf, PAGE_SIZE, "%s", sess->sess_ops->InitiatorAlias);
+}
+
+static DEVICE_ATTR(state, S_IRUGO, lio_show_sess_state, NULL);
+static DEVICE_ATTR(initiator_alias, S_IRUGO, lio_show_initiator_alias, NULL);
+
+static struct attribute *lio_sess_attrs[] = {
+	&dev_attr_state.attr,
+	&dev_attr_initiator_alias.attr,
+	NULL,
+};
+
+static struct attribute_group lio_sess_attr_group = {
+	.name = "iscsi_session",
+	.attrs = lio_sess_attrs,
+};
+
+static const struct attribute_group *lio_sess_attr_groups[] = {
+	&lio_sess_attr_group,
+	NULL,
+};
+
 /* Start functions for target_core_fabric_ops */
 
 static int iscsi_get_cmd_state(struct se_cmd *se_cmd)
@@ -1556,6 +1609,8 @@ static void lio_release_cmd(struct se_cmd *se_cmd)
 	.fabric_drop_np			= lio_target_call_delnpfromtpg,
 	.fabric_init_nodeacl		= lio_target_init_nodeacl,
 
+	.session_attr_groups		= lio_sess_attr_groups,
+
 	.tfc_discovery_attrs		= lio_target_discovery_auth_attrs,
 	.tfc_wwn_attrs			= lio_target_wwn_attrs,
 	.tfc_tpg_base_attrs		= lio_target_tpg_attrs,
-- 
1.8.3.1

Re: [PATCH 16/17] iscsi target: export session state and alias in sysfs

[Hannes Reinecke]

On 6/7/20 10:36 PM, Mike Christie wrote:
> Export the session state and alias in sysfs.
> 
> Note:
> It does not export the per connection state. In the future we can
> have the iscsi target add/rm an attribute group directly for each
> connection when we login and logout/drop a connection. Since those
> are dynamic and can change while the session exists, we don't need
> them at device addition so it can be done directly from the module.
> 
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> ---
>   drivers/target/iscsi/iscsi_target_configfs.c | 97 ++++++++++++++++++++++------
>   1 file changed, 76 insertions(+), 21 deletions(-)
> Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

[PATCH 17/17] target: drop sess_get_index

[Mike Christie]

Use the LIO session id for the scsiAttIntrPortIndex. iSCSI was
already using this value and the other drivers used hard coded
values of 1 or 0. The SCSI-MIB specs says:

This object represents an arbitrary integer used to uniquely
identify a particular attached remote initiator port to a
particular SCSI target port within a particular SCSI target device
within a particular SCSI instance.

So the lio session sid can be used.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c        | 15 ---------------
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c     |  6 ------
 drivers/scsi/qla2xxx/tcm_qla2xxx.c           |  7 -------
 drivers/target/iscsi/iscsi_target_configfs.c |  6 ------
 drivers/target/loopback/tcm_loop.c           |  6 ------
 drivers/target/sbp/sbp_target.c              |  6 ------
 drivers/target/target_core_configfs.c        |  4 ----
 drivers/target/target_core_stat.c            |  5 +----
 drivers/target/tcm_fc/tfc_conf.c             |  1 -
 drivers/target/tcm_fc/tfc_sess.c             |  7 -------
 drivers/usb/gadget/function/f_tcm.c          |  6 ------
 drivers/vhost/scsi.c                         |  6 ------
 drivers/xen/xen-scsiback.c                   |  6 ------
 include/target/target_core_fabric.h          |  1 -
 14 files changed, 1 insertion(+), 81 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 3cda73c..7fe76e0 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -3353,20 +3353,6 @@ static void srpt_close_session(struct se_session *se_sess)
 	srpt_disconnect_ch_sync(ch);
 }
 
-/**
- * srpt_sess_get_index - return the value of scsiAttIntrPortIndex (SCSI-MIB)
- * @se_sess: SCSI target session.
- *
- * A quote from RFC 4455 (SCSI-MIB) about this MIB object:
- * This object represents an arbitrary integer used to uniquely identify a
- * particular attached remote initiator port to a particular SCSI target port
- * within a particular SCSI target device within a particular SCSI instance.
- */
-static u32 srpt_sess_get_index(struct se_session *se_sess)
-{
-	return 0;
-}
-
 static void srpt_set_default_node_attrs(struct se_node_acl *nacl)
 {
 }
@@ -3843,7 +3829,6 @@ static ssize_t srpt_wwn_version_show(struct config_item *item, char *buf)
 	.release_cmd			= srpt_release_cmd,
 	.check_stop_free		= srpt_check_stop_free,
 	.close_session			= srpt_close_session,
-	.sess_get_index			= srpt_sess_get_index,
 	.write_pending			= srpt_write_pending,
 	.set_default_node_attributes	= srpt_set_default_node_attrs,
 	.get_cmd_state			= srpt_get_tcm_cmd_state,
diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 31459f3..8d37375 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -3739,11 +3739,6 @@ static void ibmvscsis_release_cmd(struct se_cmd *se_cmd)
 	spin_unlock_bh(&vscsi->intr_lock);
 }
 
-static u32 ibmvscsis_sess_get_index(struct se_session *se_sess)
-{
-	return 0;
-}
-
 static int ibmvscsis_write_pending(struct se_cmd *se_cmd)
 {
 	struct ibmvscsis_cmd *cmd = container_of(se_cmd, struct ibmvscsis_cmd,
@@ -4034,7 +4029,6 @@ static ssize_t ibmvscsis_tpg_enable_store(struct config_item *item,
 	.tpg_get_inst_index		= ibmvscsis_tpg_get_inst_index,
 	.check_stop_free		= ibmvscsis_check_stop_free,
 	.release_cmd			= ibmvscsis_release_cmd,
-	.sess_get_index			= ibmvscsis_sess_get_index,
 	.write_pending			= ibmvscsis_write_pending,
 	.set_default_node_attributes	= ibmvscsis_set_default_node_attrs,
 	.get_cmd_state			= ibmvscsis_get_cmd_state,
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index fff695a..8a68085 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -377,11 +377,6 @@ static void tcm_qla2xxx_close_session(struct se_session *se_sess)
 	tcm_qla2xxx_put_sess(sess);
 }
 
-static u32 tcm_qla2xxx_sess_get_index(struct se_session *se_sess)
-{
-	return 0;
-}
-
 static int tcm_qla2xxx_write_pending(struct se_cmd *se_cmd)
 {
 	struct qla_tgt_cmd *cmd = container_of(se_cmd,
@@ -1851,7 +1846,6 @@ static ssize_t tcm_qla2xxx_wwn_version_show(struct config_item *item,
 	.check_stop_free		= tcm_qla2xxx_check_stop_free,
 	.release_cmd			= tcm_qla2xxx_release_cmd,
 	.close_session			= tcm_qla2xxx_close_session,
-	.sess_get_index			= tcm_qla2xxx_sess_get_index,
 	.write_pending			= tcm_qla2xxx_write_pending,
 	.set_default_node_attributes	= tcm_qla2xxx_set_default_node_attrs,
 	.get_cmd_state			= tcm_qla2xxx_get_cmd_state,
@@ -1890,7 +1884,6 @@ static ssize_t tcm_qla2xxx_wwn_version_show(struct config_item *item,
 	.check_stop_free                = tcm_qla2xxx_check_stop_free,
 	.release_cmd			= tcm_qla2xxx_release_cmd,
 	.close_session			= tcm_qla2xxx_close_session,
-	.sess_get_index			= tcm_qla2xxx_sess_get_index,
 	.write_pending			= tcm_qla2xxx_write_pending,
 	.set_default_node_attributes	= tcm_qla2xxx_set_default_node_attrs,
 	.get_cmd_state			= tcm_qla2xxx_get_cmd_state,
diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c
index 64de2ec..5bb3375 100644
--- a/drivers/target/iscsi/iscsi_target_configfs.c
+++ b/drivers/target/iscsi/iscsi_target_configfs.c
@@ -1395,11 +1395,6 @@ static int iscsi_get_cmd_state(struct se_cmd *se_cmd)
 	return cmd->i_state;
 }
 
-static u32 lio_sess_get_index(struct se_session *se_sess)
-{
-	return se_sess->sid;
-}
-
 static int lio_queue_data_in(struct se_cmd *se_cmd)
 {
 	struct iscsi_cmd *cmd = container_of(se_cmd, struct iscsi_cmd, se_cmd);
@@ -1592,7 +1587,6 @@ static void lio_release_cmd(struct se_cmd *se_cmd)
 	.release_cmd			= lio_release_cmd,
 	.close_session			= lio_tpg_close_session,
 	.free_session			= lio_free_session,
-	.sess_get_index			= lio_sess_get_index,
 	.write_pending			= lio_write_pending,
 	.set_default_node_attributes	= lio_set_default_node_attributes,
 	.get_cmd_state			= iscsi_get_cmd_state,
diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c
index 64e5f1f..8daa424 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -512,11 +512,6 @@ static u32 tcm_loop_get_inst_index(struct se_portal_group *se_tpg)
 	return 1;
 }
 
-static u32 tcm_loop_sess_get_index(struct se_session *se_sess)
-{
-	return 1;
-}
-
 static void tcm_loop_set_default_node_attributes(struct se_node_acl *se_acl)
 {
 	return;
@@ -1093,7 +1088,6 @@ static ssize_t tcm_loop_wwn_version_show(struct config_item *item, char *page)
 	.tpg_get_inst_index		= tcm_loop_get_inst_index,
 	.check_stop_free		= tcm_loop_check_stop_free,
 	.release_cmd			= tcm_loop_release_cmd,
-	.sess_get_index			= tcm_loop_sess_get_index,
 	.write_pending			= tcm_loop_write_pending,
 	.set_default_node_attributes	= tcm_loop_set_default_node_attributes,
 	.get_cmd_state			= tcm_loop_get_cmd_state,
diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c
index 2cac4d9..782c326 100644
--- a/drivers/target/sbp/sbp_target.c
+++ b/drivers/target/sbp/sbp_target.c
@@ -1708,11 +1708,6 @@ static void sbp_release_cmd(struct se_cmd *se_cmd)
 	sbp_free_request(req);
 }
 
-static u32 sbp_sess_get_index(struct se_session *se_sess)
-{
-	return 0;
-}
-
 static int sbp_write_pending(struct se_cmd *se_cmd)
 {
 	struct sbp_target_request *req = container_of(se_cmd,
@@ -2309,7 +2304,6 @@ static ssize_t sbp_tpg_attrib_max_logins_per_lun_store(struct config_item *item,
 	.tpg_check_prod_mode_write_protect = sbp_check_false,
 	.tpg_get_inst_index		= sbp_tpg_get_inst_index,
 	.release_cmd			= sbp_release_cmd,
-	.sess_get_index			= sbp_sess_get_index,
 	.write_pending			= sbp_write_pending,
 	.set_default_node_attributes	= sbp_set_default_node_attrs,
 	.get_cmd_state			= sbp_get_cmd_state,
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index c23552b..a2a4641 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -385,10 +385,6 @@ static int target_fabric_tf_ops_check(const struct target_core_fabric_ops *tfo)
 		pr_err("Missing tfo->release_cmd()\n");
 		return -EINVAL;
 	}
-	if (!tfo->sess_get_index) {
-		pr_err("Missing tfo->sess_get_index()\n");
-		return -EINVAL;
-	}
 	if (!tfo->write_pending) {
 		pr_err("Missing tfo->write_pending()\n");
 		return -EINVAL;
diff --git a/drivers/target/target_core_stat.c b/drivers/target/target_core_stat.c
index 3eb1b9b..621dc17 100644
--- a/drivers/target/target_core_stat.c
+++ b/drivers/target/target_core_stat.c
@@ -1264,7 +1264,6 @@ static ssize_t target_stat_iport_indx_show(struct config_item *item,
 	struct se_lun_acl *lacl = iport_to_lacl(item);
 	struct se_node_acl *nacl = lacl->se_lun_nacl;
 	struct se_session *se_sess;
-	struct se_portal_group *tpg;
 	ssize_t ret;
 
 	spin_lock_irq(&nacl->nacl_sess_lock);
@@ -1274,10 +1273,8 @@ static ssize_t target_stat_iport_indx_show(struct config_item *item,
 		return -ENODEV;
 	}
 
-	tpg = nacl->se_tpg;
 	/* scsiAttIntrPortIndex */
-	ret = snprintf(page, PAGE_SIZE, "%u\n",
-			tpg->se_tpg_tfo->sess_get_index(se_sess));
+	ret = snprintf(page, PAGE_SIZE, "%u\n", se_sess->sid);
 	spin_unlock_irq(&nacl->nacl_sess_lock);
 	return ret;
 }
diff --git a/drivers/target/tcm_fc/tfc_conf.c b/drivers/target/tcm_fc/tfc_conf.c
index 870b7bb..747824f 100644
--- a/drivers/target/tcm_fc/tfc_conf.c
+++ b/drivers/target/tcm_fc/tfc_conf.c
@@ -426,7 +426,6 @@ static u32 ft_tpg_get_inst_index(struct se_portal_group *se_tpg)
 	.check_stop_free =		ft_check_stop_free,
 	.release_cmd =			ft_release_cmd,
 	.close_session =		ft_sess_close,
-	.sess_get_index =		ft_sess_get_index,
 	.write_pending =		ft_write_pending,
 	.set_default_node_attributes =	ft_set_default_node_attr,
 	.get_cmd_state =		ft_get_cmd_state,
diff --git a/drivers/target/tcm_fc/tfc_sess.c b/drivers/target/tcm_fc/tfc_sess.c
index 1d1c460..84a2c5d 100644
--- a/drivers/target/tcm_fc/tfc_sess.c
+++ b/drivers/target/tcm_fc/tfc_sess.c
@@ -326,13 +326,6 @@ void ft_sess_close(struct se_session *se_sess)
 	synchronize_rcu();		/* let transport deregister happen */
 }
 
-u32 ft_sess_get_index(struct se_session *se_sess)
-{
-	struct ft_sess *sess = se_sess->fabric_sess_ptr;
-
-	return sess->port_id;	/* XXX TBD probably not what is needed */
-}
-
 u32 ft_sess_get_port_name(struct se_session *se_sess,
 			  unsigned char *buf, u32 len)
 {
diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
index e81bdbd..7bdd7c5 100644
--- a/drivers/usb/gadget/function/f_tcm.c
+++ b/drivers/usb/gadget/function/f_tcm.c
@@ -1287,11 +1287,6 @@ static void usbg_release_cmd(struct se_cmd *se_cmd)
 	target_free_tag(se_sess, se_cmd);
 }
 
-static u32 usbg_sess_get_index(struct se_session *se_sess)
-{
-	return 0;
-}
-
 static void usbg_set_default_node_attrs(struct se_node_acl *nacl)
 {
 }
@@ -1715,7 +1710,6 @@ static int usbg_check_stop_free(struct se_cmd *se_cmd)
 	.tpg_check_prod_mode_write_protect = usbg_check_false,
 	.tpg_get_inst_index		= usbg_tpg_get_inst_index,
 	.release_cmd			= usbg_release_cmd,
-	.sess_get_index			= usbg_sess_get_index,
 	.write_pending			= usbg_send_write_request,
 	.set_default_node_attributes	= usbg_set_default_node_attrs,
 	.get_cmd_state			= usbg_get_cmd_state,
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index d7c9bfc..14f7c3bf 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -340,11 +340,6 @@ static void vhost_scsi_release_cmd(struct se_cmd *se_cmd)
 	target_free_tag(se_sess, se_cmd);
 }
 
-static u32 vhost_scsi_sess_get_index(struct se_session *se_sess)
-{
-	return 0;
-}
-
 static int vhost_scsi_write_pending(struct se_cmd *se_cmd)
 {
 	/* Go ahead and process the write immediately */
@@ -2244,7 +2239,6 @@ static void vhost_scsi_drop_tport(struct se_wwn *wwn)
 	.tpg_get_inst_index		= vhost_scsi_tpg_get_inst_index,
 	.release_cmd			= vhost_scsi_release_cmd,
 	.check_stop_free		= vhost_scsi_check_stop_free,
-	.sess_get_index			= vhost_scsi_sess_get_index,
 	.write_pending			= vhost_scsi_write_pending,
 	.set_default_node_attributes	= vhost_scsi_set_default_node_attrs,
 	.get_cmd_state			= vhost_scsi_get_cmd_state,
diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 883b15c..923c96a 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -1392,11 +1392,6 @@ static void scsiback_release_cmd(struct se_cmd *se_cmd)
 	target_free_tag(se_cmd->se_sess, se_cmd);
 }
 
-static u32 scsiback_sess_get_index(struct se_session *se_sess)
-{
-	return 0;
-}
-
 static int scsiback_write_pending(struct se_cmd *se_cmd)
 {
 	/* Go ahead and process the write immediately */
@@ -1768,7 +1763,6 @@ static int scsiback_check_false(struct se_portal_group *se_tpg)
 	.tpg_get_inst_index		= scsiback_tpg_get_inst_index,
 	.check_stop_free		= scsiback_check_stop_free,
 	.release_cmd			= scsiback_release_cmd,
-	.sess_get_index			= scsiback_sess_get_index,
 	.write_pending			= scsiback_write_pending,
 	.set_default_node_attributes	= scsiback_set_default_node_attrs,
 	.get_cmd_state			= scsiback_get_cmd_state,
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index af7d585..56be2ae 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -71,7 +71,6 @@ struct target_core_fabric_ops {
 	 */
 	void (*free_session)(struct se_session *);
 	void (*close_session)(struct se_session *);
-	u32 (*sess_get_index)(struct se_session *);
 	int (*write_pending)(struct se_cmd *);
 	void (*set_default_node_attributes)(struct se_node_acl *);
 	int (*get_cmd_state)(struct se_cmd *);
-- 
1.8.3.1

Re: [PATCH 17/17] target: drop sess_get_index

[Hannes Reinecke]

On 6/7/20 10:36 PM, Mike Christie wrote:
> Use the LIO session id for the scsiAttIntrPortIndex. iSCSI was
> already using this value and the other drivers used hard coded
> values of 1 or 0. The SCSI-MIB specs says:
> 
> This object represents an arbitrary integer used to uniquely
> identify a particular attached remote initiator port to a
> particular SCSI target port within a particular SCSI target device
> within a particular SCSI instance.
> 
> So the lio session sid can be used.
> 
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> ---
>   drivers/infiniband/ulp/srpt/ib_srpt.c        | 15 ---------------
>   drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c     |  6 ------
>   drivers/scsi/qla2xxx/tcm_qla2xxx.c           |  7 -------
>   drivers/target/iscsi/iscsi_target_configfs.c |  6 ------
>   drivers/target/loopback/tcm_loop.c           |  6 ------
>   drivers/target/sbp/sbp_target.c              |  6 ------
>   drivers/target/target_core_configfs.c        |  4 ----
>   drivers/target/target_core_stat.c            |  5 +----
>   drivers/target/tcm_fc/tfc_conf.c             |  1 -
>   drivers/target/tcm_fc/tfc_sess.c             |  7 -------
>   drivers/usb/gadget/function/f_tcm.c          |  6 ------
>   drivers/vhost/scsi.c                         |  6 ------
>   drivers/xen/xen-scsiback.c                   |  6 ------
>   include/target/target_core_fabric.h          |  1 -
>   14 files changed, 1 insertion(+), 81 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer