[patch v5 1/2] RDMA/srp: Add parse function for maximum initiator to target IU size

[patch v5 1/2] RDMA/srp: Add parse function for maximum initiator to target IU size

[Honggang LI]

From: Honggang Li <honli@redhat.com>

According to SRP specifications 'srp-r16a' and 'srp2r06',
IOControllerProfile attributes for SRP target port include
the maximum initiator to target IU size.

SRP connection daemons, such as srp_daemon, can get the value
from subnet manager. The SRP connection daemon can pass this
value to kernel.

This patch add parse function for it.

Upstream commit [1] enables the kernel parameter, 'use_imm_data',
by default. [1] also use (8 * 1024) as the default value for
kernel parameter 'max_imm_data'. With those default values, the
maximum initiator to target IU size will be 8260.

In case the SRPT modules, which include the in-tree 'ib_srpt.ko'
module, do not support SRP-2 'immediate data' feature, the default
maximum initiator to target IU size is significantly smaller than
8260. For 'ib_srpt.ko' module, which built from source before
[2], the default maximum initiator to target IU is 2116.

[1] introduces a regression issue for old srp target with default
kernel parameters, as the connection will be reject because of
too large maximum initiator to target IU size.

[1] commit 882981f4a411 ("RDMA/srp: Add support for immediate data")
[2] commit 5dabcd0456d7 ("RDMA/srpt: Add support for immediate data")

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Honggang Li <honli@redhat.com>
---
 Documentation/ABI/stable/sysfs-driver-ib_srp |  2 ++
 drivers/infiniband/ulp/srp/ib_srp.c          | 10 ++++++++++
 drivers/infiniband/ulp/srp/ib_srp.h          |  1 +
 3 files changed, 13 insertions(+)

diff --git a/Documentation/ABI/stable/sysfs-driver-ib_srp b/Documentation/ABI/stable/sysfs-driver-ib_srp
index 7049a2b50359..84972a57caae 100644
--- a/Documentation/ABI/stable/sysfs-driver-ib_srp
+++ b/Documentation/ABI/stable/sysfs-driver-ib_srp
@@ -67,6 +67,8 @@ Description:	Interface for making ib_srp connect to a new target.
 		  initiator is allowed to queue per SCSI host. The default
 		  value for this parameter is 62. The lowest supported value
 		  is 2.
