diff mbox series

[v2,12/12] target: add virtual remote target

Message ID 20230307080742.24631-13-d.bogdanov@yadro.com (mailing list archive)
State Superseded
Headers show
Series add virtual remote fabric | expand

Commit Message

Dmitry Bogdanov March 7, 2023, 8:07 a.m. UTC
Create virtual remote target module.
It can be used to see a whole acl/lun/tpg configuration from all nodes
in storage cluster.
For example, it allows to setup remote ports in ALUA port groups. To
report all ports in a cluster in REPORT TARGET PORT GROUP command.

Suggested-by: Konstantin Shelekhin <k.shelekhin@yadro.com>
Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
---
v2:
   code style cleanup
   remove default ops implementations
---
 drivers/target/Kconfig                 |   1 +
 drivers/target/Makefile                |   1 +
 drivers/target/tcm_remote/Kconfig      |   8 +
 drivers/target/tcm_remote/Makefile     |   2 +
 drivers/target/tcm_remote/tcm_remote.c | 277 +++++++++++++++++++++++++
 drivers/target/tcm_remote/tcm_remote.h |  20 ++
 6 files changed, 309 insertions(+)
 create mode 100644 drivers/target/tcm_remote/Kconfig
 create mode 100644 drivers/target/tcm_remote/Makefile
 create mode 100644 drivers/target/tcm_remote/tcm_remote.c
 create mode 100644 drivers/target/tcm_remote/tcm_remote.h

Comments

Mike Christie March 10, 2023, 10:32 p.m. UTC | #1
Just some nits.

