Commit 7466a7a3 authored by Martin K. Petersen's avatar Martin K. Petersen
Browse files

Merge patch series "scsi: target: make RTPI an TPG identifier"



Dmitry Bogdanov <d.bogdanov@yadro.com> says:

SAM-5 4.6.5.2 (Relative Port Identifier attribute) defines the attribute as
unique across SCSI target ports:

  The Relative Port Identifier attribute identifies a SCSI target port or a
  SCSI initiator port relative to other SCSI ports in a SCSI target device
  and any SCSI initiator devices contained within that SCSI target device. A
  SCSI target device may assign relative port identifiers to its SCSI target
  ports and any SCSI initiator ports. If relative port identifiers are
  assigned, the SCSI target device shall assign each of its SCSI target
  ports and any SCSI initiator ports a unique relative port identifier from
  1 to 65 535. SCSI target ports and SCSI initiator ports share the same
  number space.

In the current TCM implementation, auto-incremented lun_rtpi weakly follows
the model outlined by SAM-5 and SPC-4. In case of multiple SCSI target
ports (se_portal_groups), which is common to scenario with multiple HBAs or
multiple iSCSI/FC targets, it's possible to have two backstores (se_devices)
with different values of lun_rtpi on the same SCSI target port.

Similar issue happens during re-export. If a LUN of a backstore is removed
from a target port and added again to the same target port, RTPI is
incremented again and will be different from the first time.

The two issues happen because each se_device increments RTPI for its own
LUNs independently.

The behaviour means that a SCSI application client can't reliably make any
sense of RTPI values reported by a LUN as it's not really related to SCSI
target ports. A conforming target implementation must ensure that RTPI field
is unique per port. The patchset resolves the issue.

Make RTPI be part of se_tpg instead of se_lun. Make it configurable.

Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parents 04d02221 31177b74
Loading
Loading
Loading
Loading
+2 −2
Original line number Diff line number Diff line
@@ -225,7 +225,7 @@ target_emulate_report_target_port_groups(struct se_cmd *cmd)
			/*
			 * Set RELATIVE TARGET PORT IDENTIFIER
			 */
			put_unaligned_be16(lun->lun_rtpi, &buf[off]);
			put_unaligned_be16(lun->lun_tpg->tpg_rtpi, &buf[off]);
			off += 2;
			rd_len += 4;
		}
@@ -399,7 +399,7 @@ target_emulate_set_target_port_groups(struct se_cmd *cmd)
			spin_lock(&dev->se_port_lock);
			list_for_each_entry(lun, &dev->dev_sep_list,
							lun_dev_link) {
				if (lun->lun_rtpi != rtpi)
				if (lun->lun_tpg->tpg_rtpi != rtpi)
					continue;

				// XXX: racy unlock
+1 −42
Original line number Diff line number Diff line
@@ -223,7 +223,7 @@ struct se_dev_entry *core_get_se_deve_from_rtpi(
				tpg->se_tpg_tfo->fabric_name);
			continue;
		}
		if (lun->lun_rtpi != rtpi)
		if (lun->lun_tpg->tpg_rtpi != rtpi)
			continue;

		kref_get(&deve->pr_kref);
@@ -479,47 +479,6 @@ void core_clear_lun_from_tpg(struct se_lun *lun, struct se_portal_group *tpg)
	mutex_unlock(&tpg->acl_node_mutex);
}

int core_alloc_rtpi(struct se_lun *lun, struct se_device *dev)
{
	struct se_lun *tmp;

	spin_lock(&dev->se_port_lock);
	if (dev->export_count == 0x0000ffff) {
		pr_warn("Reached dev->dev_port_count =="
				" 0x0000ffff\n");
		spin_unlock(&dev->se_port_lock);
		return -ENOSPC;
	}
again:
	/*
	 * Allocate the next RELATIVE TARGET PORT IDENTIFIER for this struct se_device
	 * Here is the table from spc4r17 section 7.7.3.8.
	 *
	 *    Table 473 -- RELATIVE TARGET PORT IDENTIFIER field
	 *
	 * Code      Description
	 * 0h        Reserved
	 * 1h        Relative port 1, historically known as port A
	 * 2h        Relative port 2, historically known as port B
	 * 3h to FFFFh    Relative port 3 through 65 535
	 */
	lun->lun_rtpi = dev->dev_rpti_counter++;
	if (!lun->lun_rtpi)
		goto again;

	list_for_each_entry(tmp, &dev->dev_sep_list, lun_dev_link) {
		/*
		 * Make sure RELATIVE TARGET PORT IDENTIFIER is unique
		 * for 16-bit wrap..
		 */
		if (lun->lun_rtpi == tmp->lun_rtpi)
			goto again;
	}
	spin_unlock(&dev->se_port_lock);

	return 0;
}

