[Stgt-devel] FirstBurstLenght > MaxBurstLength --> error

[FUJITA Tomonori]

From: Albert Pauw <albert.pauw at gmail.com>
Subject: [Stgt-devel] FirstBurstLenght > MaxBurstLength --> error
Date: Sun, 15 Apr 2007 11:33:27 +0200

> Running the test regression script of open-iscsi on stgt I found an 
> error, which I did not encounter with other targets.
> 
> It is test #17 with the following parameters:
> 
> ================== TEST #17 BEGIN ====================
> ImmediateData = No
> InitialR2T = No
> HeaderDigest = None
> DataDigest = None
> FirstBurstLength = 8192
> MaxBurstLength = 4096
> MaxRecvDataSegmentLength = 4096
> MaxOutstandingR2T = 1
> 
> I get the error "Invalid burst lengths first_burst 8192 max_burst 4096" 
> in /var/log/messages and the login breaks of.
> According to the RFC 3720 "FirstBurstLength MUST NOT exceed 
> MaxBurstLength" (chap 12.14), which means that the target
> should answer back with a FirstBurstLenght parameters equal to 
> MaxBurstLenght.
> 
> I investigated with wireshark and found that this is exactly the 
> behaviour of the Wasabi Storagebuilder target,
> iscsi-target silently accepts FirstBurstLength > MaxBurstLength, and 
> stgt only gives an error.

I've attached a patch. It's not pretty but I don't have a better idea.


diff --git a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
index 2377952..d973409 100644
--- a/usr/iscsi/iscsid.c
+++ b/usr/iscsi/iscsid.c
@@ -226,8 +226,10 @@ static void login_security_done(struct i
 static void text_scan_login(struct iscsi_connection *conn)
 {
 	char *key, *value, *data;
-	int datasize, idx;
+	int datasize, idx, first_burst = 0;
 	struct iscsi_login_rsp *rsp = (struct iscsi_login_rsp *)&conn->rsp.bhs;
+	struct param *p = conn->session_param;
+	char buf[32];
 
 	data = conn->req.data;
 	datasize = conn->req.datasize;
@@ -240,14 +242,12 @@ static void text_scan_login(struct iscsi
 		else if (!((idx = param_index_by_name(key, session_keys)) < 0)) {
 			int err;
 			unsigned int val;
-			char buf[32];
 
 			if (idx == ISCSI_PARAM_MAX_RECV_DLENGTH)
 				idx = ISCSI_PARAM_MAX_XMIT_DLENGTH;
 
 			if (param_str_to_val(session_keys, idx, value, &val) < 0) {
-				if (conn->session_param[idx].state
-				    == KEY_STATE_START) {
+				if (p[idx].state == KEY_STATE_START) {
 					text_key_add_reject(conn, key);
 					continue;
 				} else {
@@ -261,9 +261,14 @@ static void text_scan_login(struct iscsi
 			}
 
 			err = param_check_val(session_keys, idx, &val);
-			err = param_set_val(session_keys, conn->session_param, idx, &val);
+			if (idx == ISCSI_PARAM_FIRST_BURST &&
+			    p[idx].state == KEY_STATE_START) {
+				first_burst = val;
+				continue;
+			}
+			err = param_set_val(session_keys, p, idx, &val);
 
-			switch (conn->session_param[idx].state) {
+			switch (p[idx].state) {
 			case KEY_STATE_START:
 				if (idx == ISCSI_PARAM_MAX_XMIT_DLENGTH)
 					break;
@@ -272,25 +277,35 @@ static void text_scan_login(struct iscsi
 				text_key_add(conn, key, buf);
 				break;
 			case KEY_STATE_REQUEST:
-				if (val != conn->session_param[idx].val) {
+				if (val != p[idx].val) {
 					rsp->status_class =
 						ISCSI_STATUS_CLS_INITIATOR_ERR;
 					rsp->status_detail =
 						ISCSI_LOGIN_STATUS_INIT_ERR;
 					conn->state = STATE_EXIT;
-					log_warning("%s %u %u\n", key,
-					val, conn->session_param[idx].val);
+					eprintf("invalid response %s %u %u\n",
+						key, val, p[idx].val);
 					goto out;
 				}
 				break;
 			case KEY_STATE_DONE:
 				break;
 			}
-			conn->session_param[idx].state = KEY_STATE_DONE;
+			p[idx].state = KEY_STATE_DONE;
 		} else
 			text_key_add(conn, key, "NotUnderstood");
 	}
 
+	if (first_burst) {
+		idx = ISCSI_PARAM_MAX_BURST;
+		if (first_burst > p[idx].val)
+			first_burst = p[idx].val;
+		idx = ISCSI_PARAM_FIRST_BURST;
+		p[idx].state = KEY_STATE_DONE;
+		param_set_val(session_keys, p, idx, &first_burst);
+		param_val_to_str(session_keys, idx, first_burst, buf);
+		text_key_add(conn, "FirstBurstLength", buf);
+	}
 out:
 	return;
 }