On 3/7/23 2:07 AM, Dmitry Bogdanov wrote:
> +
> +static int tcm_remote_port_link(
> +	struct se_portal_group *se_tpg,
> +	struct se_lun *lun)
> +{
> +	pr_debug("TCM_Remote_ConfigFS: Port Link LUN %lld Successful\n",
> +		  lun->unpacked_lun);

The l in lun should be one space to the left so it's under the ".
It will then match the other code and checkpatch won't complain.


> +	return 0;
> +}
> +
> +static void tcm_remote_port_unlink(
> +	struct se_portal_group *se_tpg,
> +	struct se_lun *lun)
> +{
> +	pr_debug("TCM_Remote_ConfigFS: Port Unlink LUN %lld Successful\n",
> +		  lun->unpacked_lun);

Same as above.

> +}
> +
> +static struct se_portal_group *tcm_remote_make_tpg(
> +	struct se_wwn *wwn,
> +	const char *name)
> +{
> +	struct tcm_remote_hba *remote_hba = container_of(wwn,
> +			struct tcm_remote_hba, remote_hba_wwn);
> +	struct tcm_remote_tpg *remote_tpg;
> +	unsigned long tpgt;
> +	int ret;
> +
> +	if (strstr(name, "tpgt_") != name) {
> +		pr_err("Unable to locate \"tpgt_#\" directory group\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +	if (kstrtoul(name+5, 10, &tpgt))

Add spaces so it looks like: "name + 5"



> +}
> +
> +static ssize_t tcm_remote_wwn_version_show(struct config_item *item, char *page)
> +{
> +	return sprintf(page, "TCM Remote Fabric module %s\n", TCM_REMOTE_VERSION);
> +}


Do we need this? I saw other LIO drivers like iscsi, fcoe and loop have them
but they are never updated so it seems useless.

For something like stable trying to manage versions is going to be difficult
as well, so it might just be more confusing.

> +
> +static int __init tcm_remote_fabric_init(void)
> +{
> +	int ret = -ENOMEM;

You can drop the -ENOMEM since we set ret the next line.

> +
> +	ret = target_register_template(&remote_ops);
> +	if (ret)
> +		goto out;
> +
> +	return 0;
> +
> +out:
> +	return ret;
> +}
> +
Dmitry Bogdanov March 13, 2023, 2:13 p.m. UTC | #2
On Fri, Mar 10, 2023 at 04:32:32PM -0600, Mike Christie wrote:
> 
> Just some nits.
> 
> On 3/7/23 2:07 AM, Dmitry Bogdanov wrote:
> > +
> > +static int tcm_remote_port_link(
> > +     struct se_portal_group *se_tpg,
> > +     struct se_lun *lun)
> > +{
> > +     pr_debug("TCM_Remote_ConfigFS: Port Link LUN %lld Successful\n",
> > +               lun->unpacked_lun);
> 
> The l in lun should be one space to the left so it's under the ".
> It will then match the other code and checkpatch won't complain.
> 
> 
> > +     return 0;
> > +}
> > +
> > +static void tcm_remote_port_unlink(
> > +     struct se_portal_group *se_tpg,
> > +     struct se_lun *lun)
> > +{
> > +     pr_debug("TCM_Remote_ConfigFS: Port Unlink LUN %lld Successful\n",
> > +               lun->unpacked_lun);
> 
> Same as above.
> 
> > +}
> > +
> > +static struct se_portal_group *tcm_remote_make_tpg(
> > +     struct se_wwn *wwn,
> > +     const char *name)
> > +{
> > +     struct tcm_remote_hba *remote_hba = container_of(wwn,
> > +                     struct tcm_remote_hba, remote_hba_wwn);
> > +     struct tcm_remote_tpg *remote_tpg;
> > +     unsigned long tpgt;
> > +     int ret;
> > +
> > +     if (strstr(name, "tpgt_") != name) {
> > +             pr_err("Unable to locate \"tpgt_#\" directory group\n");
> > +             return ERR_PTR(-EINVAL);
> > +     }
> > +     if (kstrtoul(name+5, 10, &tpgt))
> 
> Add spaces so it looks like: "name + 5"
> 
> 
> 
> > +}
> > +
> > +static ssize_t tcm_remote_wwn_version_show(struct config_item *item, char *page)
> > +{
> > +     return sprintf(page, "TCM Remote Fabric module %s\n", TCM_REMOTE_VERSION);
> > +}
> 
> 
> Do we need this? I saw other LIO drivers like iscsi, fcoe and loop have them
> but they are never updated so it seems useless.
> 
> For something like stable trying to manage versions is going to be difficult
> as well, so it might just be more confusing.
> 

I absolutely agree that in-kernel modules should not have its own
versions. Kernel version is enough. 

But targetcli tool reads that file for a fabric and I don't want to
work it around when I will add support of remote fabric to it.
So, I am going to keep the version attribute.

> > +
> > +static int __init tcm_remote_fabric_init(void)
> > +{
> > +     int ret = -ENOMEM;
> 
> You can drop the -ENOMEM since we set ret the next line.

I can drop ret variable here at all.

> > +
> > +     ret = target_register_template(&remote_ops);
> > +     if (ret)
> > +             goto out;
> > +
> > +     return 0;
> > +
> > +out:
> > +     return ret;
> > +}
> > +
>
diff mbox series

Patch

diff --git a/drivers/target/Kconfig b/drivers/target/Kconfig
index 72171ea3dd53..92641d39126a 100644
--- a/drivers/target/Kconfig
+++ b/drivers/target/Kconfig
@@ -47,5 +47,6 @@  source "drivers/target/loopback/Kconfig"
 source "drivers/target/tcm_fc/Kconfig"
 source "drivers/target/iscsi/Kconfig"
 source "drivers/target/sbp/Kconfig"
+source "drivers/target/tcm_remote/Kconfig"
 
 endif
diff --git a/drivers/target/Makefile b/drivers/target/Makefile
index 45634747377e..431b84abfb94 100644
--- a/drivers/target/Makefile
+++ b/drivers/target/Makefile
@@ -30,3 +30,4 @@  obj-$(CONFIG_LOOPBACK_TARGET)	+= loopback/
 obj-$(CONFIG_TCM_FC)		+= tcm_fc/
 obj-$(CONFIG_ISCSI_TARGET)	+= iscsi/
 obj-$(CONFIG_SBP_TARGET)	+= sbp/
+obj-$(CONFIG_REMOTE_TARGET)	+= tcm_remote/
diff --git a/drivers/target/tcm_remote/Kconfig b/drivers/target/tcm_remote/Kconfig
new file mode 100644
index 000000000000..e6bebb5fe6f1
--- /dev/null
+++ b/drivers/target/tcm_remote/Kconfig
@@ -0,0 +1,8 @@ 
+# SPDX-License-Identifier: GPL-2.0-only
+config REMOTE_TARGET
+	tristate "TCM Virtual Remote target"
+	depends on SCSI
+	help
+	  Say Y here to enable the TCM Virtual Remote fabric
+	  That fabric is a dummy fabric to tell TCM about configuration
+	  of TPG/ACL/LUN on peer nodes in a cluster.
diff --git a/drivers/target/tcm_remote/Makefile b/drivers/target/tcm_remote/Makefile
new file mode 100644
index 000000000000..5818ffd0b0fa
--- /dev/null
+++ b/drivers/target/tcm_remote/Makefile
@@ -0,0 +1,2 @@ 
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_REMOTE_TARGET)	+= tcm_remote.o
diff --git a/drivers/target/tcm_remote/tcm_remote.c b/drivers/target/tcm_remote/tcm_remote.c
new file mode 100644
index 000000000000..1b7ce7803685
--- /dev/null
+++ b/drivers/target/tcm_remote/tcm_remote.c
@@ -0,0 +1,277 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/configfs.h>
+#include <scsi/scsi.h>
+#include <scsi/scsi_tcq.h>
+#include <scsi/scsi_host.h>
+#include <scsi/scsi_device.h>
+#include <scsi/scsi_cmnd.h>
+
+#include <target/target_core_base.h>
+#include <target/target_core_fabric.h>
+
+#include "tcm_remote.h"
+
+static inline struct tcm_remote_tpg *remote_tpg(struct se_portal_group *se_tpg)
+{
+	return container_of(se_tpg, struct tcm_remote_tpg, remote_se_tpg);
+}
+
+static char *tcm_remote_get_endpoint_wwn(struct se_portal_group *se_tpg)
+{
+	/*
+	 * Return the passed NAA identifier for the Target Port
+	 */
+	return &remote_tpg(se_tpg)->remote_hba->remote_wwn_address[0];
+}
+
+static u16 tcm_remote_get_tag(struct se_portal_group *se_tpg)
+{
+	/*
+	 * This Tag is used when forming SCSI Name identifier in EVPD=1 0x83
+	 * to represent the SCSI Target Port.
+	 */
+	return remote_tpg(se_tpg)->remote_tpgt;
+}
+
+static int tcm_remote_dummy_cmd_fn(struct se_cmd *se_cmd)
+{
+	return 0;
+}
+
+static void tcm_remote_dummy_cmd_void_fn(struct se_cmd *se_cmd)
+{
+
+}
+
+static char *tcm_remote_dump_proto_id(struct tcm_remote_hba *remote_hba)
+{
+	switch (remote_hba->remote_proto_id) {
+	case SCSI_PROTOCOL_SAS:
+		return "SAS";
+	case SCSI_PROTOCOL_SRP:
+		return "SRP";
+	case SCSI_PROTOCOL_FCP:
+		return "FCP";
+	case SCSI_PROTOCOL_ISCSI:
+		return "iSCSI";
+	default:
+		break;
+	}
+
+	return "Unknown";
+}
+
+static int tcm_remote_port_link(
+	struct se_portal_group *se_tpg,
+	struct se_lun *lun)
+{
+	pr_debug("TCM_Remote_ConfigFS: Port Link LUN %lld Successful\n",
+		  lun->unpacked_lun);
+	return 0;
+}
+
+static void tcm_remote_port_unlink(
+	struct se_portal_group *se_tpg,
+	struct se_lun *lun)
+{
+	pr_debug("TCM_Remote_ConfigFS: Port Unlink LUN %lld Successful\n",
+		  lun->unpacked_lun);
+}
+
+static struct se_portal_group *tcm_remote_make_tpg(
+	struct se_wwn *wwn,
+	const char *name)
+{
+	struct tcm_remote_hba *remote_hba = container_of(wwn,
+			struct tcm_remote_hba, remote_hba_wwn);
+	struct tcm_remote_tpg *remote_tpg;
+	unsigned long tpgt;
+	int ret;
+
+	if (strstr(name, "tpgt_") != name) {
+		pr_err("Unable to locate \"tpgt_#\" directory group\n");
+		return ERR_PTR(-EINVAL);
+	}
+	if (kstrtoul(name+5, 10, &tpgt))
+		return ERR_PTR(-EINVAL);
+
+	if (tpgt >= TL_TPGS_PER_HBA) {
+		pr_err("Passed tpgt: %lu exceeds TL_TPGS_PER_HBA: %u\n",
+		       tpgt, TL_TPGS_PER_HBA);
+		return ERR_PTR(-EINVAL);
+	}
+	remote_tpg = &remote_hba->remote_hba_tpgs[tpgt];
+	remote_tpg->remote_hba = remote_hba;
+	remote_tpg->remote_tpgt = tpgt;
+	/*
+	 * Register the remote_tpg as a emulated TCM Target Endpoint
+	 */
+	ret = core_tpg_register(wwn, &remote_tpg->remote_se_tpg,
+				remote_hba->remote_proto_id);
+	if (ret < 0)
+		return ERR_PTR(-ENOMEM);
+
+	pr_debug("TCM_Remote_ConfigFS: Allocated Emulated %s Target Port %s,t,0x%04lx\n",
+		 tcm_remote_dump_proto_id(remote_hba),
+		 config_item_name(&wwn->wwn_group.cg_item), tpgt);
+	return &remote_tpg->remote_se_tpg;
+}
+
+static void tcm_remote_drop_tpg(struct se_portal_group *se_tpg)
+{
+	struct se_wwn *wwn = se_tpg->se_tpg_wwn;
+	struct tcm_remote_tpg *remote_tpg = container_of(se_tpg,
+				struct tcm_remote_tpg, remote_se_tpg);
+	struct tcm_remote_hba *remote_hba;
+	unsigned short tpgt;
+
+	remote_hba = remote_tpg->remote_hba;
+	tpgt = remote_tpg->remote_tpgt;
+
+	/*
+	 * Deregister the remote_tpg as a emulated TCM Target Endpoint
+	 */
+	core_tpg_deregister(se_tpg);
+
+	remote_tpg->remote_hba = NULL;
+	remote_tpg->remote_tpgt = 0;
+
+	pr_debug("TCM_Remote_ConfigFS: Deallocated Emulated %s Target Port %s,t,0x%04x\n",
+		 tcm_remote_dump_proto_id(remote_hba),
+		 config_item_name(&wwn->wwn_group.cg_item), tpgt);
+}
+
+static struct se_wwn *tcm_remote_make_wwn(
+	struct target_fabric_configfs *tf,
+	struct config_group *group,
+	const char *name)
+{
+	struct tcm_remote_hba *remote_hba;
+	char *ptr;
+	int ret, off = 0;
+
+	remote_hba = kzalloc(sizeof(*remote_hba), GFP_KERNEL);
+	if (!remote_hba)
+		return ERR_PTR(-ENOMEM);
+
+	/*
+	 * Determine the emulated Protocol Identifier and Target Port Name
+	 * based on the incoming configfs directory name.
+	 */
+	ptr = strstr(name, "naa.");
+	if (ptr) {
+		remote_hba->remote_proto_id = SCSI_PROTOCOL_SAS;
+		goto check_len;
+	}
+	ptr = strstr(name, "fc.");
+	if (ptr) {
+		remote_hba->remote_proto_id = SCSI_PROTOCOL_FCP;
+		off = 3; /* Skip over "fc." */
+		goto check_len;
+	}
+	ptr = strstr(name, "0x");
+	if (ptr) {
+		remote_hba->remote_proto_id = SCSI_PROTOCOL_SRP;
+		off = 2; /* Skip over "0x" */
+		goto check_len;
+	}
+	ptr = strstr(name, "iqn.");
+	if (!ptr) {
+		pr_err("Unable to locate prefix for emulated Target Port: %s\n",
+		       name);
+		ret = -EINVAL;
+		goto out;
+	}
+	remote_hba->remote_proto_id = SCSI_PROTOCOL_ISCSI;
+
+check_len:
+	if (strlen(name) >= TL_WWN_ADDR_LEN) {
+		pr_err("Emulated NAA %s Address: %s, exceeds max: %d\n",
+		       name, tcm_remote_dump_proto_id(remote_hba), TL_WWN_ADDR_LEN);
+		ret = -EINVAL;
+		goto out;
+	}
+	snprintf(&remote_hba->remote_wwn_address[0], TL_WWN_ADDR_LEN, "%s", &name[off]);
+
+	pr_debug("TCM_Remote_ConfigFS: Allocated emulated Target %s Address: %s\n",
+		 tcm_remote_dump_proto_id(remote_hba), name);
+	return &remote_hba->remote_hba_wwn;
+out:
+	kfree(remote_hba);
+	return ERR_PTR(ret);
+}
+
+static void tcm_remote_drop_wwn(struct se_wwn *wwn)
+{
+	struct tcm_remote_hba *remote_hba = container_of(wwn,
+				struct tcm_remote_hba, remote_hba_wwn);
+
+	pr_debug("TCM_Remote_ConfigFS: Deallocating emulated Target %s Address: %s\n",
+		 tcm_remote_dump_proto_id(remote_hba),
+		 remote_hba->remote_wwn_address);
+	kfree(remote_hba);
+}
+
+static ssize_t tcm_remote_wwn_version_show(struct config_item *item, char *page)
+{
+	return sprintf(page, "TCM Remote Fabric module %s\n", TCM_REMOTE_VERSION);
+}
+
+CONFIGFS_ATTR_RO(tcm_remote_wwn_, version);
+
+static struct configfs_attribute *tcm_remote_wwn_attrs[] = {
+	&tcm_remote_wwn_attr_version,
+	NULL,
+};
+
+static const struct target_core_fabric_ops remote_ops = {
+	.module				= THIS_MODULE,
+	.fabric_name			= "remote",
+	.tpg_get_wwn			= tcm_remote_get_endpoint_wwn,
+	.tpg_get_tag			= tcm_remote_get_tag,
+	.check_stop_free		= tcm_remote_dummy_cmd_fn,
+	.release_cmd			= tcm_remote_dummy_cmd_void_fn,
+	.write_pending			= tcm_remote_dummy_cmd_fn,
+	.queue_data_in			= tcm_remote_dummy_cmd_fn,
+	.queue_status			= tcm_remote_dummy_cmd_fn,
+	.queue_tm_rsp			= tcm_remote_dummy_cmd_void_fn,
+	.aborted_task			= tcm_remote_dummy_cmd_void_fn,
+	.fabric_make_wwn		= tcm_remote_make_wwn,
+	.fabric_drop_wwn		= tcm_remote_drop_wwn,
+	.fabric_make_tpg		= tcm_remote_make_tpg,
+	.fabric_drop_tpg		= tcm_remote_drop_tpg,
+	.fabric_post_link		= tcm_remote_port_link,
+	.fabric_pre_unlink		= tcm_remote_port_unlink,
+	.tfc_wwn_attrs			= tcm_remote_wwn_attrs,
+};
+
+static int __init tcm_remote_fabric_init(void)
+{
+	int ret = -ENOMEM;
+
+	ret = target_register_template(&remote_ops);
+	if (ret)
+		goto out;
+
+	return 0;
+
+out:
+	return ret;
+}
+
+static void __exit tcm_remote_fabric_exit(void)
+{
+	target_unregister_template(&remote_ops);
+}
+
+MODULE_DESCRIPTION("TCM virtual remote target");
+MODULE_AUTHOR("Dmitry Bogdanov <d.bogdanov@yadro.com>");
+MODULE_LICENSE("GPL");
+module_init(tcm_remote_fabric_init);
+module_exit(tcm_remote_fabric_exit);
diff --git a/drivers/target/tcm_remote/tcm_remote.h b/drivers/target/tcm_remote/tcm_remote.h
new file mode 100644
index 000000000000..913d1a6eb3a2
--- /dev/null
+++ b/drivers/target/tcm_remote/tcm_remote.h
@@ -0,0 +1,20 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <linux/types.h>
+#include <linux/device.h>
+
+#define TCM_REMOTE_VERSION		"v0.1"
+#define TL_WWN_ADDR_LEN			256
+#define TL_TPGS_PER_HBA			32
+
+struct tcm_remote_tpg {
+	unsigned short remote_tpgt;
+	struct se_portal_group remote_se_tpg;
+	struct tcm_remote_hba *remote_hba;
+};
+
+struct tcm_remote_hba {
+	u8 remote_proto_id;
+	unsigned char remote_wwn_address[TL_WWN_ADDR_LEN];
+	struct tcm_remote_tpg remote_hba_tpgs[TL_TPGS_PER_HBA];
+	struct se_wwn remote_hba_wwn;
+};