static void se_release_vpd_for_dev(struct se_device *dev)
{
	struct t10_vpd *vpd, *vpd_tmp;
+39 −5
Original line number Diff line number Diff line
@@ -836,24 +836,56 @@ static ssize_t target_fabric_tpg_base_enable_store(struct config_item *item,

	if (se_tpg->enabled == op)
		return count;

	ret = se_tpg->se_tpg_tfo->fabric_enable_tpg(se_tpg, op);
	if (op)
		ret = target_tpg_enable(se_tpg);
	else
		ret = target_tpg_disable(se_tpg);
	if (ret)
		return ret;
	return count;
}
static ssize_t target_fabric_tpg_base_rtpi_show(struct config_item *item, char *page)
{
	struct se_portal_group *se_tpg = to_tpg(item);

	se_tpg->enabled = op;
	return sysfs_emit(page, "%#x\n", se_tpg->tpg_rtpi);
}

static ssize_t target_fabric_tpg_base_rtpi_store(struct config_item *item,
				   const char *page, size_t count)
{
	struct se_portal_group *se_tpg = to_tpg(item);
	u16 val;
	int ret;

	ret = kstrtou16(page, 0, &val);
	if (ret < 0)
		return ret;
	if (val == 0)
		return -EINVAL;

	if (se_tpg->enabled) {
		pr_info("%s_TPG[%hu] - Can not change RTPI on enabled TPG",
			se_tpg->se_tpg_tfo->fabric_name,
			se_tpg->se_tpg_tfo->tpg_get_tag(se_tpg));
		return -EINVAL;
	}

	se_tpg->tpg_rtpi = val;
	se_tpg->rtpi_manual = true;

	return count;
}

CONFIGFS_ATTR(target_fabric_tpg_base_, enable);
CONFIGFS_ATTR(target_fabric_tpg_base_, rtpi);

static int
target_fabric_setup_tpg_base_cit(struct target_fabric_configfs *tf)
{
	struct config_item_type *cit = &tf->tf_tpg_base_cit;
	struct configfs_attribute **attrs = NULL;
	size_t nr_attrs = 0;
	size_t nr_attrs = 1;
	int i = 0;

	if (tf->tf_ops->tfc_tpg_base_attrs)
@@ -876,7 +908,9 @@ target_fabric_setup_tpg_base_cit(struct target_fabric_configfs *tf)
			attrs[i] = tf->tf_ops->tfc_tpg_base_attrs[i];

	if (tf->tf_ops->fabric_enable_tpg)
		attrs[i] = &target_fabric_tpg_base_attr_enable;
		attrs[i++] = &target_fabric_tpg_base_attr_enable;

	attrs[i++] = &target_fabric_tpg_base_attr_rtpi;

done:
	cit->ct_item_ops = &target_fabric_tpg_base_item_ops;
+2 −1
Original line number Diff line number Diff line
@@ -59,7 +59,6 @@ struct target_fabric_configfs {
extern struct t10_alua_lu_gp *default_lu_gp;

/* target_core_device.c */
int	core_alloc_rtpi(struct se_lun *lun, struct se_device *dev);
struct se_dev_entry *core_get_se_deve_from_rtpi(struct se_node_acl *, u16);
void	target_pr_kref_release(struct kref *);
void	core_free_device_list_for_node(struct se_node_acl *,
@@ -132,6 +131,8 @@ void core_tpg_remove_lun(struct se_portal_group *, struct se_lun *);
struct se_node_acl *core_tpg_add_initiator_node_acl(struct se_portal_group *tpg,
		const char *initiatorname);
void core_tpg_del_initiator_node_acl(struct se_node_acl *acl);
int target_tpg_enable(struct se_portal_group *se_tpg);
int target_tpg_disable(struct se_portal_group *se_tpg);

/* target_core_transport.c */
int	init_se_kmem_caches(void);
+4 −4
Original line number Diff line number Diff line
@@ -663,7 +663,7 @@ static struct t10_pr_registration *__core_scsi3_do_alloc_registration(
	}
	pr_reg->pr_res_mapped_lun = mapped_lun;
	pr_reg->pr_aptpl_target_lun = lun->unpacked_lun;
	pr_reg->tg_pt_sep_rtpi = lun->lun_rtpi;
	pr_reg->tg_pt_sep_rtpi = lun->lun_tpg->tpg_rtpi;
	pr_reg->pr_res_key = sa_res_key;
	pr_reg->pr_reg_all_tg_pt = all_tg_pt;
	pr_reg->pr_reg_aptpl = aptpl;
@@ -967,7 +967,7 @@ static int __core_scsi3_check_aptpl_registration(
			rcu_read_unlock();

			pr_reg->pr_reg_nacl = nacl;
			pr_reg->tg_pt_sep_rtpi = lun->lun_rtpi;
			pr_reg->tg_pt_sep_rtpi = lun->lun_tpg->tpg_rtpi;
			list_del(&pr_reg->pr_reg_aptpl_list);
			spin_unlock(&pr_tmpl->aptpl_reg_lock);
			/*
@@ -1567,7 +1567,7 @@ core_scsi3_decode_spec_i_port(
			 */
			if (tmp_tpg->proto_id != proto_ident)
				continue;
			dest_rtpi = tmp_lun->lun_rtpi;
			dest_rtpi = tmp_lun->lun_tpg->tpg_rtpi;

			iport_ptr = NULL;
			i_str = target_parse_pr_out_transport_id(tmp_tpg,
@@ -3225,7 +3225,7 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd *cmd, u64 res_key,

	spin_lock(&dev->se_port_lock);
	list_for_each_entry(tmp_lun, &dev->dev_sep_list, lun_dev_link) {
		if (tmp_lun->lun_rtpi != rtpi)
		if (tmp_lun->lun_tpg->tpg_rtpi != rtpi)
			continue;
		dest_se_tpg = tmp_lun->lun_tpg;
		dest_tf_ops = dest_se_tpg->se_tpg_tfo;
Loading