[Stgt-devel] [PATCH] iscsi param cleanup

Pete Wyckoff pw
Fri Jul 27 01:18:13 CEST 2007


Remove inconsistencies in param structures and index usage.  This
seems like the cleanest way to keep the three separate lists of 19
parameters in sync.  The three spots are:  lookup of special params
by index #defines during login, session defaults, and target
defaults.  This fixes one real (although oddball) bug and several
potential bugs.

The real bug left the recv dlength size at 8k in spite of both
initiator and target wanting 256k: text_add_key thought 256k was
default and did not bother to send it.  The OFMarker code was errant
too, but it is unlikely that that was ever a problem for someone.

Target parameter defaults are not changed.  Except for the case
where you asked for a dlength of 256k using an explicit iscsiadm
target update command.

Further cleanups to iscsi_if.h would be possible.  Most of that is
legacy code from the target-in-kernel days.

Signed-off-by: Pete Wyckoff <pw at osc.edu>
---
 usr/iscsi/iscsi_if.h |   43 +++++++------------------------------------
 usr/iscsi/iscsid.h   |    4 ----
 usr/iscsi/param.c    |   43 ++++++++++++++++++++++++++++++++++++-------
 usr/iscsi/param.h    |    4 ++++
 usr/iscsi/target.c   |   38 +++++++++++++++++++-------------------
 5 files changed, 66 insertions(+), 66 deletions(-)