+		* max_it_iu_size, a decimal number specifying the maximum
+		  initiator to target information unit length.
 
 What:		/sys/class/infiniband_srp/srp-<hca>-<port_number>/ibdev
 Date:		January 2, 2006
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index b5960351bec0..b829dab0df77 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -3411,6 +3411,7 @@ enum {
 	SRP_OPT_IP_SRC		= 1 << 15,
 	SRP_OPT_IP_DEST		= 1 << 16,
 	SRP_OPT_TARGET_CAN_QUEUE= 1 << 17,
+	SRP_OPT_MAX_IT_IU_SIZE  = 1 << 18,
 };
 
 static unsigned int srp_opt_mandatory[] = {
@@ -3443,6 +3444,7 @@ static const match_table_t srp_opt_tokens = {
 	{ SRP_OPT_QUEUE_SIZE,		"queue_size=%d"		},
 	{ SRP_OPT_IP_SRC,		"src=%s"		},
 	{ SRP_OPT_IP_DEST,		"dest=%s"		},
+	{ SRP_OPT_MAX_IT_IU_SIZE,	"max_it_iu_size=%d"	},
 	{ SRP_OPT_ERR,			NULL 			}
 };
 
@@ -3736,6 +3738,14 @@ static int srp_parse_options(struct net *net, const char *buf,
 			target->tl_retry_count = token;
 			break;
 
+		case SRP_OPT_MAX_IT_IU_SIZE:
+			if (match_int(args, &token) || token < 0) {
+				pr_warn("bad maximum initiator to target IU size '%s'\n", p);
+				goto out;
+			}
+			target->max_it_iu_size = token;
+			break;
+
 		default:
 			pr_warn("unknown parameter or missing value '%s' in target creation request\n",
 				p);
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index b2861cd2087a..105b2bc6aa2f 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -209,6 +209,7 @@ struct srp_target_port {
 	u32			ch_count;
 	u32			lkey;
 	enum srp_target_state	state;
+	uint32_t		max_it_iu_size;
 	unsigned int		cmd_sg_cnt;
 	unsigned int		indirect_size;
 	bool			allow_ext_sg;
-- 
2.21.0

[patch v5 2/2] RDMA/srp: calculate max_it_iu_size if remote max_it_iu length available

[Honggang LI]

From: Honggang Li <honli@redhat.com>

The default maximum immediate size is too big for old srp clients,
which do not support immediate data.

According to the SRP and SRP-2 specifications, the IOControllerProfile
attributes for SRP target ports contains the maximum initiator to target
iu length.

The maximum initiator to target iu length can be obtained by sending
MAD packets to query subnet manager port and SRP target ports. We should
calculate the max_it_iu_size instead of the default value, when remote
maximum initiator to target iu length available.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Honggang Li <honli@redhat.com>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index b829dab0df77..b48088af2ea1 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1362,7 +1362,8 @@ static void srp_terminate_io(struct srp_rport *rport)
 }
 
 /* Calculate maximum initiator to target information unit length. */
-static uint32_t srp_max_it_iu_len(int cmd_sg_cnt, bool use_imm_data)
+static uint32_t srp_max_it_iu_len(int cmd_sg_cnt, bool use_imm_data,
+				  uint32_t max_it_iu_size)
 {
 	uint32_t max_iu_len = sizeof(struct srp_cmd) + SRP_MAX_ADD_CDB_LEN +
 		sizeof(struct srp_indirect_buf) +
@@ -1372,6 +1373,11 @@ static uint32_t srp_max_it_iu_len(int cmd_sg_cnt, bool use_imm_data)
 		max_iu_len = max(max_iu_len, SRP_IMM_DATA_OFFSET +
 				 srp_max_imm_data);
 
+	if (max_it_iu_size)
+		max_iu_len = min(max_iu_len, max_it_iu_size);
+
+	pr_debug("max_iu_len = %d\n", max_iu_len);
+
 	return max_iu_len;
 }
 
@@ -1389,7 +1395,8 @@ static int srp_rport_reconnect(struct srp_rport *rport)
 	struct srp_target_port *target = rport->lld_data;
 	struct srp_rdma_ch *ch;
 	uint32_t max_iu_len = srp_max_it_iu_len(target->cmd_sg_cnt,
-						srp_use_imm_data);
+						srp_use_imm_data,
+						target->max_it_iu_size);
 	int i, j, ret = 0;
 	bool multich = false;
 
@@ -2538,7 +2545,8 @@ static void srp_cm_rep_handler(struct ib_cm_id *cm_id,
 		ch->req_lim       = be32_to_cpu(lrsp->req_lim_delta);
 		ch->use_imm_data  = lrsp->rsp_flags & SRP_LOGIN_RSP_IMMED_SUPP;
 		ch->max_it_iu_len = srp_max_it_iu_len(target->cmd_sg_cnt,
-						      ch->use_imm_data);
+						      ch->use_imm_data,
+						      target->max_it_iu_size);
 		WARN_ON_ONCE(ch->max_it_iu_len >
 			     be32_to_cpu(lrsp->max_it_iu_len));
 
@@ -3897,7 +3905,9 @@ static ssize_t srp_create_target(struct device *dev,
 	target->mr_per_cmd = mr_per_cmd;
 	target->indirect_size = target->sg_tablesize *
 				sizeof (struct srp_direct_buf);
-	max_iu_len = srp_max_it_iu_len(target->cmd_sg_cnt, srp_use_imm_data);
+	max_iu_len = srp_max_it_iu_len(target->cmd_sg_cnt,
+				       srp_use_imm_data,
+				       target->max_it_iu_size);
 
 	INIT_WORK(&target->tl_err_work, srp_tl_err_work);
 	INIT_WORK(&target->remove_work, srp_remove_work);
-- 
2.21.0

Re: [patch v5 1/2] RDMA/srp: Add parse function for maximum initiator to target IU size

[Jason Gunthorpe]

On Sat, Sep 28, 2019 at 01:43:51AM +0800, Honggang LI wrote:
> From: Honggang Li <honli@redhat.com>
> 
> According to SRP specifications 'srp-r16a' and 'srp2r06',
> IOControllerProfile attributes for SRP target port include
> the maximum initiator to target IU size.
> 
> SRP connection daemons, such as srp_daemon, can get the value
> from subnet manager. The SRP connection daemon can pass this
> value to kernel.
> 
> This patch add parse function for it.
> 
> Upstream commit [1] enables the kernel parameter, 'use_imm_data',
> by default. [1] also use (8 * 1024) as the default value for
> kernel parameter 'max_imm_data'. With those default values, the
> maximum initiator to target IU size will be 8260.
> 
> In case the SRPT modules, which include the in-tree 'ib_srpt.ko'
> module, do not support SRP-2 'immediate data' feature, the default
> maximum initiator to target IU size is significantly smaller than
> 8260. For 'ib_srpt.ko' module, which built from source before
> [2], the default maximum initiator to target IU is 2116.
> 
> [1] introduces a regression issue for old srp target with default
> kernel parameters, as the connection will be reject because of
> too large maximum initiator to target IU size.
> 
> [1] commit 882981f4a411 ("RDMA/srp: Add support for immediate data")
> [2] commit 5dabcd0456d7 ("RDMA/srpt: Add support for immediate data")
> 
> Reviewed-by: Bart Van Assche <bvanassche@acm.org>
> Signed-off-by: Honggang Li <honli@redhat.com>
> ---
>  Documentation/ABI/stable/sysfs-driver-ib_srp |  2 ++
>  drivers/infiniband/ulp/srp/ib_srp.c          | 10 ++++++++++
>  drivers/infiniband/ulp/srp/ib_srp.h          |  1 +
>  3 files changed, 13 insertions(+)

Applied to for-next, thanks

Jason