diff --git a/usr/iscsi/iscsi_if.h b/usr/iscsi/iscsi_if.h
index 9008b5f..58a76a2 100644
--- a/usr/iscsi/iscsi_if.h
+++ b/usr/iscsi/iscsi_if.h
@@ -192,10 +192,10 @@ enum iscsi_err {
 };
 
 /*
- * iSCSI Parameters (RFC3720)
+ * iSCSI Parameters (RFC3720).  Keep the session_keys and
+ * default_tgt_session_param arrays consistent with this list.
  */
 enum iscsi_param {
-	/* passed in using netlink set param */
 	ISCSI_PARAM_MAX_RECV_DLENGTH,
 	ISCSI_PARAM_MAX_XMIT_DLENGTH,
 	ISCSI_PARAM_HDRDGST_EN,
@@ -210,44 +210,15 @@ enum iscsi_param {
 	ISCSI_PARAM_ERL,
 	ISCSI_PARAM_IFMARKER_EN,
 	ISCSI_PARAM_OFMARKER_EN,
-	ISCSI_PARAM_EXP_STATSN,
-	ISCSI_PARAM_TARGET_NAME,
-	ISCSI_PARAM_TPGT,
-	ISCSI_PARAM_PERSISTENT_ADDRESS,
-	ISCSI_PARAM_PERSISTENT_PORT,
-	ISCSI_PARAM_SESS_RECOVERY_TMO,
-
-	/* pased in through bind conn using transport_fd */
-	ISCSI_PARAM_CONN_PORT,
-	ISCSI_PARAM_CONN_ADDRESS,
-
+	ISCSI_PARAM_DEFAULTTIME2WAIT,
+	ISCSI_PARAM_DEFAULTTIME2RETAIN,
+	ISCSI_PARAM_OFMARKINT,
+	ISCSI_PARAM_IFMARKINT,
+	ISCSI_PARAM_MAXCONNECTIONS,
 	/* must always be last */
 	ISCSI_PARAM_MAX,
 };
 
-#define ISCSI_MAX_RECV_DLENGTH		(1 << ISCSI_PARAM_MAX_RECV_DLENGTH)
-#define ISCSI_MAX_XMIT_DLENGTH		(1 << ISCSI_PARAM_MAX_XMIT_DLENGTH)
-#define ISCSI_HDRDGST_EN		(1 << ISCSI_PARAM_HDRDGST_EN)
-#define ISCSI_DATADGST_EN		(1 << ISCSI_PARAM_DATADGST_EN)
-#define ISCSI_INITIAL_R2T_EN		(1 << ISCSI_PARAM_INITIAL_R2T_EN)
-#define ISCSI_MAX_R2T			(1 << ISCSI_PARAM_MAX_R2T)
-#define ISCSI_IMM_DATA_EN		(1 << ISCSI_PARAM_IMM_DATA_EN)
-#define ISCSI_FIRST_BURST		(1 << ISCSI_PARAM_FIRST_BURST)
-#define ISCSI_MAX_BURST			(1 << ISCSI_PARAM_MAX_BURST)
-#define ISCSI_PDU_INORDER_EN		(1 << ISCSI_PARAM_PDU_INORDER_EN)
-#define ISCSI_DATASEQ_INORDER_EN	(1 << ISCSI_PARAM_DATASEQ_INORDER_EN)
-#define ISCSI_ERL			(1 << ISCSI_PARAM_ERL)
-#define ISCSI_IFMARKER_EN		(1 << ISCSI_PARAM_IFMARKER_EN)
-#define ISCSI_OFMARKER_EN		(1 << ISCSI_PARAM_OFMARKER_EN)
-#define ISCSI_EXP_STATSN		(1 << ISCSI_PARAM_EXP_STATSN)
-#define ISCSI_TARGET_NAME		(1 << ISCSI_PARAM_TARGET_NAME)
-#define ISCSI_TPGT			(1 << ISCSI_PARAM_TPGT)
-#define ISCSI_PERSISTENT_ADDRESS	(1 << ISCSI_PARAM_PERSISTENT_ADDRESS)
-#define ISCSI_PERSISTENT_PORT		(1 << ISCSI_PARAM_PERSISTENT_PORT)
-#define ISCSI_SESS_RECOVERY_TMO		(1 << ISCSI_PARAM_SESS_RECOVERY_TMO)
-#define ISCSI_CONN_PORT			(1 << ISCSI_PARAM_CONN_PORT)
-#define ISCSI_CONN_ADDRESS		(1 << ISCSI_PARAM_CONN_ADDRESS)
-
 #define iscsi_ptr(_handle) ((void*)(unsigned long)_handle)
 #define iscsi_handle(_ptr) ((uint64_t)(unsigned long)_ptr)
 #define hostdata_session(_hostdata) (iscsi_ptr(*(unsigned long *)_hostdata))
diff --git a/usr/iscsi/iscsid.h b/usr/iscsi/iscsid.h
index 6704488..a8c8cb1 100644
--- a/usr/iscsi/iscsid.h
+++ b/usr/iscsi/iscsid.h
@@ -60,10 +60,6 @@ struct iscsi_pdu {
 	unsigned int datasize;
 };
 
-#define KEY_STATE_START		0
-#define KEY_STATE_REQUEST	1
-#define KEY_STATE_DONE		2
-
 struct iscsi_session {
 	int refcount;
 
diff --git a/usr/iscsi/param.c b/usr/iscsi/param.c
index 29007d7..9eac62c 100644
--- a/usr/iscsi/param.c
+++ b/usr/iscsi/param.c
@@ -212,10 +212,10 @@ static int marker_val_to_str(unsigned int val, char *str)
 
 static int marker_set_val(struct param *param, int idx, unsigned int *val)
 {
-	if ((idx == ISCSI_OFMARKER_EN &&
-	     param[ISCSI_OFMARKER_EN].state == KEY_STATE_DONE) ||
-	    (idx == ISCSI_IFMARKER_EN &&
-	     param[ISCSI_IFMARKER_EN].state == KEY_STATE_DONE))
+	if ((idx == ISCSI_PARAM_OFMARKER_EN &&
+	     param[ISCSI_PARAM_OFMARKER_EN].state == KEY_STATE_DONE) ||
+	    (idx == ISCSI_PARAM_IFMARKER_EN &&
+	     param[ISCSI_PARAM_IFMARKER_EN].state == KEY_STATE_DONE))
 		*val = 0;
 	else
 		*val = 1;
@@ -297,25 +297,54 @@ static struct iscsi_key_ops marker_ops = {
 
 #define	SET_KEY_VALUES(x)	DEFAULT_NR_##x,MIN_NR_##x, MAX_NR_##x
 
+/*
+ * The defaults here are according to the spec and must not be changed,
+ * otherwise the initiator may make the wrong assumption.  If you want
+ * to change a value, edit the value in iscsi_target_create.
+ *
+ * The param MaxXmitDataSegmentLength doesn't really exist.  It's a way
+ * to remember the RDSL of the initiator, which defaults to 8k if he has
+ * not told us otherwise.
+ */
 struct iscsi_key session_keys[] = {
-	{"MaxRecvDataSegmentLength", 262144, 512, 16777215, &minimum_ops},
-	{"MaxXmitDataSegmentLength", 262144, 512, 16777215, &minimum_ops},
+	[ISCSI_PARAM_MAX_RECV_DLENGTH] =
+	{"MaxRecvDataSegmentLength", 8192, 512, 16777215, &minimum_ops},
+	[ISCSI_PARAM_MAX_XMIT_DLENGTH] =
+	{"MaxXmitDataSegmentLength", 8192, 512, 16777215, &minimum_ops},
+	[ISCSI_PARAM_HDRDGST_EN] =
 	{"HeaderDigest", DIGEST_NONE, DIGEST_NONE, DIGEST_ALL, &digest_ops},
+	[ISCSI_PARAM_DATADGST_EN] =
 	{"DataDigest", DIGEST_NONE, DIGEST_NONE, DIGEST_ALL, &digest_ops},
-	{"InitialR2T", 0, 0, 1, &or_ops},
+	[ISCSI_PARAM_INITIAL_R2T_EN] =
+	{"InitialR2T", 1, 0, 1, &or_ops},
+	[ISCSI_PARAM_MAX_R2T] =
 	{"MaxOutstandingR2T", 1, 1, 65535, &minimum_ops},
+	[ISCSI_PARAM_IMM_DATA_EN] =
 	{"ImmediateData", 1, 0, 1, &and_ops},
+	[ISCSI_PARAM_FIRST_BURST] =
 	{"FirstBurstLength", 65536, 512, 16777215, &minimum_ops},
+	[ISCSI_PARAM_MAX_BURST] =
 	{"MaxBurstLength", 262144, 512, 16777215, &minimum_ops},
+	[ISCSI_PARAM_PDU_INORDER_EN] =
 	{"DataPDUInOrder", 1, 0, 1, &or_ops},
+	[ISCSI_PARAM_DATASEQ_INORDER_EN] =
 	{"DataSequenceInOrder", 1, 0, 1, &or_ops},
+	[ISCSI_PARAM_ERL] =
 	{"ErrorRecoveryLevel", 0, 0, 2, &minimum_ops},
+	[ISCSI_PARAM_IFMARKER_EN] =
 	{"IFMarker", 0, 0, 1, &and_ops},
+	[ISCSI_PARAM_OFMARKER_EN] =
 	{"OFMarker", 0, 0, 1, &and_ops},
+	[ISCSI_PARAM_DEFAULTTIME2WAIT] =
 	{"DefaultTime2Wait", 2, 0, 3600, &maximum_ops},
+	[ISCSI_PARAM_DEFAULTTIME2RETAIN] =
 	{"DefaultTime2Retain", 20, 0, 3600, &minimum_ops},
+	[ISCSI_PARAM_OFMARKINT] =
 	{"OFMarkInt", 2048, 1, 65535, &marker_ops},
+	[ISCSI_PARAM_IFMARKINT] =
 	{"IFMarkInt", 2048, 1, 65535, &marker_ops},
+	[ISCSI_PARAM_MAXCONNECTIONS] =
 	{"MaxConnections", 1, 1, 65535, &minimum_ops},
+	[ISCSI_PARAM_MAX] =
 	{NULL,},
 };
diff --git a/usr/iscsi/param.h b/usr/iscsi/param.h
index fcee1e1..b5dd567 100644
--- a/usr/iscsi/param.h
+++ b/usr/iscsi/param.h
@@ -26,6 +26,10 @@ struct param {
 	unsigned int val;
 };
 
+#define KEY_STATE_START		0
+#define KEY_STATE_REQUEST	1
+#define KEY_STATE_DONE		2
+
 struct iscsi_key_ops {
 	int (*val_to_str)(unsigned int, char *);
 	int (*str_to_val)(char *, unsigned int *);
diff --git a/usr/iscsi/target.c b/usr/iscsi/target.c
index f2ecfd8..3e63485 100644
--- a/usr/iscsi/target.c
+++ b/usr/iscsi/target.c
@@ -265,25 +265,25 @@ int iscsi_target_create(struct target *t)
 	int tid = t->tid;
 	struct iscsi_target *target;
 	struct param default_tgt_session_param[] = {
-		{0, 8192},
-		{0, 8192},
-		{0, DIGEST_NONE},
-		{0, DIGEST_NONE},
-		{0, 1},
-		{0, 1},
-		{0, 1},
-		{0, 65536},
-		{0, 262144},
-		{0, 1},
-		{0, 1},
-		{0, 0},
-		{0, 0},
-		{0, 0},
-		{0, 2},
-		{0, 20},
-		{0, 2048},
-		{0, 2048},
-		{0, 1},
+		[ISCSI_PARAM_MAX_RECV_DLENGTH] = {0, 8192},
+		[ISCSI_PARAM_MAX_XMIT_DLENGTH] = {0, 8192},  /* do not edit */
+		[ISCSI_PARAM_HDRDGST_EN] = {0, DIGEST_NONE},
+		[ISCSI_PARAM_DATADGST_EN] = {0, DIGEST_NONE},
+		[ISCSI_PARAM_INITIAL_R2T_EN] = {0, 1},
+		[ISCSI_PARAM_MAX_R2T] = {0, 1},
+		[ISCSI_PARAM_IMM_DATA_EN] = {0, 1},
+		[ISCSI_PARAM_FIRST_BURST] = {0, 65536},
+		[ISCSI_PARAM_MAX_BURST] = {0, 262144},
+		[ISCSI_PARAM_PDU_INORDER_EN] = {0, 1},
+		[ISCSI_PARAM_DATASEQ_INORDER_EN] = {0, 1},
+		[ISCSI_PARAM_ERL] = {0, 0},
+		[ISCSI_PARAM_IFMARKER_EN] = {0, 0},
+		[ISCSI_PARAM_OFMARKER_EN] = {0, 0},
+		[ISCSI_PARAM_DEFAULTTIME2WAIT] = {0, 2},
+		[ISCSI_PARAM_DEFAULTTIME2RETAIN] = {0, 20},
+		[ISCSI_PARAM_OFMARKINT] = {0, 2048},
+		[ISCSI_PARAM_IFMARKINT] = {0, 2048},
+		[ISCSI_PARAM_MAXCONNECTIONS] = {0, 1},
 	};
 
 	target = malloc(sizeof(*target));
-- 
1.5.2.4




More information about the stgt mailing list